fix: Unable to reorder categories in legend (PT-187392421) (#1255)

* fix: Unable to reorder categories in legend (PT-187392421) [#187392421](https://www.pivotaltracker.com/story/show/187392421)
concord-consortium · May 13, 2024 · 3aeda13 · 3aeda13
1 parent 0fa05b2
commit 3aeda13
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 32 deletions.
diff --git a/v3/src/components/data-display/components/legend/categorical-legend.tsx b/v3/src/components/data-display/components/legend/categorical-legend.tsx
@@ -88,6 +88,19 @@ export const CategoricalLegend = observer(
     const
       keysElt = useRef(null)
 
+    const setCategoryData = useCallback(() => {
+      if (categoriesRef.current) {
+        const newCategoryData = categoriesRef.current.map((cat: string, index) => ({
+          category: cat,
+          color: dataConfiguration?.getLegendColorForCategory(cat) || missingColor,
+          column: index % layoutData.current.numColumns,
+          index,
+          row: Math.floor(index / layoutData.current.numColumns)
+        }))
+        categoryData.current = newCategoryData
+      }
+    }, [dataConfiguration])
+
       const computeLayout = useCallback(() => {
         categoriesRef.current = dataConfiguration?.categoryArrayForAttrRole('legend')
         const numCategories = categoriesRef.current?.length,
@@ -101,18 +114,9 @@ export const CategoricalLegend = observer(
         lod.numColumns = Math.max(Math.floor(lod.fullWidth / lod.maxWidth), 1)
         lod.columnWidth = lod.fullWidth / lod.numColumns
         lod.numRows = Math.ceil((numCategories ?? 0) / lod.numColumns)
-        categoryData.current.length = 0
-        categoriesRef.current && Array.from(categoriesRef.current).forEach((cat: string, index) => {
-          categoryData.current.push({
-            category: cat,
-            color: dataConfiguration?.getLegendColorForCategory(cat) || missingColor,
-            index,
-            row: Math.floor(index / lod.numColumns),
-            column: index % lod.numColumns
-          })
-        })
+        setCategoryData()
         layoutData.current = lod
-      }, [dataConfiguration, tileWidth])
+      }, [dataConfiguration, setCategoryData, tileWidth])
 
       const computeDesiredExtent = useCallback(() => {
         if (dataConfiguration?.placeCanHaveZeroExtent('legend')) {
@@ -204,16 +208,17 @@ export const CategoricalLegend = observer(
             newCatIndex = coordinatesToCatIndex(lod, numCategories, newDragPosition)
           if (newCatIndex >= 0 && newCatIndex !== dI.indexOfCategory) {
             // swap the two categories
-            duration.current = transitionDuration / 2
             dataConfiguration?.storeAllCurrentColorsForAttrRole('legend')
             dataConfiguration?.swapCategoriesForAttrRole('legend', dI.indexOfCategory, newCatIndex)
+            categoriesRef.current = dataConfiguration?.categoryArrayForAttrRole('legend')
+            setCategoryData()
             dI.indexOfCategory = newCatIndex
           } else {
             refreshKeys()
           }
           dI.currentDragPosition = newDragPosition
         }
-      }, [dataConfiguration, refreshKeys])
+      }, [dataConfiguration, setCategoryData, refreshKeys])
 
       const onDragEnd = useCallback(() => {
         duration.current = transitionDuration

diff --git a/v3/src/components/data-display/models/data-configuration-model.ts b/v3/src/components/data-display/models/data-configuration-model.ts
@@ -258,6 +258,9 @@ export const DataConfigurationModel = types
         const attributeID = self.attributeID(role) || ''
         return self.metadata.getCategorySet(attributeID)
       }
+    },
+    potentiallyCategoricalRoles(): AttrRole[] {
+      return ["legend"] as const
     }
   }))
   .views(self => ({
@@ -283,7 +286,17 @@ export const DataConfigurationModel = types
         })
         return orderedCategories
       }
-    })
+    }),
+    getAllCategoriesForRoles() {
+      const categories: Map<AttrRole, string[]> = new Map()
+      self.potentiallyCategoricalRoles().forEach(role => {
+        const categorySet = self.categorySetForAttrRole(role)
+        if (categorySet) {
+          categories.set(role, categorySet.valuesArray)
+        }
+      })
+      return categories
+    }
   }))
   .views(self => ({
     getUnsortedCaseDataArray(caseArrayNumber: number): CaseData[] {
@@ -615,6 +628,14 @@ export const DataConfigurationModel = types
         data => self.handleDataSetChange(data),
         {name: "DataConfigurationModel.afterCreate.reaction [dataset]", fireImmediately: true }
       ))
+      addDisposer(self, reaction(
+        () => self.getAllCategoriesForRoles(),
+        () => self.clearCasesCache(),
+        {
+          name: "DataConfigurationModel.afterCreate.reaction [getAllCategoriesForRoles]",
+          equals: comparer.structural
+        }
+      ))
       // respond to change of legend attribute
       addDisposer(self, reaction(
         () => JSON.stringify(self.attributeDescriptionForRole("legend")),

diff --git a/v3/src/components/graph/models/graph-data-configuration-model.ts b/v3/src/components/graph/models/graph-data-configuration-model.ts
@@ -232,6 +232,9 @@ export const GraphDataConfigurationModel = DataConfigurationModel
       const attrTypes = self.attrTypes
       return Object.values(attrTypes).filter(a => a === "categorical").length
     },
+    potentiallyCategoricalRoles(): AttrRole[] {
+      return ["legend", "x", "y", "topSplit", "rightSplit"] as const
+    },
     get hasExactlyOneCategoricalAxis() {
       const attrTypes = self.attrTypes
       const xHasCategorical = attrTypes.bottom === "categorical" || attrTypes.top === "categorical"
@@ -253,16 +256,6 @@ export const GraphDataConfigurationModel = DataConfigurationModel
     }
   }))
   .views(self => ({
-    getAllCategoriesForRoles() {
-      const categories: Map<AttrRole, string[]> = new Map()
-      ;(["x", "y", "topSplit", "rightSplit"] as const).forEach(role => {
-        const categorySet = self.categorySetForAttrRole(role)
-        if (categorySet) {
-          categories.set(role, categorySet.valuesArray)
-        }
-      })
-      return categories
-    },
     getCategoriesOptions() {
       // Helper used often by adornments that usually ask about the same categories and their specifics.
       const xAttrType = self.attributeType("x")
@@ -648,14 +641,6 @@ export const GraphDataConfigurationModel = DataConfigurationModel
     const baseRemoveAttributeFromRole = self.removeAttributeFromRole
     return {
       afterCreate() {
-        addDisposer(self, reaction(
-          () => self.getAllCategoriesForRoles(),
-          () => self.clearCasesCache(),
-          {
-            name: "GraphDataConfigurationModel.afterCreate.reaction [getAllCategoriesForRoles]",
-            equals: comparer.structural
-          }
-        ))
         addDisposer(self, reaction(
           () => self.getAllCellKeys(),
           () => self.clearGraphSpecificCasesCache(),