Table block: Make figcaption styles consistent between editor and front end
#46172
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
mikachan
added
[Block] Table
|
Size Change: -1.85 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
jffng
reviewed
Nov 29, 2022
| @@ -18,6 +18,13 @@ | |||
| padding: 0.5em; | |||
| } | |||
|
|
|||
| // Match caption styles across front end and editor | |||
| figcaption { | |||
| @include caption-style-theme(); | |||
There was a problem hiding this comment.
It seems there is a convention for mixins with the suffix -theme to only use them in the theme styles — is it an issue to create an inconsistency here?
There was a problem hiding this comment.
Thanks both, that makes sense. I've removed the use of this mixin here and instead used specific styles. I went with:
figcaption {
color: inherit;
font-size: max(13px, 80%);
text-align: center;
}
The only difference here is that the font size is set to a minimum of 13px, as this is the same size that was set in the mixin, but then I've also allowed it to be 80% of the font size set by the theme. I'm not sure if this is too opinionated, but I thought it would allow for a more theme-related font size.
|
Thanks for working on this. I'm a bit split. Part of me thinks we should just remove the |
Hmm yeah, that fixes the consistency issue and keeps the theme styles more opinionated (which I think is their intention). Also, if we start to go in a direction where the opinionated styles are moved to style.scss / the default style, the theme.scss styles / I've removed the more opinionated styles from style.scss, and added the mixin back to theme.scss, so we can test that out. Thanks for the feedback! Just to clarify for anyone following along, this is what I'm now seeing with 6671cba:
|
|
Thanks for working on this. Should we investigate making |
Yeah, that makes sense. They're used in quite a few blocks, I think it would be great to style them via Global Styles. |
|
If you're willing to create a PR, i'm willing to review it :D |
mpkelly
pushed a commit
to mpkelly/gutenberg
that referenced
this pull request
Dec 7, 2022
…ront end (WordPress#46172) * Move figcaption styles to style.scss * Remove theme mixin from style.scss * Increase font size percentage * Remove figcaption styles from style, keep them in theme
|
We already have a Do you think we should break this out so we have a separate |
|
I think they should be the same. In that case, is it possible to migrate the styles from CSS to block.json |
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 makes the styling of the
figcaptionelement in the Table block consistent between the editor and the front end. Currently, these are styled slightly differently.Why?
Visual consistency between the editor and the front end provides a better user experience.
Also addresses a comment on trac here: https://core.trac.wordpress.org/ticket/56818#comment:16
How?
This moves the styling for the table
figcaptionelement to the block'sstyle.scssfile, rather than in theeditor.scssandtheme.scssfiles.Testing Instructions
Example block markup:
Screenshots or screencast
Before:
After:
cc. @WordPress/block-themers