Save expanded/collapsed state of segment groups

[knollengewaechs]

Steps to test:

• Open an annotation and go to segments tab. Create a group hierarchy (around three levels min) with some segments in each group.
• Expand and collapse some groups, remember state.
• Change to another tab and back. The state (groups expanded/collapsed) should be the same.
• Reload the page. The state should still be the same
• Test that old behavior is still working. Expanding and collapsing via arrow icon, expansion/collapse of all subgroups etc.

TODOs:

• fix bug that expand children on root doesnt work after collapsing all subgroups

Issues:

• fixes Reloading the page should retain collapsed-state of (segment) groups #7825

---

(Please delete unneeded items, merge only when none are left open)

• Updated changelog
• Updated migration guide if applicable
• Updated documentation if applicable
• Adapted wk-libs python client if relevant API parts change
• Removed dev-only changes like prints and application.conf edits
• Considered common edge cases
• Needs datastore update after deployment

[fm3]

Backend LGTM :) Testing worked nicely, backwards compatibility looks OK. As mentioned in Slack, the root group gets expanded again after reload, raising the question if it needs to be collapsible in the first place, but I’d say that’s not particularly important.

Please add a changelog entry :)

[knollengewaechs]

I am not 100% sure, but I feel like expanding and collapsing got slower. I compared it to webknossos.org and I am still not super sure. But I wanted to mention it. Maybe there is a faster way to get the current expanded/collapsed group state from the segmentGroups? @philippotto

[philippotto]

cool :) I left some feedback.

[philippotto]

why doesn't this method handle the root group like the expandGroups method does? is it not necessary in this case?

[philippotto]

also: the two methods differ in their semantics of the parameters. for collapseGroups I understand it so that the referenced groups will be collapsed (and other collapsed groups will stay as before). for expandGroups, it seems like only the referenced groups should be expanded (and all the others should not be expanded). can this be unified?

[knollengewaechs]

you're right, the methods differ. this is because expandGroups is called by the Tree component and the expandSubgroups menu entry. collapseGroups is only called to collapse the children. The tree component also calls this if groups are collapsed, by removing the collapsed groups trees from the list. Thus isExpanded has to be set to false for these groups. Other than that, the root group is handled differently because on expand all subgroups the parent group is expanded if it was collapsed before. This is different for expanded groups, their state doesn't change if their children are collapsed.
I could use three methods: one for the Tree component and two for "collapse/expand subgroups". And then comment what I said above. Do you think that this might be a good solution? @philippotto

[philippotto]

ah, I understand. two methods are fine then. I'd the expandGroups method to setExpandedGroups. That way, it should be clearer that the semantics is different to collapseGroups.

Other than that, the root group is handled differently because on expand all subgroups the parent group is expanded if it was collapsed before. This is different for expanded groups, their state doesn't change if their children are collapsed.

A comment for that would be good, yes 👍

[knollengewaechs]

I was wrong: this code snippet enables an update of the root group itself. the state update (isRootGroupExpanded) triggers an update of the root group object which is separate because the rootgroup is only stored in the component.

[philippotto]

Only after reviewing, I noticed that this PR is based on #7911. I manually changed the base of the PR now. In the future, you should do this when creating the PR :) Can be done here:

Otherwise, all the changes of the first PR will also be shown here in the diff, too.

[knollengewaechs]

Otherwise, all the changes of the first PR will also be shown here in the diff, too.

I see. So I set it to the base branch although I will merge it into master eventually. And then I can probably set it to master once the base branch is merged and I have rebased the commits of this PR, right?

[philippotto]

I see. So I set it to the base branch although I will merge it into master eventually. And then I can probably set it to master once the base branch is merged and I have rebased the commits of this PR, right?

Exactly! Even better, GitHub will automatically set the base to "master" once the first PR is merged.

[knollengewaechs]

@philippotto because of the different base branch I rebased my changes on master. The numbers of the diff are slightly different (used to be +170, -12). I hope that is somewhat expected. It still works well, so I hope the rebase was fine.

[philippotto]

Great 🎉 Performance feels still a bit laggy, but this is unrelated to this PR. Let's merge it and iterate later :)

[knollengewaechs]

Great 🎉 Performance feels still a bit laggy, but this is unrelated to this PR. Let's merge it and iterate later :)

Thank you for the review! I feel like the skeleton tab made some different decisions regarding the removal of "unnecessary" options, by just showing them and saving some cost there. Maybe that is among the things to look out for.