Problem/Motivation

This is part of the CSS modernization initiative.

Steps to reproduce

The stylesheet at https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/themes/claro/css/components/fieldset.pcss.css needs to be refactored to make use of modern CSS and Drupal core's PostCSS tooling.

Proposed resolution

Use CSS Logical Properties where appropriate
Use CSS nesting where appropriate

Remaining tasks

We need two patches. One for Drupal 9.5.x and one for Drupal 10.0.x
We need a followup issue to refactor this component in Drupal 10.0.x to make use of component-level CSS custom properties and remove IE specific style definitions.

User interface changes

None. There should be no visual differences.

CommentFileSizeAuthor
#15 3303548-15.patch6.67 KBstanzin
#12 Screenshot 2023-06-29 at 10.12.39 AM.png239.65 KBgauravvvv
#10 interdiff-8_10.txt3.25 KBgauravvvv
#10 3303548-10.patch6.68 KBgauravvvv
#8 3303548-8.patch7.52 KBstanzin
#3 3303548-version10.0.x.patch7.97 KBaditya4478
#2 3303548-version9.5.x.patch8.59 KBaditya4478

Comments

sasanikolic created an issue. See original summary.

aditya4478’s picture

aditya4478 commented
Status: Active » Needs review
StatusFileSize
new 3303548-version9.5.x.patch8.59 KB
aditya4478’s picture

aditya4478 commented
Version: 9.5.x-dev » 10.0.x-dev
StatusFileSize
new 3303548-version10.0.x.patch7.97 KB
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Parent issue: » #3324398: [META] Update Claro CSS with new coding standards
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Postponed

Believe this is postponed via our conversation a few weeks ago @ckrina. For the followup and a path forward

aditya4478’s picture

aditya4478 commented
Status: Postponed » Needs work
stanzin’s picture

stanzin commented
Version: 10.0.x-dev » 11.x-dev
Status: Needs work » Needs review
StatusFileSize
new 3303548-8.patch7.52 KB
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs followup +Needs screenshots

Can we get before/after screenshots added to the IS please.

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review
StatusFileSize
new 3303548-10.patch6.68 KB
new interdiff-8_10.txt3.25 KB

I have fixed the selectors and short hand properties. Attached interdiff for same. So many wrong selectors were used in patch #8

Exp: +--composite.fieldset__legend {

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Can we get screenshots please.

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review
Issue tags: -Needs screenshots
StatusFileSize
new Screenshot 2023-06-29 at 10.12.39 AM.png239.65 KB

I have added before and after patch screenshot.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 3303548-10.patch, failed testing. View results

stanzin’s picture

stanzin commented
Status: Needs work » Needs review
StatusFileSize
new 3303548-15.patch6.67 KB

Re-Rolling patch. I think #10 patch is failed due to unrelated failure.

aditya4478’s picture

aditya4478 commented
Status: Needs review » Reviewed & tested by the community

LGTM !

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: 3303548-15.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

Seems random failure.

quietone’s picture

quietone commented

I'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions but the proposed resolution is out of date.

Leaving at RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: 3303548-15.patch, failed testing. View results

shweta__sharma’s picture

shweta__sharma commented
Status: Needs work » Reviewed & tested by the community

Seems random failure.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented

  • nod_ committed f0ef38ff on 11.x 
    Issue #3303548 by Aditya4478, Stanzin, Gauravvvv: Refactor Claro's...

  • nod_ committed 59050575 on 10.2.x 
    Issue #3303548 by Aditya4478, Stanzin, Gauravvvv: Refactor Claro's...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Fixed

Committed f0ef38f and pushed to 11.x. Thanks!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

There was a small regression to the fieldset styles. I opened separate issue for addressing that: #3396738: Regression in fieldset legend positioning. 😊

Status: Fixed » Closed (fixed)

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