Button: Update to use border support provided styles and classes #33017
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
aaronrobertshaw
added
[Type] Bug
|
Size Change: +18 B (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
7 tasks
|
Thanks for taking a look at this one @aaronrobertshaw! It's testing nicely according to the instructions, and it respects an existing (valid) Button with border radius (and ones with individual border radii):
It looks like one of the test fixtures is failing (for the deprecated radius on the button), and I can see in the editor if I manually copy + paste the content of
Do we need to update one of the deprecations, too? |
|
Thanks for testing @andrewserong 👍 I'm actually working through the deprecations now. |
|
@andrewserong The problem came down to the deprecation attempting to migrate an old border radius attribute to the This meant after the deprecation it was trying to save a number to a string attribute and so was ignored. Without the attribute saved, there was nothing to trigger the application of an inline style. The deprecations have been updated to fix this. Pasting the block code from your comment into the code editor and switching back to the visual editor does result in the block being correctly migrated to the latest version. The block fixtures have been regenerated to accomodate the border radius with unit in the resulting attributes. All the block fixture tests are now passing both locally and on this PR. Let me know if I've missed anything else. |
7 tasks
andrewserong
approved these changes
Jun 29, 2021
There was a problem hiding this comment.
This is testing nicely for me now, thanks for fixing up the deprecations @aaronrobertshaw! I just took another look at the useBorderProps function to refresh myself on how it works grabbing the styles from the block's attributes, and the logic looks good to me. I can't see any other issues 🙂
7 tasks
7 tasks
|
This PR introduced a regression with blocks which have |
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.
Related: #31585
Description
Updates the ad hoc use of the border radius block support on the button block. This block support isn't serialized for buttons so it can be applied to an inner element.
Previously, the button block only applied the shorthand border radius style. Now that the border radius support has been updated to support longhand border radius styles for each corner, this needed updating. This PR uses the
useBorderPropsutility to achieve this.How has this been tested?
Manually.
Screenshots
There should be no change to the UI from this PR. See related PR #31585 for new UI control to manage corner radii.
Types of changes
Bug fix.
Checklist:
*.native.jsfiles for terms that need renaming or removal).