Refactor Claro's container-inline.module stylesheet

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

container-inline.module.css file is not updated, I have attached the path with the updated file. Attached interdiff for same.

Ignore patch in #32.

Since the .css file changed think this will require before/after screenshots.

Added shorthand properties, attached interdiff for same. leaving it as NW, as it still needs after patch screenshot.

#36 Patch applied successfully.
For ref attached screenshot.

Testing by using the cd_core test modules.

Attached before/after screenshots of a fieldset. Can see nothing changed.

Before

After

The last submitted patch, 34: 3293997-32.patch, failed testing. View results →

The screenshots in #38 appear to be from Olivero (while this patch only affects Claro).

The CSS looks fine. The only change I would make is to use 2px instead of 0.125em. Core includes the pxToRem PostCSS plugin that automatically converts it to REM. If we specify px, it'd be easier to read and would negate the need for the comment.

Also, where is this being used? The only place I found was in datetime-form.html.twig, and I could find where that was used?

@mherchel that's my mistake using those claro test modules.

But the template can be found on a node edit form, the author on field.

Here's a screenshot that shows nothing was broken but the PM on the time is cut off. Think if we are updating the css might as well fix that right?

Addressed #40, Made use of px unit on place of em. Attached interdiff for same.

@smustgrave, I can confirm that #41, is happening before patch as well. We can open a follow up issue to address it.

Personally think it would be good to try and fix now but won't hold the ticket up.

The last submitted patch, 42: 3293997-42.patch, failed testing. View results →

@Team, Could we have created variables for the Margin that we are using in the "container-inline.module.css" file?.

Created a new patch. Added margin variables in the "container-inline.module.css" file Please review it.

For #46 please include interdiffs and check your patches before uploading.

Remarking RTBC as #42 was previously marked and had a random failure.

The last submitted patch, 42: 3293997-42.patch, failed testing. View results →

One question is that we're supposed to have px in the pcss file but rem in the generated css file. That isn't the case here is this expected?

One question is that we're supposed to have px in the pcss file but rem in the generated css file. That isn't the case here is this expected?

Only values 3px or greater get converted to REM (see https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/scripts/css...). This was added in the initial implementation of PxToRem at https://www.drupal.org/project/drupal/issues/3117698#comment-13857994 →

Thanks for the confirmation

Committed e2fde99 and pushed to 10.1.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.