Persist skeleton group expansion state

CHANGELOG.unreleased.md

@@ -19,6 +19,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - The context menu that is opened upon right-clicking a segment in the dataview port now contains the segment's name. [#7920](https://github.com/scalableminds/webknossos/pull/7920) 
 - Upgraded backend dependencies for improved performance and stability. [#7922](https://github.com/scalableminds/webknossos/pull/7922)
 - It is now saved whether segment groups are collapsed or expanded, so this information doesn't get lost e.g. upon page reload. [#7928](https://github.com/scalableminds/webknossos/pull/7928/)
+- It is now saved whether skeleton groups are collapsed or expanded. This information is also persisted to NML output. [#7939](https://github.com/scalableminds/webknossos/pull/7939)
 - The context menu entry "Focus in Segment List" expands all necessary segment groups in the segments tab to show the highlighted segment. [#7950](https://github.com/scalableminds/webknossos/pull/7950)
 - In the proofreading mode, you can enable/disable that only the active segment and the hovered segment are rendered. [#7654](https://github.com/scalableminds/webknossos/pull/7654)
 

app/models/annotation/AnnotationUploadService.scala

@@ -115,7 +115,7 @@ class AnnotationUploadService @Inject()(tempFileService: TempFileService) extend
     def wrapTreesInGroup(name: String, tracing: SkeletonTracing): SkeletonTracing = {
       val unusedGroupId = getMaximumTreeGroupId(tracing.treeGroups) + 1
       val newTrees = tracing.trees.map(tree => tree.copy(groupId = Some(tree.groupId.getOrElse(unusedGroupId))))
-      val newTreeGroups = Seq(TreeGroup(name, unusedGroupId, tracing.treeGroups))
+      val newTreeGroups = Seq(TreeGroup(name, unusedGroupId, tracing.treeGroups, isExpanded = Some(true)))
       tracing.copy(trees = newTrees, treeGroups = newTreeGroups)
     }
 

app/models/annotation/nml/NmlParser.scala

@@ -165,7 +165,8 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
       id <- idText.toIntOpt ?~ Messages("nml.treegroup.id.invalid", idText)
       children <- (node \ "group").map(parseTreeGroup).toList.toSingleBox("")
       name = getSingleAttribute(node, "name")
-    } yield TreeGroup(name, id, children)
+      isExpanded = getSingleAttribute(node, "isExpanded").toBooleanOpt.getOrElse(true)
+    } yield TreeGroup(name, id, children, isExpanded = Some(isExpanded))
   }
 
   private def extractVolumes(volumeNodes: NodeSeq)(implicit m: MessagesProvider): immutable.Seq[NmlVolumeTag] =

app/models/annotation/nml/NmlWriter.scala

@@ -373,6 +373,7 @@ class NmlWriter @Inject()(implicit ec: ExecutionContext) extends FoxImplicits {
       Xml.withinElementSync("group") {
         writer.writeAttribute("name", t.name)
         writer.writeAttribute("id", t.groupId.toString)
+        writer.writeAttribute("isExpanded", t.isExpanded.getOrElse(true).toString)
         writeTreeGroupsAsXml(t.children)
       }
     }

frontend/javascripts/oxalis/model/helpers/nml_helpers.ts

@@ -504,6 +504,7 @@ function serializeTreeGroups(treeGroups: Array<TreeGroup>, trees: Array<Tree>):
         {
           id: treeGroup.groupId,
           name: treeGroup.name,
+          ...(treeGroup.isExpanded ? {} : { isExpanded: treeGroup.isExpanded }),
         },
         serializeTreeGroups(treeGroup.children, trees),
       ),
@@ -931,6 +932,7 @@ export function parseNml(nmlString: string): Promise<{
             const newGroup = {
               groupId: _parseInt(attr, "id"),
               name: _parseEntities(attr, "name"),
+              isExpanded: _parseBool(attr, "isExpanded", true),
               children: [],
             };
             if (existingTreeGroupIds.has(newGroup.groupId)) {

frontend/javascripts/oxalis/view/right-border-tabs/skeleton_tab_view.tsx

@@ -346,10 +346,11 @@ class SkeletonTabView extends React.PureComponent<Props, State> {
   getTreeAndTreeGroupList = memoizeOne(
     (trees: TreeMap, treeGroups: Array<TreeGroup>, sortBy: string): Array<TreeOrTreeGroup> => {
       const groupToTreesMap = createGroupToTreesMap(trees);
-      const rootGroup = {
+      const rootGroup: TreeGroup = {
         name: "Root",
         groupId: MISSING_GROUP_ID,
         children: treeGroups,
+        isExpanded: true,
       };
 
       const makeTree = (tree: Tree) => ({

frontend/javascripts/oxalis/view/right-border-tabs/tree_hierarchy_view.tsx

@@ -42,7 +42,6 @@ import {
   GroupTypeEnum,
   deepFlatFilter,
   getNodeKey,
-  getNodeKeyFromNode,
   moveGroupsHelper,
   findParentGroupNode,
 } from "oxalis/view/right-border-tabs/tree_hierarchy_view_helpers";
@@ -69,8 +68,8 @@ import { formatNumberToLength, formatLengthAsVx } from "libs/format_utils";
 import { api, Store } from "oxalis/singletons";
 import { ChangeColorMenuItemContent } from "components/color_picker";
 import { HideTreeEdgesIcon } from "./hide_tree_eges_icon";
-import { useEffectOnlyOnce } from "libs/react_hooks";
 import { ColoredDotIcon } from "./segments_tab/segment_list_item";
+import { mapGroups } from "oxalis/model/accessors/skeletontracing_accessor";
 
 type Props = {
   activeTreeId: number | null | undefined;
@@ -106,25 +105,20 @@ function TreeHierarchyView(props: Props) {
       name: "Root",
       groupId: MISSING_GROUP_ID,
       children: props.treeGroups,
+      isExpanded: true,
     };
 
     const generatedGroupTree = insertTreesAndTransform([rootGroup], groupToTreesMap, props.sortBy);
     setUITreeData(generatedGroupTree);
   }, [props.trees, props.sortBy, props.treeGroups]);
 
-  useEffectOnlyOnce(() => {
-    // set default expanded keys
-    // the defaults should include the root node and at the active tree's group if applicable
-    let defaultExpandedKeys: React.Key[] = [getNodeKey(GroupTypeEnum.GROUP, MISSING_GROUP_ID)];
-    if (props.activeTreeId) {
-      const activeTreesGroupId = props.trees[props.activeTreeId].groupId;
-      if (activeTreesGroupId) {
-        defaultExpandedKeys.push(getNodeKey(GroupTypeEnum.GROUP, activeTreesGroupId));
-      }
-
-      setExpandedNodeKeys(defaultExpandedKeys);
-    }
-  });
+  useEffect(() => {
+    const expandedKeys = deepFlatFilter(
+      UITreeData,
+      (node) => node.type === GroupTypeEnum.GROUP && node.expanded,
+    ).map((node) => node.key);
+    setExpandedNodeKeys(expandedKeys);
+  }, [UITreeData]);
 
   useEffect(() => {
     // scroll to active tree if it changes
@@ -148,17 +142,25 @@ function TreeHierarchyView(props: Props) {
 
   const onExpand: TreeProps<TreeNode>["onExpand"] = (expandedKeys, info) => {
     const clickedNode = info.node;
+    const expandedKeySet = new Set(expandedKeys);
 
     if (clickedNode.type === GroupTypeEnum.GROUP && info.expanded === false) {
       // when collapsing a group, we need to collapse all its sub-gropus
       const subGroupKeys = deepFlatFilter(
         [clickedNode],
         (node) => node.type === GroupTypeEnum.GROUP,
       ).map((node) => node.key);
-      expandedKeys = _.without(expandedKeys, ...subGroupKeys);
+      subGroupKeys.forEach((key) => expandedKeySet.delete(key));
     }
-
-    setExpandedNodeKeys(expandedKeys);
+    const newGroups = mapGroups(props.treeGroups, (group) => {
+      const shouldBeExpanded = expandedKeySet.has(getNodeKey(GroupTypeEnum.GROUP, group.groupId));
+      if (shouldBeExpanded !== group.isExpanded) {
+        return { ...group, isExpanded: shouldBeExpanded };
+      } else {
+        return group;
+      }
+    });
+    setUpdateTreeGroups(newGroups);
   };
 
   const onCheck: TreeProps<TreeNode>["onCheck"] = (_checkedKeysValue, info) => {
@@ -238,17 +240,30 @@ function TreeHierarchyView(props: Props) {
     }
   }
 
-  function setExpansionOfAllSubgroupsTo(group: TreeNode, expanded: boolean) {
-    const selectedGroupIdKey = getNodeKeyFromNode(group);
-    const subGroupKeys = deepFlatFilter([group], (node) => node.type === GroupTypeEnum.GROUP).map(
-      (node) => node.key,
-    );
-
-    if (expanded) {
-      setExpandedNodeKeys(_.uniq([...expandedNodeKeys, ...subGroupKeys]));
-    } else {
-      setExpandedNodeKeys(_.without(expandedNodeKeys, selectedGroupIdKey, ...subGroupKeys));
+  function setExpansionOfAllSubgroupsTo(parentGroup: TreeNode, expanded: boolean) {
+    if (parentGroup.id === MISSING_GROUP_ID) {
+      const newGroups = mapGroups(props.treeGroups, (group) => {
+        if (group.isExpanded !== expanded) {
+          return { ...group, isExpanded: expanded };
+        }
+        return group;
+      });
+      setUpdateTreeGroups(newGroups);
+      return;
     }
+    const subGroups = getGroupByIdWithSubgroups(props.treeGroups, parentGroup.id);
+    const subGroupsMap = new Set(subGroups);
+    // If the subgroups should be collapsed, do not collapse the group itself.
+    // Do expand the group if the subgroups are expanded though.
+    if (expanded === false) subGroupsMap.delete(parentGroup.id);
+    const newGroups = mapGroups(props.treeGroups, (group) => {
+      if (subGroupsMap.has(group.groupId) && expanded !== group.isExpanded) {
+        return { ...group, isExpanded: expanded };
+      } else {
+        return group;
+      }
+    });
+    setUpdateTreeGroups(newGroups);
   }
 
   function onMoveWithContextAction(targetParentNode: TreeNode) {
@@ -281,6 +296,7 @@ function TreeHierarchyView(props: Props) {
         ? dragTargetNode.id
         : props.trees[dragTargetNode.id].groupId ?? MISSING_GROUP_ID;
 
+    let updatedTreeGroups: TreeGroup[] = props.treeGroups;
     if (draggedNode.type === GroupTypeEnum.TREE) {
       let allTreesToMove = [draggedNode.id];
 
@@ -297,12 +313,18 @@ function TreeHierarchyView(props: Props) {
       onBatchActions(moveActions, "SET_TREE_GROUP");
     } else {
       // A group was dragged - update the groupTree
-      const newTreeGroups = moveGroupsHelper(props.treeGroups, draggedNode.id, parentGroupId);
-      setUpdateTreeGroups(newTreeGroups);
+      updatedTreeGroups = moveGroupsHelper(props.treeGroups, draggedNode.id, parentGroupId);
     }
 
     // in either case expand the parent group
-    setExpandedNodeKeys([...expandedNodeKeys, getNodeKey(GroupTypeEnum.GROUP, parentGroupId)]);
+    const newGroups = mapGroups(updatedTreeGroups, (group) => {
+      if (group.groupId === parentGroupId && !group.isExpanded) {
+        return { ...group, isExpanded: true };
+      } else {
+        return group;
+      }
+    });
+    setUpdateTreeGroups(newGroups);
   }
 
   function createGroup(groupId: number) {

frontend/javascripts/oxalis/view/right-border-tabs/tree_hierarchy_view_helpers.ts

@@ -90,6 +90,7 @@ export function insertTreesAndTransform(
       timestamp: 0,
       children: _.orderBy(treeNodeChildren, ["name"], ["asc"]),
       disableCheckbox: treeNodeChildren.length === 0,
+      expanded: group.isExpanded == null || group.isExpanded,
       isChecked: treeNodeChildren.every(
         // Groups that don't contain any trees should not influence the state of their parents
         (groupOrTree) => groupOrTree.isChecked || !groupOrTree.containsTrees,

frontend/javascripts/test/libs/nml.spec.ts

@@ -126,18 +126,21 @@ const initialSkeletonTracing: SkeletonTracing = {
     {
       groupId: 1,
       name: "Axon 1",
+      isExpanded: true,
       children: [
         {
           groupId: 3,
           name: "Blah",
           children: [],
+          isExpanded: false,
         },
       ],
     },
     {
       groupId: 2,
       name: "Axon 2",
       children: [],
+      isExpanded: true,
     },
   ],
   activeTreeId: 1,

frontend/javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.md

@@ -57,7 +57,7 @@ Generated by [AVA](https://avajs.dev).
       </comments>␊
       <groups>␊
         <group id="1" name="Axon 1">␊
-          <group id="3" name="Blah" />␊
+          <group id="3" name="Blah" isExpanded="false" />␊
         </group>␊
         <group id="2" name="Axon 2" />␊
       </groups>␊
@@ -119,7 +119,7 @@ Generated by [AVA](https://avajs.dev).
       </comments>␊
       <groups>␊
         <group id="1" name="Axon 1">␊
-          <group id="3" name="Blah" />␊
+          <group id="3" name="Blah" isExpanded="false" />␊
         </group>␊
         <group id="2" name="Axon 2" />␊
       </groups>␊
@@ -179,7 +179,7 @@ Generated by [AVA](https://avajs.dev).
       </comments>␊
       <groups>␊
         <group id="1" name="Axon 1">␊
-          <group id="3" name="Blah" />␊
+          <group id="3" name="Blah" isExpanded="false" />␊
         </group>␊
         <group id="2" name="Axon 2" />␊
       </groups>␊

test/backend/Dummies.scala

@@ -46,8 +46,12 @@ object Dummies {
     Some(true)
   )
 
-  val treeGroup1: TreeGroup = TreeGroup("Axon 1", 1, Seq(TreeGroup("Blah", 3), TreeGroup("Blah 2", 4)))
-  val treeGroup2: TreeGroup = TreeGroup("Axon 2", 2)
+  val treeGroup1: TreeGroup = TreeGroup(
+    "Axon 1",
+    1,
+    Seq(TreeGroup("Blah", 3, Seq.empty, Some(false)), TreeGroup("Blah 2", 4, Seq.empty, Some(false))),
+    Some(true))
+  val treeGroup2: TreeGroup = TreeGroup("Axon 2", 2, Seq.empty, Some(true))
 
   val skeletonTracing: SkeletonTracing = SkeletonTracing(
     "dummy_dataset",

test/backend/NMLUnitTestSuite.scala

@@ -80,6 +80,28 @@ class NMLUnitTestSuite extends PlaySpec {
     }
   }
 
+  "NML writing and parsing" should {
+    "should add missing isExpanded props with a default of true" in {
+      val treeGroupsWithOmittedIsExpanded = dummyTracing.treeGroups.map(
+        treeGroup =>
+          new TreeGroup(name = treeGroup.name,
+                        groupId = treeGroup.groupId,
+                        children = treeGroup.children,
+                        isExpanded = if (treeGroup.isExpanded.getOrElse(true)) None else Some(false)))
+      val dummyTracingWithOmittedIsExpandedTreeGroupProp =
+        dummyTracing.copy(treeGroups = treeGroupsWithOmittedIsExpanded)
+      writeAndParseTracing(dummyTracingWithOmittedIsExpandedTreeGroupProp) match {
+        case Full(tuple) =>
+          tuple match {
+            case (Some(tracing), _, _, _) =>
+              assert(tracing == dummyTracing)
+            case _ => throw new Exception
+          }
+        case _ => throw new Exception
+      }
+    }
+  }
+
   "NML Parser" should {
     "throw an error for invalid comment with a non-existent nodeId" in {
       // the comment nodeId is referring to a non-existent node therefore invalid

test/backend/SkeletonUpdateActionsUnitTestSuite.scala

@@ -215,7 +215,7 @@ class SkeletonUpdateActionsUnitTestSuite extends PlaySpec {
     "update a top level tree group" in {
       val updatedName = "Axon 2 updated"
       val updateTreeGroupsSkeletonAction = new UpdateTreeGroupsSkeletonAction(
-        List(UpdateActionTreeGroup(updatedName, 2, List()))
+        List(UpdateActionTreeGroup(updatedName, 2, Some(true), List()))
       )
       val result = applyUpdateAction(updateTreeGroupsSkeletonAction)
       assert(result.trees == Dummies.skeletonTracing.trees)
@@ -226,7 +226,11 @@ class SkeletonUpdateActionsUnitTestSuite extends PlaySpec {
       val updatedNameTop = "Axon 1 updated"
       val updatedNameNested = "Axon 3 updated"
       val updateTreeGroupsSkeletonAction = new UpdateTreeGroupsSkeletonAction(
-        List(UpdateActionTreeGroup(updatedNameTop, 1, List(UpdateActionTreeGroup(updatedNameNested, 3, List()))))
+        List(
+          UpdateActionTreeGroup(updatedNameTop,
+                                1,
+                                Some(true),
+                                List(UpdateActionTreeGroup(updatedNameNested, 3, Some(false), List()))))
       )
       val result = applyUpdateAction(updateTreeGroupsSkeletonAction)
       assert(result.trees == Dummies.skeletonTracing.trees)

webknossos-datastore/proto/SkeletonTracing.proto

@@ -56,6 +56,7 @@ message TreeGroup {
   required string name = 1;
   required int32 groupId = 2;
   repeated TreeGroup children = 3;
+  optional bool isExpanded = 4;
 }
 
 message SkeletonTracing {

webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActionHelper.scala

@@ -22,5 +22,5 @@ trait SkeletonUpdateActionHelper extends ProtoGeometryImplicits {
     Comment(aComment.nodeId, aComment.content)
 
   protected def convertTreeGroup(aTreeGroup: UpdateActionTreeGroup): TreeGroup =
-    TreeGroup(aTreeGroup.name, aTreeGroup.groupId, aTreeGroup.children.map(convertTreeGroup))
+    TreeGroup(aTreeGroup.name, aTreeGroup.groupId, aTreeGroup.children.map(convertTreeGroup), aTreeGroup.isExpanded)
 }

webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/UpdateActionTreeGroup.scala

@@ -2,7 +2,10 @@ package com.scalableminds.webknossos.tracingstore.tracings.skeleton.updating
 
 import play.api.libs.json.{Json, OFormat}
 
-case class UpdateActionTreeGroup(name: String, groupId: Int, children: List[UpdateActionTreeGroup])
+case class UpdateActionTreeGroup(name: String,
+                                 groupId: Int,
+                                 isExpanded: Option[Boolean],
+                                 children: List[UpdateActionTreeGroup])
 
 object UpdateActionTreeGroup {
   implicit val jsonFormat: OFormat[UpdateActionTreeGroup] = Json.format[UpdateActionTreeGroup]