BoxControl: Allow configurable sides #30606
Merged
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] Enhancement
aaronrobertshaw
requested review from
ajitbohra and
chrisvanpatten
as code owners
|
Size Change: +42.7 kB (+3%) Total Size: 1.46 MB
ℹ️ View Unchanged
|
7 tasks
youknowriad
reviewed
Apr 8, 2021
This was referenced Apr 8, 2021
|
Thanks for the quick feedback @youknowriad! I can definitely look to switch to an array prop. I'll make some time tomorrow to make these updates and address any other feedback. Appreciate your assistance on this 👍 |
This was referenced Apr 8, 2021
youknowriad
reviewed
Apr 9, 2021
There was a problem hiding this comment.
This is looking good.
Maybe we can consider this
Thanks for extracting this. Potentially we could consider adding a storybook story with an example of that.
Also, some thing I've been thinking about: what happens when you pass 0 or 1 side:
- Should we ignore the prop when sides is an empty array? (make it work like undefined)
- Should we not render the "toggle" to switch between merged state and unmerged state when there's only one side?
They sound like some good improvements. I'll make it happen. |
|
The BoxControl will now treat an empty array the same as undefined. If there is only one side selected in the config, the BoxControl will not display the "unlink" button.
Would the story be acceptable as a separate PR? It's not something I've looked at before but it would be good to take some time and update it with examples of the new features. |
youknowriad
approved these changes
Apr 19, 2021
7 tasks
|
Storybook updates can be found in #31029 |
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.
Description
This allows for the
BoxControlcomponent to accept a newsidesprop allowing omission of specific sides. For example, with this you could now choose to only allow editing of top and bottom values.This was originally part of #29363 where it was used to manage top/bottom margins on the separator block. It has been extracted into this PR for finer granularity.
How has this been tested?
Manually.
Test Instructions
The easiest method to test this updated
BoxControlwill be to test #30607.That PR is based upon this one and through checking it out and testing it, this updated
BoxControlwill also be tested.Screenshots
Types of changes
New feature.
Checklist:
*.native.jsfiles for terms that need renaming or removal).