-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add color tools to time to read block #49496
Conversation
This issue actually highlights a bigger problem with the block. Any global style applied to the paragraph block type will also be applied to this block. This problem would be side-stepped by using the Other alternatives might be altering the selectors used however that might require changes to the paragraph block selectors which could carry significant risk. It's a shame that block's selector is so broad. What downsides do you see to using |
I personally think it'd be ok, and it would match what other similar blocks do. I'll push a commit that does that. |
Sounds like a plan. Is this block still experimental or has it been in a release that people could have used? If so, we'll need a deprecation as well. |
It's completely dynamic, so I don't think a deprecation will be needed. I've made the change in 588d465. |
Flaky tests detected in 7fca9e4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4571283699
|
Size Change: +8 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
So it is. I should have looked 🤦 Looks like we'll still need some tests to be updated to expect the new markup. Other than that it tests as advertised for me. As a separate issue, the global styles padding on the Paragraph block doesn't get applied on the frontend if you select a background color on the block. |
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.
What downsides do you see to using div for this block, less semantic HTML?
I personally think it'd be ok, and it would match what other similar blocks do. I'll push a commit that does that.
I agree with this as well. In the current block system, it may be best to use the div
element to unify with other blocks that display post meta.
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.
LGTM 👍
Is this block already released? I found this by chance and can't see it neither in 6.2 or 6.3 beta, using block theme, neither on post editor or site editor. |
This block is treated as experimental on the Gutenberg plugin and is not included with WordPress 6.3. Here is the core ticket to add this function: https://core.trac.wordpress.org/ticket/57987 |
What?
Adds color tools to the time to read block
Why?
This brings parity with other similar post blocks.
How?
Adds the color support option.
The only contentious aspect to this is that when applying a background, padding is applied via the
p.has-background
class. This is an inconsistency with other blocks like post author name and post date.Some options:
<div>
instead of a<p>
in the time to read blockTesting Instructions
Screenshots or screencast