Comments Block: fixed issue with custom font sizes and links color #41627
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaggieCabrera
requested review from
michalczaplinski,
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
MaggieCabrera
added
[Block] Comment Template
|
Size Change: +2.12 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
cbravobernal
added
the
[Type] Bug
|
Do we have to create a deprecation for blocks that have the fontSize attribute here? The block is not crashing on the editor, as I think the block supports for fontSize is already adding it. But, if we check the deprecated files, all of them have a v1 without that attribute (those deprecations were created when we added that fontSize, that now we are reverting due to the issue) |
cbravobernal
requested review from
DAreRodz,
ockham and
gziolo
and removed request for
juanmaguitar and
ajitbohra
ockham
added
the
Backport to WP Minor Release
ockham
reviewed
Jun 13, 2022
Comment on lines
-21
to
-24
| }, | ||
| "fontSize": { | ||
| "type": "string", | ||
| "default": "small" |
There was a problem hiding this comment.
Don't we need to keep the attribute in order for the default template settings to work? 🤔 After all, this PR is setting the blocks' fontSize attributes (e.g. here), isn't it?
In order to achieve the desired effect, would it be enough to only remove the default here?
There was a problem hiding this comment.
I've tested this locally, and it seems to do the trick 🙂
There was a problem hiding this comment.
As discussed with @c4rl0sbr4v0 in Slack, I was mistaken here -- the attribute is added implicitly thanks to fontSize block-supports 😅
FWIW, I can repro (on |
cbravobernal
force-pushed
the
fix-font-size-comment-date
branch
from
282500e to
681428d
Compare
cbravobernal
changed the title
ockham
reviewed
Jun 14, 2022
ockham
reviewed
Jun 14, 2022
I think we can get away without a deprecation. However, note that block-supports adds a nested In addition, prior to this PR, we had explicit, "flat" LMK if this reasoning makes sense 😅 |
Seems legit 😄 |
|
Given that we're using flat attributes again, I'd like to briefly reconsider if this has any implications for needing a deprecation (or not) before we merge 🙂 |
Okay, I still think we don't need a deprecation. If someone changed the font size on or this (for a custom font size): That's the exact same way the font size attribute is serialized now. No need for a deprecation or migration 😄 |
ockham
approved these changes
Jun 16, 2022
There was a problem hiding this comment.
Thanks a lot @MaggieCabrera and @c4rl0sbr4v0!
My apologies, two of my review comments turned out to be mistaken -- thanks for bearing with me! 😅
Anyway, LGTM! 
adamziel
pushed a commit
that referenced
this pull request
Jun 21, 2022
…41627) Fix an issue where custom font sizes were not showing up properly on the frontend and after a reload they were disappearing from the editor. Also fix an issue where link color is not applied on site editor, both in Comment Author and Comment Date block. Co-authored-by: Carlos Bravo <[email protected]> Co-authored-by: Bernie Reiter <[email protected]>
|
I just cherry-picked this PR to the wp/6.0 branch to get it included in the next release: 2c2970e |
adamziel
removed
the
Backport to WP Minor Release
|
@MaggieCabrera this PR contains PHP changes that need a matching PR in the https://github.com/WordPress/wordpress-develop repository to be included in WordPress 6.0.1. Would you please create such a PR? CC @SergeyBiryukov |
@adamziel is WordPress/wordpress-develop#2852 ok? It's the first time I have to do this :D |
|
@MaggieCabrera Thank you! I just commented on the other PR, we're good :D |
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR fixes an issue where custom font sizes were not showing up properly on the frontend and after a reload they were disappearing from the editor.
It also fixes an issue where link color is not applied on site editor, both in Comment Author and Comment Date block.
Co-created with @c4rl0sbr4v0
Why?
So that the user can use custom font sizes in the comment author name, comment date, comment reply link and comment edit link blocks.
How?
The current implementation of the comments blocks was defining a small size attribute that was overriding any custom font sizes defined by the user or the theme. I removed that definition from the blocks and instead moved it to the template of the comments block since it's a style decision.
Testing Instructions
With TT2 active, insert the Comments block
Change the font sizes of the affected blocks to some custom value
Save and check the frontend, reload the editor and the font size selected should be the one you see.
Change the link color of Comments Date and Comments Author in the Site Editor.
Check that changes are applied in both editor and frontend.
Screenshots or screencast
Before:
After: