Closed (fixed)
Project:
Drupal core
Version:
9.3.x-dev
Component:
Claro theme
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
27 Apr 2022 at 00:17 UTC
Updated:
14 May 2022 at 02:49 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
larowlanComment #3
larowlanComment #5
richardrobinson commentedI'm a novice. I am at DrupalCon. I'm working with a mentor. I will work on this for the next hour.
Slack thread: https://drupal.slack.com/archives/C1BMUQ9U6/p1651098488766439
Comment #6
w01f commentedI'm a novice - I'll work on this issue for the next hour with the mentor contribution group at Portland Drupalcon.
Comment #7
mcolebank commentedI am a novice at Drupalcon working on this issue in a mentor contribution group for the next hour.
Comment #8
joshmillerI'm a mentor. We are at Portland Drupalcon 2022. I will be working on this issue for the next hour.
Comment #9
apocalypticjakeYo! Novice here at Portland DrupalCon 2022. I'll be working on this issue the next hour as part of a mentored contribution group.
Comment #10
richardrobinson commentedIf the color-whitesmoke variable doesn't exist, should we create it or can we just use an existing one? White might work.
Comment #11
apocalypticjakeComment #12
richardrobinson commentedFound out that ckeditor uses standard "whitesmoke" CSS color. Currently looking through the admin theme on DrupalPod to reproduce the issue/find where it's being used. Views uses modals/dialogs.
Comment #13
markie commentedFWIW: Whitesmoke is a defined color of #F5F5F5;
https://www.canva.com/colors/color-meanings/whitesmoke/
https://www.color-hex.com/color/f5f5f5/
Comment #14
apocalypticjakeAdded Tag: GiftofOpenSource tag to issue
Comment #15
richardrobinson commentedCommitted to branch
Comment #17
richardrobinson commentedhttps://www.drupal.org/project/drupal/issues/3154539
It turns out that they never intended on using the default "whitesmoke" CSS color. They did have a variable named --color-white and it was changed to --color-gray-50. I'm going to make another commit using that.
Comment #18
joshmillerHi I'm working on this issue in Mentored Contribution at Drupalcon Portland for the next hour.
Comment #19
pilot3 commentedHi, I'm working on this issue in Mentored Contribution at DrupalCon Portland for the next hour.
Comment #20
saki007sterHi, I'm working on this issue in Mentored Contribution at DrupalCon Portland for the next hour.
Comment #21
apocalypticjakeI'm assisting with this issue for the next 15-30mins in Mentored Contribution at DrupalCon Portland.
Comment #22
sjothivelu commentedHi, I am assisting on this issue in Mentored Contribution at DrupalCon Portland.
Comment #23
sjothivelu commentedHi, I am volunteering in DrupalCon Portland 2022
Comment #24
mcolebank commentedVerified that richardrobinson fix is correct. Tested and verified that the variable is correct, tested that description text shows in Layout Builder custom block. Portland Drupalcon mentored contribution group thinks this is ready.
Comment #25
mradcliffeThank you all for working on the issue.
It would be helpful to add Before/After screenshots. I added the Needs manual testing tag for this. Once those images are uploaded and embedded into the issue, please remove the tag and set back to RTBC.
Comment #26
bnjmnmWhitesmoke was one of the variables converted to grayscale naming in this issue #3154539: Implement new Gray scale on Claro this was a whitesmoke use that should have been changed there but was apparently missed. Use that as a reference to confirm it's being changed to the right equivalent variable.
Comment #27
larowlanYep, looks good - thanks for the link
Comment #28
saki007sterBefore - https://www.drupal.org/files/issues/2022-04-29/3277274_27_before.png
After - https://www.drupal.org/files/issues/2022-04-29/3277274_27_after.png
Comment #29
larowlanCrediting everyone who worked on this at the mentored sprint - thanks all
Comment #30
saki007sterGrepped all the other color variables to make sure there is no color variables that is missed and causing errors
Comment #31
sjothivelu commentedHi, Thank you for finally approving my account. I have been contributing today in mentored DrupalCon Portland 2022. Hopefully, we get commit this issue!
Comment #32
larowlanActually @markie was right here
For Drupal 9.4 (where we support IE11) we remove the variables - but for Drupal 10, we keep them (because we don't support IE11)
So we need a branch for 10.0.x here too.
It will basically be the same code as the 9.4.x code, but we'll need to run the yarn script to rebuild the .css file
The pcss file will be the same.
Comment #33
larowlanComment #34
richardrobinson commented(╯°□°)╯︵ ┻━┻
Working on this for the next hour or so. Going to regenerate the file on the 10.0.x branch.
Created new branch (3277274-10.0.x) off 10.0.x.
Comment #36
richardrobinson commentedChanges pushed to branch for 10.0.x
Comment #37
richardrobinson commentedComment #41
larowlanThanks @richardrobinson, ordinarily we require someone other than the person who wrote the patch/MR to RTBC an issue, but since this was just a minor re-roll for a D10 version, I think it's ok.
Committed to 10.0.x and 9.5.x. Backported the 9.5.x patch to 9.4.x and 9.3.x
Thanks everyone, congratulations to those for whom this is their first core commit, hope to see you in the issue queue.
🎉