File block: Add typography block support #55693
Conversation
carolinan
added
[Type] Enhancement
|
Size Change: +21 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
@WordPress/native-mobile Hi, I am adding typography support to the file block, and I need help with a failing test. The mobile unit tests are showing that the markup of the block has changed and I don't know how to solve it. |
Hey @carolinan 👋, I've checked the failing tests and noticed that the problem is caused by the hook diff --git forkSrcPrefix/packages/block-editor/src/hooks/index.native.js forkDstPrefix/packages/block-editor/src/hooks/index.native.js
index 42bda25bfe1ccf47a8c2909c2708f69be0934cef..137231942470e1936c66d562149e7145d63c2bee 100644
--- forkSrcPrefix/packages/block-editor/src/hooks/index.native.js
+++ forkDstPrefix/packages/block-editor/src/hooks/index.native.js
@@ -16,3 +16,4 @@ export { getColorClassesAndStyles, useColorProps } from './use-color-props';
export { getSpacingClassesAndStyles } from './use-spacing-props';
export { useCachedTruthy } from './use-cached-truthy';
export { useEditorWrapperStyles } from './use-editor-wrapper-styles';
+export { getTypographyClassesAndStyles } from './use-typography-props';As a side note, I'd like to note that typography attributes are not supported yet in the native mobile version for the File block, so this feature won't be available there until we implement it. |
|
Flaky tests detected in 48b6740. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9204821851
|
carolinan
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
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. |
|
@tellthemachines @aaronrobertshaw When time permits, I'd like to ask what we can do to support font size settings on links, when the link has a font size set in theme.json:
It is not so easy to discover what the problem is if you are not very familiar with theme.json. 🤔 |
| &:not(.wp-element-button) { | ||
| font-size: 0.8em; | ||
| } | ||
|
|
There was a problem hiding this comment.
Won't this break back compat for themes that may be relying on this CSS?
There was a problem hiding this comment.
Yes, but the style was not originally there: the addition of the style was a back compat break itself.
In 2022, it was moved from the button to the link text. That is the origin of the open issue.
| @@ -32,6 +33,7 @@ export default function save( { attributes } ) { | |||
| fileName.toString(); | |||
|
|
|||
| const hasFilename = ! RichText.isEmpty( fileName ); | |||
| const typographyProps = getTypographyClassesAndStyles( attributes ); | |||
There was a problem hiding this comment.
Is there any particular reason for adding these manually. Isn't this something that the hook adds automatically?
There was a problem hiding this comment.
It is because I needed the typography class names and inline styles to be on both the wrapper, and the download button.
If I only use the save props on the wrapper, or if I try to use an experimental selector for the typography in block.json, then the button styles override the user settings from the controls in the block editor.
This isn't straightforward 😅 because the element font size is set on the link or button tag, whereas the block font size is set on the block itself. The Search block solves this problem by adding either the preset classname or the custom style to its inner tags, but I'm not sure that's a good solution for link elements given there could be any number of them within a block. This is also not a new issue; it occurs in blocks like Paragraph if you style its link element font size. One (unrelated) thing I'm noticing with the File block is that color support could also be improved; currently it's possible to set the link color and the background of the whole block, but the button is unstylable: Ultimately it would be good to have more fine-grained UI controls to solve these issues. |
|
Yes there is an open issue for using an inner button block. #57314 It needs a decision. |
That's why I am asking ;) |
There was a problem hiding this comment.
Thanks for tackling this @carolinan, it's another tricky one!
I've given it a quick test and in addition to the issues already highlighted and being discussed there are a couple around the text decoration support. It has a habit of throwing up edge cases given the way browsers render it.
While testing I noticed:
- Text decoration doesn't apply correctly to the link in the editor
- The link gets both the wrapper's text decoration as well as the link's on the frontend
- Text decoration on the link can't be removed via the block's text decoration support
I'd like to ask what we can do to support font size settings on links, when the link has a font size set in theme.json
As @tellthemachines noted, adding classnames and styles to inner anchor tags may not be viable.
An alternative, that isn't exactly clean itself, is to add CSS that resets the appropriate style to inherit when it's set on the wrapper. Thus overwriting the element's style.
For example, something like below results in the link updating its font size accordingly.
.wp-block-file {
&[style*="font-size"] a:not(.wp-block-file__button),
&[class*="-font-size"] a:not(.wp-block-file__button) {
font-size: inherit;
}
}This can get pretty bloated when juggling lots of styles like the typography supports.
Another option might be to apply the typography styles consistently. By that I mean, apply them directly to both the link and the button manually, skipping the typography styles serialization on the outer wrapper.
Combining that with leveraging the block selectors API could allow better styling of that block via theme.json without the need to resort to styling its inner elements.
"selectors": {
"typography": ".wp-block-file a, .wp-block-file .wp-block-file__button"
}
The inner element styles could be still be styled so there could be the usual conflicts between block styles and element styles still.
I hope that helps a little and might give something to explore further 🤞
|
Since this behaviour of the text decoration option has been accepted for other blocks, I don't understand why it would need to be solved for this block only, in this PR? |
This wouldn't be the first block with inner links it was solved for. The navigation block is an example, see #36345. |
|
The font size option does change the size of both the link with the file name and the link that is the download button, but does not override elements.link. |
|
I'll look close at the navigation block class name solution, but it also has the default link underline removed which is a bad practice for links. |
|
I think the has-text-decoration classes should always be applied when option is enabled, and that the style should be assigned to the class, and not be inline on the block. These inconsistencies makes it more difficult to design for WordPress. |
Is there anything preventing this change? |
I think it is worth exploring 👍 Most other instances where |
I don't understand what this means. :) |
Apologies, I only meant that as browsers paint text decoration on elements differently than most other effects, I'm not sure if the need to use |
|
I'm still hesitating about the best solution for this block. And I think that change would need to be be made first, because it will affect where the styles and class names are output: |
carolinan
added
the
[Status] Blocked
What?
This PR adds typography block support to the file block.
There is more than one way to approach this issue. I chose to follow the search form block, which also has both text and a button inside the block: This means that the typography setting applies to both the text and the button.
When the file block has no typography settings added by the user, the button uses the style of the button block.
Closes #43381
Why?
How?
The PR:
Testing Instructions
Classic theme
Please use a theme that does not already style the file block, because otherwise there may be CSS conflicts. These conflicts would need to be addressed in the individual theme.
Confirm that all settings still apply correctly, in the editor and on the front.
Block theme:
Please use a theme that does not already style the file block, or, temporary remove the styles from theme.json so that
both cases can be tested: with and without styles.
Without styling the file block in theme.json:
Confirm that all settings still apply correctly, in the editor and on the front.
Open theme.json and add a typography style to styles > blocks > core/file. For example:
Confirm that 25px is used as the default size for the text link.
Confirm that this style does not affect the button text size, because this text size which is inherited from the button styles has a higher specificity.
Now style the elements in core/file:
Confirm that by default, no text is 25px.
Confirm that by default, the link is 77px and button 33px.
With these settings applied, change the font size setting in the block editor.
The button text size changes, because the style is applied inline.
The font size of the linked text does not change, because the CSS from the link style in theme.json has a higher specificity: