Closed (fixed)
Project:
Drupal core
Version:
9.5.x-dev
Component:
Claro theme
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
9 Jun 2022 at 14:40 UTC
Updated:
13 Oct 2022 at 12:34 UTC
Jump to comment: Most recent, Most recent file

Comments
Comment #2
mherchelThis issue is for the Drupal Association's Google Summer of Code student. Please don't work on this unless you are him!
Comment #3
mherchelComment #4
aditya4478 commentedPatch Attached !! :)
Comment #5
mherchelYou're redeclaring the
.accordion__itemselector here. It's already the parent selector, so this is not needed.Comment #6
aditya4478 commentedChanged redeclared selectore
Comment #7
aditya4478 commentedComment #8
asishsajeev commented@Aditya4478 Patch applied successfully.
Comment #9
sasanikolic commentedMissing ";" at the end of the css definition.
Shouldn't this also be under .accordion? Like so: &__title { ... }
I also checked the css file locally and some curly brackets are missing. Let's try compiling once more and running the linter.
Also, let's make sure to post interdiffs between patches so we can see the change from patch 1 to patch 2. That's always very helpful to have.
Comment #10
sasanikolic commentedComment #11
pooja saraah commentedAddressed the comment #9
Attached interdiff patch
Comment #12
sasanikolic commented@pooja saraah thanks for your patch, but please be aware that this is a task for our GSoC student and we'd like him to contribute to this task.
Checking the patch above, this piece of code got lost and needs to be added.
And please disregard my comment regarding the
&__title { ... }change, that can't be done with PostCSS.Comment #13
sasanikolic commentedAlso, we need a separate patch for Drupal 10, which will contain different code.
Comment #14
aditya4478 commentedPatch for Drupal 9
Comment #15
sasanikolic commentedThe changes look good to me now.
We just need to make sure the patch applies correctly and please also provide another patch for Drupal 10.
Comment #16
aditya4478 commentedAttached the patch for Drupal 10
Comment #17
aditya4478 commentedAttached patch for Drupal 10.0.x (Ignore the previous one)
Comment #18
aditya4478 commentedSorry for the multiple Comments. Please ignore comment #16,#17
Consider this Patch for 10.0.x.
Comment #19
aditya4478 commentedComment #20
sasanikolic commentedHi @Aditya4478. I think your patches are failing because of bad encoding. Can you try to do the following and see if it helps? This did the trick for me and the patch was applied locally.
Also this might be relevant to you, since you're on a Windows machine:
Comment #21
aditya4478 commentedChecking Notepad method if it is works for me..
Comment #22
aditya4478 commentedPatch with VS Code changes
Comment #23
aditya4478 commentedComment #24
aditya4478 commentedComment #25
aditya4478 commentedComment #26
aditya4478 commentedComment #27
aditya4478 commentedComment #28
sasanikolic commentedThese changes look good to me except for the formatting and the semicolon missing, but that looks like a compilation issue.
Comment #32
lauriiiCommitted #27 3f6a0e9 and pushed to 10.1.x and cherry-picked to 10.0.x. Also committed #26 to 9.5.x. Thanks!
Leaving needs work for opening the follow-up.
Comment #33
mherchelFollowup opened at #3312586: Refactor Claro's accordion stylesheet to take advantage of component-level custom properties.