-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Border controls: add base styles #34131
Border controls: add base styles #34131
Conversation
Size Change: +155 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
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.
I went through and tested this:
✅ Verify that border style is no longer a default control, Check that the Border controls appear in the ToolsPanel, and that border style is not displayed in the panel by default.
✅ Create a new post and insert a Group block. Add a non-zero border width. You should immediately see a solid border of the appropriate width reflected in the editor, set to whatever your currentColor is
I saw a solid border with currentColor.
✅ Save and make sure you see the solid border on the frontend
The border appeared correctly in the frontend.
❓ You should be able to change the style and color using the controls. These should also reflect on the frontend
I could not change style since this no longer appears, however color, width, and radius changes all worked and were reflected in the front end.
✅ Insert a new Group block. If you haven't set a width yet, you should not see a border.
I did not see a border.
✅ Adjust the width in the Border panel. You should immediately see a visible border, and the theme.json styles should override the base styles (so here we'd see a solid gray border)
I saw a solid grey border.
✅ Verify the appearance on the frontend
✅ Verify that changes made through the Border panel override both the theme and the base styles
✅ Verify global styles override the base styles
I changed the global styles to a dashed style, with pink color, and 5px width. All group blocks updated their settings where I had not previously set them at the block level.
@apeatling Sorry, just updated the testing instructions -- since I removed border style as a default control, you won't see it in the panel immediately. You should still be able to add it from the ToolsPanel menu and make changes, though. Does this work for you, or are you not able to see the border style in the menu at all? |
Thanks, I misunderstood. I've added this from the menu and border style is working correctly. 👍 |
d6e8a5d
to
f8dc90f
Compare
f963a47
to
f49ff27
Compare
f8dc90f
to
ab905e1
Compare
ab905e1
to
2e8cdcc
Compare
f49ff27
to
73f72f3
Compare
I've rebased this, fixed a minor issue with the selector for the frontend styles, and retested everything. This is now looking good again for me when tested on top of #33743. |
Closing, this was resolved in #33743 |
Requires #33743, #34061
Description
As we convert the border supports to use the new Tools Panel (#33743), it's necessary to think about what controls should be enabled as defaults for each block (#34061).
This PR is based on that work, and seeks to set some base styles for border in order to prevent needing to enable the border style as a default control. Without base styles, the user needs to actually select a style in order to see changes to other border attributes (color/width).
The approach taken here is to default to
border: 0 solid currentColor;
. We use the:where
CSS selector to ensure 0 specificity, so that any values set by theme.json or global styles (or anywhere else for that matter) will override them correctly.This should be safe to apply to all blocks, since the default width of
0
will prevent it from adding in visible borders where we don’t explicitly specify them in the theme/global styles etc. We’re also able to safely hide the border style control in the Border panel now.This leaves us with one problem — you still need to set a width before setting a color or style. In a further iteration we could attempt to set, say, a 1px default width when a color is selected; to avoid accidentally overriding theme/global styles we’d first need to explore accessing the merged styles from the editor. In the meantime, this solution allows us to hide the border style controls, prevents any overriding of parent styles, and the user is more likely to get immediate visual feedback when editing border controls than they were before.
Testing
Make sure you have #33743 and #34061!
Ensure that the border supports are switched on in your
theme.json
:Verify that border style is no longer a default control
For each of the blocks that previously had border style enabled by default:
Check that the Border controls appear in the ToolsPanel, and that border style is not displayed in the panel by default.
Verify the base styles are set
currentColor
is...
icon in order to make changes. These should also reflect on the frontendVerify theme.json overrides base styles
Verify global styles override the base styles
Repeat the above tests by setting some global styles for the Group block.
currentColor
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).