-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Link Control labels to use gray-900 #55867
Conversation
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @joanrodas! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
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.
I tested this and looked at all the labels I could find to check they were using $gray-900;
(aka #1e1e1e
) and they were.
Thank you for the PR 👏
@ciampo Is this something we can fix from the modal itself? This is the case throughout the editor where $gray-900 should be used as the default text color in modals. |
Bear in mind that LinkUI does not use |
I'll write up a separate issue. 😅 |
I don't think that Therefore, more than in the specific component, this is something that we should address holistically at the package level. We could add CSS The "drawback" of this approach is that we would stop relying on the CSS cascade — meaning that, if a consumer of the components package wanted to use a different text color for any reason, they would have to override the CSS color for every component used (instead of declaring it once and forget it). |
.components-base-control__label { | ||
color: $gray-900; | ||
} |
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.
Adding style overrides is usually not the best way to fix these issues:
- If
BaseControl
's default label color is wrong, maybe the fix should happen directly inBaseControl
(see the component in isolation here) - Otherwise, if
BaseControl
's default label color is correct, but the color is wrong specifically forLinkControl
, then we should find which other CSS rule is changing the color, and work on a fix at that level - We shouldn't be using internal
.components-
classnames, since those are meant to be implementation details of those components.
Any chance we can try to solve this issue in an alternative way?
What?
Update Link Control labels to use correct gray color ($gray-900)
Fixes #54589
PR in WordCamp Madrid 2023 Contributor Day with @Albert-Sirvelia @AlexBrea
Why?
Labels in Link Control are not using the correct gray color ($gray-900).
How?
Modifying the style.scss of the Link Control component and changing the color of the labels
Testing Instructions
Screenshots or screencast