-
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
Fix: Remove color styles from typography variation controls - #61217 #61280
Conversation
This would fix the issue added in WordPress#61217, This wanted to remove the color combination in the typography preview shown in the styles editor page
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. |
@@ -94,6 +94,7 @@ function SidebarNavigationScreenGlobalStylesContent() { | |||
) } | |||
{ typographyVariations?.length && ( | |||
<TypographyVariations | |||
addGlobalStyles |
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.
Hello @richtabor,
I am passing this prop as true here, because I am thinking we should keep the color combination in the sidebar navigation global screen, Let me know if we need to remove the color from here as well, I will update the PR.
Thank You.
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 think we should remove it here.
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.
Sure, I will work on this PR and update it by removing the color from here as well.
So after update, both the global screen and side panel would not have the color effect in the typography. Am I correct?
Hence, I am guessing that, we should remove the props from all places, if we make it consistent.
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.
Yeah, I had a different idea for how to do it which I tried here: #61898
I'm not sure which option is better, what do you think?
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.
Yeah, looks like the good option. Just add the styles to iframe whatever we want using this prop.
I'm not sure about the approach here. Adding a new |
This was closed by #62578 |
What?
Why?
How?
addGlobalStyles
, if this is passed as false, it would not add the style to the iframe preview.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
GB.Issue-.61217.webm