feat(group): make inital open state configurable #139
Conversation
marstamm
force-pushed
the
618-element-template-sections
branch
from
36579d3 to
ca634c7
Compare
bpmn-io-tasks
bot
added
needs review
|
Ready for review. I also adjusted the downstream integration: bpmn-io/bpmn-js-properties-panel#621 |
There was a problem hiding this comment.
Looks good in general, makes sense to make it possible to set the initial state from a parent component. 👍
Just for my understanding: did we consider building it via the useEvent API we recently introduced? So to make it also possible to open groups from elsewhere? /cc @philippfromme
Probably that's a different use case because this open state should also be persisted, right?
src/context/LayoutContext.js
Outdated
| @@ -5,7 +5,7 @@ import { | |||
| const LayoutContext = createContext({ | |||
| layout: {}, | |||
| setLayout: () => {}, | |||
| getLayoutForKey: () => {}, | |||
| getLayoutForKey: (key, defaultValue) => defaultValue, | |||
There was a problem hiding this comment.
Do we need to be that explicit at this stage?
There was a problem hiding this comment.
You are right, we can mock it in the tests instead. I went ahead and did that
I did consider going this way. However, the use case is slightly different and not as clean as solving it via props. E.g., we would always have to fire the event right after rendering while also checking that the state is persisted. Also, the current API only supports elements and not groups. In the end, I decided to go with |
marstamm
force-pushed
the
618-element-template-sections
branch
from
9f8b1e4 to
2c242ac
Compare
There was a problem hiding this comment.
I like the way this is going + agree to #139 (comment).
@pinussilvestrus Please give your 👍 to confirm implementation.
related to bpmn-io/bpmn-js-properties-panel#618
for integration PR in bpmn-js properties panel, see bpmn-io/bpmn-js-properties-panel#621