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/clar... 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.

User interface changes

None. There should be no visual differences.

CommentFileSizeAuthor
#23 icon-link-active-after.png1011 KBakshayadhav
#23 icon-link-active-before.png1.01 MBakshayadhav
#23 icon-link-focus-after.png992 KBakshayadhav
#23 icon-link-focus-before.png967.89 KBakshayadhav
#23 icon-link-hover-after.png983.22 KBakshayadhav
#23 icon-link-hover-before.png976.58 KBakshayadhav
#23 icon-link-idle-after.png961.39 KBakshayadhav
#23 icon-link-idle-before.png948.4 KBakshayadhav
#18 icon-link-after.png13.75 KBrupeshghar
#18 icon-link-before.png13.91 KBrupeshghar
#13 3305147-13.patch909 bytesgauravvvv
#5 3305147-version9.5.x.patch2.43 KBaditya4478
#3 3305147-version10.0.x.patch2.77 KBaditya4478
#2 3305147-version9.5.x.patch2.43 KBaditya4478

Issue fork drupal-3305147

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Aditya4478 created an issue. See original summary.

aditya4478’s picture

aditya4478 commented
Status: Active » Needs review
StatusFileSize
new 3305147-version9.5.x.patch2.43 KB
aditya4478’s picture

aditya4478 commented
Version: 9.5.x-dev » 10.0.x-dev
StatusFileSize
new 3305147-version10.0.x.patch2.77 KB

The last submitted patch, 2: 3305147-version9.5.x.patch, failed testing. View results

aditya4478’s picture

aditya4478 commented
Version: 10.0.x-dev » 9.5.x-dev
StatusFileSize
new 3305147-version9.5.x.patch2.43 KB

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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
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
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

Was discussed in slack #frontend channel slightly with @ckrina and @quietone
Moving to postponed as there are still some decisions to be made about how to go about these changes and what's needed in the follow up.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

aditya4478’s picture

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

gauravvvv commented
Status: Needs work » Needs review
StatusFileSize
new 3305147-13.patch909 bytes

I have refactored the icon-link stylesheet. please verify

smustgrave’s picture

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

Before/after screenshots should be added to the issue summary.

sonam_sharma made their first commit to this issue’s fork.

rupeshghar’s picture

rupeshghar commented
Assigned: Unassigned » rupeshghar

Working on this issue as a part of Claro Contribution Day.

rupeshghar opened merge request !5824

rupeshghar’s picture

rupeshghar commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new icon-link-before.png13.91 KB
new icon-link-after.png13.75 KB

I have added nesting for icon links
Before

After

rupeshghar’s picture

rupeshghar commented
Assigned: rupeshghar » Unassigned
rupeshghar’s picture

rupeshghar commented
Issue tags: +ClaroContributionDay2023
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Does not appear css was compiled

Hiding patches for clarity.

shmy’s picture

shmy commented
Status: Needs work » Needs review

I've compiled the committed CSS and didn't got any modifications on the CSS file too, which makes sense as this is just different nesting.

akshayadhav’s picture

akshayadhav
he/him
Location Pune, India 🇮🇳
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new icon-link-idle-before.png948.4 KB
new icon-link-idle-after.png961.39 KB
new icon-link-hover-before.png976.58 KB
new icon-link-hover-after.png983.22 KB
new icon-link-focus-before.png967.89 KB
new icon-link-focus-after.png992 KB
new icon-link-active-before.png1.01 MB
new icon-link-active-after.png1011 KB

@smustgrave yes, there is no difference in the compiled CSS file, that's why it's not showing up in the git diff.
Looks good to me.

  • ckrina committed 7c4decb5 on 11.x 
    Issue #3305147 by Aditya4478, rupeshghar, Gauravvvv, AkshayAdhav, shmy:...
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Reviewed & tested by the community » Fixed

Committed 7c4decb and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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