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
Comments
Comment #2
fnalb2 commentedComment #3
cilefen commentedComment #4
_utsavsharma commentedPatch fixing the issue.
Please review.
Comment #5
longwaveHow 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.
Comment #6
smustgrave commentedDubbing 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.
Comment #7
kostyashupenkoBig and deep review required.
Added
'cascade-layers': falsefeature in PostcssPresetEnv incore/scripts/css/compile.js. Reason -!importantCSS 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 onbuildtask, so it's a rudiment for us.All selectors were replaced carefully
Comment #8
longwaveComment #9
fnalb2 commented@longwave: I used SonarQube from https://www.sonarsource.com/
Comment #10
smustgrave commentedWill need heavy manual testing I imagine.
Comment #11
smustgrave commentedPatch no longer applies to 11.x
Comment #12
mrinalini9 commentedRerolled patch #7, please review it.
Thanks!
Comment #13
smustgrave commentedCan you provide a diff please also CC failure.
Comment #14
gauravvvv commentedFixed coding standard, attached interdiff for same.
Comment #15
needs-review-queue-bot commentedThe 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.
Comment #18
djsagar commentedComment #19
needs-review-queue-bot commentedThe 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.
Comment #21
djsagar commentedComment #22
needs-review-queue-bot commentedThe 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.
Comment #24
djsagar commentedComment #25
needs-review-queue-bot commentedThe 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.
Comment #27
gauravvvv commentedComment #28
smustgrave commentedMR is unmergable.
Comment #29
gauravvvv commentedRebased with 11.x
Comment #33
smustgrave commentedLeft some comments if they can be answered. Reviewed maybe 3/4s of the MR.
Comment #34
gauravvvv commentedI have addressed all feedbacks, please review
Comment #35
smustgrave commentedStill appears to be an open thread.
Also needs merge conflicts resolved.
Comment #36
gauravvvv commentedAddressed #35, please review
Comment #37
smustgrave commented1 more unanswered comment.
Comment #38
gauravvvv commentedAddressed the thread @smustgrave. Setting the status as needs review
Comment #39
smustgrave commentedBelieve duplicates have been addressed.
Comment #40
nod_couple of comments
Comment #41
gauravvvv commentedAddressed #40
Comment #42
nod_didn't review everything, what i saw looked much better, thanks :)
Comment #43
smustgrave commentedDidn't review, MR has failures and some open threads.
Comment #44
gauravvvv commentedI 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).