Skip to content
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

feat(hooks): add useLayoutState #99

Merged
merged 1 commit into from
Oct 7, 2021
Merged

Conversation

pinussilvestrus
Copy link
Contributor

Proposal: This adds a useLayoutState hook to persist state inside the global LayoutContext. The LayoutContext can be used to persist state for components that might not be rendered sometimes (example groups), but we want to keep.

const groupId = 'general';

const [ open, setOpen ] = useLayoutState(
  [ 'groups', id, 'open' ],
  false
);

setOpen(true);

Potentially it would also be possible to persist this layout state outside of the application (e.g. the Camunda Modeler), via propertiesPanel.layoutChanged + the layoutConfig parameter.

Closes #98

Kapture 2021-09-24 at 10 46 30

@nikku
Copy link
Member

nikku commented Sep 24, 2021

To understand the overall direction we're heading UI-wise first: Layout state (collapsed/expanded) is sticky across elements: Section foo is open on element A, it will also be open on element B, if it exists there.

This PR adds the foundations to ensure that if element B does not contain foo the section is still opened in A once the user switches back?

@pinussilvestrus
Copy link
Contributor Author

To understand the overall direction we're heading UI-wise first: Layout state (collapsed/expanded) is sticky across elements: Section foo is open on element A, it will also be open on element B, if it exists there.

This PR adds the foundations to ensure that if element B does not contain foo the section is still opened in A once the user switches back?

Exactly!

@pinussilvestrus pinussilvestrus requested review from a team and andreasgeier and removed request for a team September 29, 2021 16:31
@pinussilvestrus
Copy link
Contributor Author

Adding @andreasgeier for a design review (proper behavior). Command to try it out

npx @bpmn-io/sr bpmn-io/bpmn-properties-panel#main -l bpmn-io/properties-panel#98-keep-group-state -c 'npm run start:platform'

@andreasgeier
Copy link

andreasgeier commented Oct 1, 2021

The root level (sections/groups) works exactly as expected - great work!

But we have inconsistency on other levels. For instance, a user creates 3 input variables and some of them are in opened state. If the input group is closed and re-opened without switching the element, the variables keep their state but when switching the element selection, all variables within the input section are closed.

@pinussilvestrus Let me know how to proceed here. Having this behavior available on the root is already a great improvement. We can discuss if we want to handle other levels as well (and how) or tackling it in another PR or ignore it for now.

@pinussilvestrus
Copy link
Contributor Author

I think we have a good chance to use the same strategy for other components as well (as sketched for list items in #90 (comment)).

I think this PR is a good starting point how to tackle a similar set of problems (persisting component state during a session). If we agree on something (+ merge) we can freely do the same for other parts (as mentioned in #99 (comment)).

Copy link

@andreasgeier andreasgeier left a comment

Choose a reason for hiding this comment

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

Based on our discussion that this solution works as expected for groups and the behavior of the sub-levels are considered in another issue, I can approve this.


const { result } = renderHook(() => useLayoutState(path), { wrapper });

const [ , setState ] = result.current;
Copy link
Member

Choose a reason for hiding this comment

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

🤯

@barmac
Copy link
Member

barmac commented Oct 7, 2021

LGTM

@barmac
Copy link
Member

barmac commented Oct 7, 2021

Really cool stuff.

@pinussilvestrus pinussilvestrus merged commit e01dd05 into main Oct 7, 2021
@pinussilvestrus pinussilvestrus deleted the 98-keep-group-state branch October 7, 2021 15:18
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 7, 2021
@pinussilvestrus
Copy link
Contributor Author

Let's roll this out for other components as well as a next step (example: #90).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Open state of groups is not persisted between element changes
4 participants