-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Dimensions Panel: Add new ToolsPanel component and update spacing supports #32392
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
795fe99
Add utils to spacing supports to check or reset values
aaronrobertshaw dfb1403
Draft block support panel
aaronrobertshaw 0cb9f82
Update spacing hook to use new block support panel
aaronrobertshaw 1b23fad
Update block support panel to handle false children
aaronrobertshaw 2487805
Conditionally display spacing block support controls
aaronrobertshaw 696070f
Draft README for BlockSupportPanel
aaronrobertshaw 03c87c4
Make block supports panel handle filtered children
aaronrobertshaw 5fa856f
Add initial tests for block supports panel
aaronrobertshaw 7fb7e8a
Add story for block support panel
aaronrobertshaw 62d7895
Rename spacing support to dimensions
aaronrobertshaw 5f7b39f
Rename spacing support to dimensions in FSE
aaronrobertshaw db0c413
Update GlobalStyles dimensions panel
aaronrobertshaw dd015b0
Add means to handle default controls in block support panel
aaronrobertshaw 659a79e
Change default controls display in block supports panel
aaronrobertshaw 213d250
Make default controls still show after reset all
aaronrobertshaw a96dc35
Change back to default controls always displaying
aaronrobertshaw 481ea39
Tweak panel to make it a little more generic
aaronrobertshaw 0d75ba7
Simplify progressive disclosure panel state
aaronrobertshaw ffedb61
Add progressive disclosure panel item component
aaronrobertshaw ae4a3b2
Update dimensions global style sidebar panel
aaronrobertshaw e7a89ea
Update docs and comments after restructure
aaronrobertshaw 07420bc
Update panel story with new item component
aaronrobertshaw 9ec400f
Update tests to handle new panel item component
aaronrobertshaw a4d96b0
Clean new style objects after resetting spacing values
aaronrobertshaw 17bd1b6
Disable default control menu items when they have no value
aaronrobertshaw 4e33e6c
Add test for disabling default control menu items
aaronrobertshaw 3609b5a
Add cleanEmptyObject to dimensions resetAll
aaronrobertshaw 5746e25
Switch panel to grid layout for future multi-column layout
aaronrobertshaw c8d96ee
Tweak defaults for controls spacing in progressive disclosure panel
aaronrobertshaw 7c22312
Fix Global Styles sidebar panel styling
aaronrobertshaw dbaa31c
Handle cases where panel item's inner element returns null
aaronrobertshaw 8f196bc
Correct component name in readme
aaronrobertshaw 76a2b8d
Update readme with experimental alert and improved prop format
aaronrobertshaw 224c0ea
Fix menuItem child filtering using Children util.
aaronrobertshaw 8343c9b
Accurately name the ProgressiveDisclosurePanelItem in tests
aaronrobertshaw 6068bec
Initial pass at restructuring component
aaronrobertshaw 48388f5
Rename title component and props to header
aaronrobertshaw 92bcf52
Change progressive panel items to register themselves
aaronrobertshaw a624202
Change menu item selection state to enum
aaronrobertshaw eaad38e
Improve tests and rename to ToolsPanel
aaronrobertshaw 8a7df11
Refactor panel context to be consistent with other components
aaronrobertshaw ccee3ac
Extract menu states to utils
aaronrobertshaw f2851b6
Fix caching of block attributes via callbacks stored in ToolsPanel state
aaronrobertshaw 3d7e73c
Move execution of panel item callbacks into the item on menu toggle
aaronrobertshaw ed518fd
Convert to styled components and connect to context system
aaronrobertshaw f9411d1
Limit component.js files to presentation
aaronrobertshaw 8218467
Switch approach back to being able to hide default controls
aaronrobertshaw 70d7275
Tweak storybook entry for ToolsPanel
aaronrobertshaw d7bb353
Highlight experimental nature of component in story title
aaronrobertshaw c18dcda
Fix typo
aaronrobertshaw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Handle cases where panel item's inner element returns null
- Loading branch information
commit dbaa31c8fc583dcc77ac0da8a70d4a8cda2f8fec
There are no files selected for viewing
This file contains hidden or 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
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.
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.
Following the case described in the testing instructions:


Padding can be toggle on and toggled off without anything visible besides the check appearing. I guess when padding has no value and given that it is a default clicking on padding should be disabled and the check should not appear.
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 agree with you that this is confusing. It was raised previously, and again when the behaviour was reverted to an earlier state when trying to nail down how the panel should behave.
I'd still like to see us come up with a better user experience here. An earlier suggestion was to disable a default control's menu item if it had been reset. It would be re-enabled once the default control's value was adjusted/set again. Another option could be to have multiple states for each menu item. Something like:
Open to suggestions.
@jasmussen Do you have any thoughts now some time has passed since we last looked at this?
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 still think we should look for the simplest possible solution for the first version of this and then get a better feel in practice for how much of a problem this is.
Could we just remove the padding control if it's empty and you uncheck it? De-select and reselect the block and it's back, still empty. That's sort of what the prototype does, even if it isn't explicitly explored.
Since I've sent you on so many wild chases on this one, I'd appreciate @mtias thoughts, as I know he worked on the initial prototype also.
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.
We did explore having default controls be removed when deselected. Essentially, they were only displayed by default on the initial selection or re-selection of the block. This made their behaviour more consistent with other controls and did simplify the code as well.
Whatever we decide here will be easy enough to work into the other updates and great suggestions Jorge has given.