Format library: add background color #34680
Conversation
ellatrix
added
[Type] Enhancement
|
Size Change: +151 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
mtias
added
the
[Feature] Design Tools
|
This is great. I'm not quite sure either if we should be using the same tool or separate ones. There are pros and cons for each. If we were to separate background, I think we should consider marking that one alone as |
I think it makes more sense together. We can mark text color without background as well, it can also carry semantic meaning in my opinion. |
ellatrix
force-pushed
the
try/inline-background-color
branch
from
f8a923a to
389f19b
Compare
ellatrix
force-pushed
the
try/inline-background-color
branch
from
8b15f85 to
4eab4e5
Compare
|
This is ready for review. |
There was a problem hiding this comment.
Thanks @ellatrix! This is very nice. Overall this tests well for me and the code makes sense. I did find some minor interaction oddities when setting dropcap, but it's not a blocker for this PR.
Let's maybe get a +1 review for interaction feedback
Here's the minor issue I saw with dropcap:
CleanShot.2021-09-17.at.10.36.01.mp4
| <Icon | ||
| icon={ textColorIcon } | ||
| style={ colorIndicatorStyle } | ||
| /> |
There was a problem hiding this comment.
Having the icon reflect color choices is pretty neat, though it is possible to create a bad combination where the icon is no longer legible:
There was a problem hiding this comment.
Right, it would be the same as the text though, making it an accurate preview. As long as the button label is readable, I find that ok.
Right the Regarding drop caps, I've always thought that maybe we should force a span around the first letter for drop caps and then style that instead of using the buggy |
Who would be good to ask? :) |
|
Maybe @jasmussen or @shaunandrews if they have time? If no one responds, it's probably fine to try landing and seeing what feedback folks had. |
|
In trunk, the same popover appears: In this branch, that popover just has tabs for text and bg color: The challenge is that we need to be able to both choose from theme palettes as well as choose custom colors. The primary challenge here is that the popover sort of just sits there, and it isn't clear what it's aligning to. Additionally, the text color icon isn't the best — perhaps it's time we tried the new color icon? This one: All that said, those are issues with the existing feature in trunk. The tab doesn't make any of this worse, and so this seems fine to land as a change that will make a lot of people happy. I'd love to come back to the interface in the future. |
It should align to the text selection right, similar to the link popover? |
We are currently using the |
|
It centers below the text selection, which is all good, so definitely land this one as is. I just can't help myself but dream of what it can become in the future, perhaps with a sliding block toolbar that lets it happen then and there. Mostly it's that the color popover is so disconnected from the dropdown that tripped me. But nothing to worry about for this PR! Something we can come back to in the future. |
I agree, it would be nice if rich text popovers are better integrated in the block toolbar. :) In the meantime, let's merge this and iterate. |
|
|
||
| const name = 'core/text-color'; | ||
| const title = __( 'Text color' ); | ||
| const title = __( 'Color' ); |
There was a problem hiding this comment.
Can we try adjusting the tool name from "Color" to "Highlight"? I think color alone is too confusing, especially with block level color in the sidebar.
ellatrix
force-pushed
the
try/inline-background-color
branch
from
4eab4e5 to
ae73b3b
Compare
|
Do we need to do any updates to how a site could disable these controls? Can background alone be disabled? Would the UI adapt? |
ellatrix
force-pushed
the
try/inline-background-color
branch
from
277f47a to
3c3a36b
Compare
|
@shaunandrews Ideally this should be built in the tabs component. Maybe it could also have a different styling option. I see the other form of tabs in the background color component, but I don't find it separately in the components package.
Let's iterate on this separately. Maybe we should also use tabs for color options in the inspector in the future. |
|
We need to consolidate all the usages of that pattern to the new design, we can follow up on the general color tools work there. |
|
Hey there @ellatrix 👋 I have a quick question, what happens for posts that already have some colors customization and they're opened when this change is live? If I open an old post with these changes I'm seeing this: And before it was: Is this expected? Thanks! |
|
Ah, the tags are not upgrading to mark properly. I'll take a look. |
format-library-text-color-button__indicatorを削除 関連 : WordPress/gutenberg#34680
URLPopoverからPopoverに変更 関連 : WordPress/gutenberg#34680
URLPopoverからPopoverに変更 関連 : WordPress/gutenberg#34680
|
I just updated to v5.9 and ended up here for a couple of reasons:
|
| @@ -94,7 +119,7 @@ function TextColorEdit( { | |||
| export const textColor = { | |||
| name, | |||
| title, | |||
| tagName: 'span', | |||
| tagName: 'mark', | |||
There was a problem hiding this comment.
Is there any reason why this was changed from a span- to mark-tag? I cant find any information in #34680 to it.
This looks like a big breaking change, also due the fact that some normalize css files might contain default styling like backgroundColor or color to those elements.
Hello, is there any way that I can get gradient option for richtext as shown in the figure? I am only getting the solid color option. |
Closes #20835
Description
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).