Skip to content

Conversation

@aduth
Copy link
Member

@aduth aduth commented Jun 14, 2017

Related: #1031

This pull request seeks to explore an alternative to assigning a leftDivider property on a control for displaying dividers, instead representing division between sets of controls by a nested array structure. With these changes, the <Toolbar /> component now accepts controls as either an array of objects, or now also as an array of array of objects, where in the latter case each top-level array is considered a "set of controls" with dividers between sets.

Testing instructions:

Verify that unit tests pass:

npm test

Ensure that there are no regressions in the behavior of toolbar controls, both for non-nested controls (e.g. image) and also for nested controls (e.g. freeform block).

@aduth aduth added the General Interface Parts of the UI which don't fall neatly under other labels. label Jun 14, 2017
@aduth aduth requested review from BoardJames and youknowriad June 14, 2017 20:12
@aduth aduth changed the title Supported nested array of toolbar controls Support nested array of toolbar controls Jun 14, 2017
Copy link

@BoardJames BoardJames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great. It is a much nicer solution and I can't find any errors.

@aduth aduth force-pushed the update/nested-toolbar-controls branch from cd1462f to bf96ef4 Compare June 15, 2017 17:37
@aduth
Copy link
Member Author

aduth commented Jun 15, 2017

Rebased and included updated text for BlockControls documentation added in #1172.

Will merge shortly. Thanks for the review!

@aduth aduth merged commit 85b1ed1 into master Jun 15, 2017
@youknowriad youknowriad deleted the update/nested-toolbar-controls branch June 15, 2017 18:28
@pinarol pinarol mentioned this pull request Jun 27, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

General Interface Parts of the UI which don't fall neatly under other labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants