allow custom ordering of default panels #784
Conversation
|
To capture our verbal conversation: I think the ideal API here would be to pass in two arrays-of-strings, which would define the desired order, and that would be applied within |
VeraZab
force-pushed
the
customize-default-editor
branch
from
5f21f1a to
cbee36f
Compare
VeraZab
changed the title
|
@dmt0 @nicolaskruchten please review |
|
Code looks good! |
|
A few test cases would be nice, including some edge cases like missing items in either the desired or actual order, mismatched group/name pairs, that sort of thing. Just to lock down the desired behaviour in a test :) |
VeraZab
force-pushed
the
customize-default-editor
branch
2 times, most recently
from
ba8c0dc to
8bb6d10
Compare
| function sortAlphabetically(a, b) { | ||
| const sortByGroup = a.props.group === b.props.group ? 0 : a.props.group < b.props.group ? -1 : 1; | ||
| const sortByName = a.props.name === b.props.name ? 0 : a.props.name < b.props.name ? -1 : 1; | ||
| return sortByGroup || sortByName; |
There was a problem hiding this comment.
clever! took me a sec to think about how that works :)
src/lib/__tests__/sortMenu-test.js
Outdated
| const orderProp = [{group: 'DEV', name: 'JSON'}, {group: 'DEV', name: 'Inspector'}]; | ||
| sortMenu(initialArray, orderProp); | ||
|
|
||
| expect(initialArray[0].props.name).toBe('JSON'); |
There was a problem hiding this comment.
I would express these as expect(initialArray).toBe( ... the full array ... ) ... More readable and more complete IMO :)
| describe('sortMenu', () => { | ||
| it('modifies original array to follow the group, then name order provided', () => { | ||
| const initialArray = [ | ||
| {props: {group: 'DEV', name: 'Inspector'}}, |
There was a problem hiding this comment.
I would consider making a helper here that takes in threes arrays of strings of form GROUP/NAME and does a split and does the comparison, just to have some very compact test cases below
| ) | ||
| ); | ||
|
|
||
| const desiredGroupOrder = order.map(panel => panel.group).filter(getUniqueValues); |
There was a problem hiding this comment.
arguably this is unnecessary right? It won't change the order :)
src/DefaultEditor.js
Outdated
| DefaultEditor.propTypes = { | ||
| children: PropTypes.node, | ||
| logoSrc: PropTypes.string, | ||
| order: PropTypes.array, |
There was a problem hiding this comment.
maybe this should be called menuPanelOrder or something, for clarity
|
💃 ... all comments are just notes, not requests to change |
VeraZab
force-pushed
the
customize-default-editor
branch
from
3213d33 to
bf7420c
Compare
No description provided.