-
Notifications
You must be signed in to change notification settings - Fork 126
Tweak variations to remove explicit color references #619
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Preview changesYou can preview these changes by following the link below: I will update this comment with the latest preview links as you push more changes to this PR. Note The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions. |
Please show that all the color contrasts have been tested. |
I asked @beafialho to review, thanks. |
I noticed a few places where links seem to have specific settings and aren't contrasting:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, Rich! It's looking good.
I've been through the different variations, and here are my findings:
Evening
- Style 3 - link - Very low contrast (3.24:1)
- Style 4 - link - Very low contrast (1.07:1)
- Style 5 - link - Very low contrast (1.42:1)
Before | After |
---|---|
Noon
- Style 4 - Headings - Very low contrast (1:1)
- Style 5 - Headings - Very low contrast (1:1)
Before | After |
---|---|
@richtabor @beafialho I've pushed a commit with changes to fix the parity that we had since this PR, and that should also be fixing the contrast issues I've found. Rich, feel free to revert if you find any issues. Bea, if you can please give this another go and see if everything looks good on your end. Thanks 🙌 |
Thank you! I only noticed issues in Noon Section Style 4 (the one with a black background). I believe it must be because Noon has the Headings and Post Titles color defined as noon-contrast-issues.mp4 |
Thanks @beafialho - I just pushed a change that should fix that. |
Thank you @juanfra, the issues I mentioned in Noon are solved now! |
Can we also make sure the Quote block's colors are looking/working ideally? For more context. |
@beafialho I just pushed a change that should fix the quote block issue for all sections in all variations. Screen.Recording.2024-10-30.at.15.21.34.mov |
Thank you for your patience on this one @juanfra. This one is a tricky one to test because you need to have all the blocks listed out on the same page to get the view. I have noticed on my site that Post Date is still using a fixed color and Pullquote seems too as well. 🫠 I'm not sure about the Code block, but it seems to have a fixed color as well: code.mp4Apologies for the back and forth but hopefully after these are addressed this is ready to merge. |
@beafialho I pushed a fix for the pullquote in the different section styles. Could you please clarify the expected behavior for the code and post-date blocks? Where are you taking those screencasts from? |
The Code block should always have a slightly lighter or darker color than the background (usually Tertiary). And the Post Date should just have an accessible color. |
I have seen the code block issue: I let it slide because the text is always visible on all variations, so it is still accessible. |
Yes, the code block isn't necessarily a bug because it is readable. |
Ok, I pushed changes for the date block, and left the code block as is. For the date block on different sections, I chose the current color at 85%. Code to test
Screen.Recording.2024-10-31.at.11.22.58.mov |
Thank you! It's working now, the slight opacity gives it exactly the intended look 🚀 If it's easy, I wonder if we could use the same approach for the code block background? If not, we can just move on. |
@beafialho That's not possible on backgrounds, as we're using |
After discussing this with Carolina and Bea, we're merging it. |
Description
Closes #620.
Using
currentColor
allows for colors to be manipulated manually within the Inspector. Otherwise users are required to modify text and link colors, which feels broken as the text and link colors are the same value to start.Testing Instructions
Test each color and full style variation across each section style.