Problem/Motivation

Unexpected duplicate selector ".dropbutton-wrapper.open .dropbutton__toggle::before" and double "margin-top" with different values, first used at line 234.

Line: 234
  .dropbutton-wrapper.open .dropbutton__toggle::before {
    margin-top: calc(0.5625rem / (2 * 1.41429));
    transform: translate(50%, -50%) rotate(315deg);
  }
...
Line: 251
  .dropbutton-wrapper.open .dropbutton__toggle::before {
    margin-top: calc(0.4375rem / (2 * 1.41429));
  }

Steps to reproduce

File: core/themes/claro/css/components/dropbutton.css

Proposed resolution

Line: 234
  .dropbutton-wrapper.open .dropbutton__toggle::before {
    margin-top: calc(0.4375rem / (2 * 1.41429));
    transform: translate(50%, -50%) rotate(315deg);
  }

Remaining tasks

no

User interface changes

no

API changes

no

Data model changes

no

Release notes snippet

Issue fork drupal-3376458

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:

Comments

fnalb2 created an issue. See original summary.

cilefen’s picture

Issue tags: -CSS classes +Novice
_utsavsharma’s picture

Status: Active » Needs review
StatusFileSize
new1.68 KB
new1.68 KB

Patch fixing the issue.
Please review.

longwave’s picture

How are you finding these duplicate selectors? Is there an Stylelint rule or similar we can enable to detect these across core?

Also we usually fix bugs by type of bug, not file by file - please read https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... before opening any more issues along these lines.

smustgrave’s picture

Title: dropbutton.css: Unexpected duplicate selector ".dropbutton-wrapper.open .dropbutton__toggle::before" » Update styelint to check for duplicate selectors
Status: Needs review » Needs work
Issue tags: -Novice +Needs Review Queue Initiative

Dubbing this one the main ticket and closed the others.

As @longwave mentioned this should be fixed with a stylelint rule if possible.

Brought up in #needs-review-queue-initative with @longwave and @lauriii

Each change will have to be manually tested for regression. Recommendation is to find more pressing issues though.

kostyashupenko’s picture

Status: Needs work » Needs review
StatusFileSize
new76.15 KB

Big and deep review required.

Added 'cascade-layers': false feature in PostcssPresetEnv in core/scripts/css/compile.js. Reason - !important CSS rule without chis change always compiling into separate selector (which brings duplicates of selectors). However with this setting or without - i didn't meet any changes on build task, so it's a rudiment for us.

All selectors were replaced carefully

longwave’s picture

Title: Update styelint to check for duplicate selectors » Update Stylelint to check for duplicate selectors
fnalb2’s picture

@longwave: I used SonarQube from https://www.sonarsource.com/

smustgrave’s picture

Issue tags: +Needs manual testing

Will need heavy manual testing I imagine.

smustgrave’s picture

Status: Needs review » Needs work

Patch no longer applies to 11.x

mrinalini9’s picture

Status: Needs work » Needs review
StatusFileSize
new75.47 KB

Rerolled patch #7, please review it.

Thanks!

smustgrave’s picture

Status: Needs review » Needs work

Can you provide a diff please also CC failure.

gauravvvv’s picture

Status: Needs work » Needs review
StatusFileSize
new78.2 KB
new3.18 KB

Fixed coding standard, attached interdiff for same.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new85 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

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

djsagar’s picture

Status: Needs work » Needs review
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new20.78 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

djsagar’s picture

Status: Needs work » Needs review
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new20.31 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

djsagar’s picture

Status: Needs work » Needs review
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

gauravvvv’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work

MR is unmergable.

gauravvvv’s picture

Status: Needs work » Needs review

Rebased with 11.x

smustgrave changed the visibility of the branch 11.x to hidden.

smustgrave changed the visibility of the branch djsagar-3aebe704-patch-6232 to hidden.

smustgrave changed the visibility of the branch djsagar-be735a74-patch-4956 to hidden.

smustgrave’s picture

Status: Needs review » Needs work

Left some comments if they can be answered. Reviewed maybe 3/4s of the MR.

gauravvvv’s picture

Status: Needs work » Needs review

I have addressed all feedbacks, please review

smustgrave’s picture

Status: Needs review » Needs work

Still appears to be an open thread.

Also needs merge conflicts resolved.

gauravvvv’s picture

Status: Needs work » Needs review

Addressed #35, please review

smustgrave’s picture

Status: Needs review » Needs work

1 more unanswered comment.

gauravvvv’s picture

Status: Needs work » Needs review

Addressed the thread @smustgrave. Setting the status as needs review

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Believe duplicates have been addressed.

nod_’s picture

Status: Reviewed & tested by the community » Needs work

couple of comments

gauravvvv’s picture

Status: Needs work » Needs review

Addressed #40

nod_’s picture

didn't review everything, what i saw looked much better, thanks :)

smustgrave’s picture

Status: Needs review » Needs work

Didn't review, MR has failures and some open threads.

gauravvvv’s picture

I updated the nesting, which was asked to be reverted in #40, just to fix these CSS linting issues. The lines showing the failure are not changed in the MR (reverted them).

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.