183987088 graph color picker UI (#1568)

* Adds color swatch palette and color picker input to categorical legend brush menu

* Refactors point color setting into its own file

* removes some positioning console.logs

* cleanup

* Refactors PointColorSetting to be more general

Adds a way to set the categorical legend color in the data configuration

Refactors the DisplayItemFormatControl to use the PointColorSetting component

* Graph plot updates colors when category color is changed

* Coordinates color picker coler with the currently selected color

* Update color picker palette color to match V2

* Moves setcatlegendcolor to its own action

* fix: use onAnyAction rather than onAction to fix undo/redo bug

* Styles select checkmark depending on swatch color

* Adds  the non-standard swatch to palette as user selects a color in the color picker.

* chore: code review tweaks

* Changes hashStrinSets to a hashStringSet to ensure reordering categorical legend does not affect the hash.

* fixes broken cypress test

---------

Co-authored-by: Kirk Swenson <[email protected]>

v3/cypress/e2e/graph.spec.ts

@@ -3,6 +3,7 @@ import { TableTileElements as table } from "../support/elements/table-tile"
 import { ComponentElements as c } from "../support/elements/component-elements"
 import { ToolbarElements as toolbar } from "../support/elements/toolbar-elements"
 import { CfmElements as cfm } from "../support/elements/cfm"
+import { ColorPickerPaletteElements as cpp} from "../support/elements/color-picker-palette"
 import graphRules from '../fixtures/graph-rules.json'
 
 const collectionName = "Mammals"
@@ -368,39 +369,36 @@ context("Graph UI", () => {
         .get('div[role="slider"]').should('have.attr', 'aria-valuenow', "0.98")
 
       cy.log("changes the stroke color value and verifies the change")
+      // Ensure stroke initially has the expected value
 
-      cy.get('input[type="color"].chakra-input.color-picker-thumb.css-12wa1y3')
-        .eq(0)
-        .should('have.value', '#ffffff') // Ensure stroke initially has the expected value
+      cy.get('.color-picker-row').eq(0).find('.color-picker-thumb-swatch')
+        .should('have.css', 'background-color', 'rgb(255, 255, 255)')
         .then((colorPicker) => {
           // Change the value of the color picker
-          cy.wrap(colorPicker)
-            .invoke('val', '#000000')
-            .trigger('input')
-            .trigger('change')
+          cy.wrap(colorPicker).click()
+          cpp.getColorSettingSwatchCell().eq(0).click()
 
           // Verify the value has been updated
-          cy.wrap(colorPicker).should('have.value', '#000000')
+          cy.wrap(colorPicker).should('have.css', 'background-color', 'rgb(0, 0, 0)')
         })
+      cy.get('.codap-inspector-palette-header-title').click() //close the color palette
 
       cy.log("changes the point color value and verifies the change")
 
-      cy.get('input[type="color"].chakra-input.color-picker-thumb.css-12wa1y3')
-        .eq(1)
-        .should('have.value', '#e6805b') // Ensure point color initially has the expected value
+      cy.get('.color-picker-row').eq(1).find('.color-picker-thumb-swatch')
+        // Ensure point color initially has the expected value
+        .should('have.css', 'background-color', 'rgb(230, 128, 91)')
         .then((colorPicker) => {
           // Change the value of the color picker
-          cy.wrap(colorPicker)
-            .invoke('val', '#ff5733')
-            .trigger('input')
-            .trigger('change')
+          cy.wrap(colorPicker).click()
+          cpp.getColorSettingSwatchCell().eq(1).click()
 
           // Verify the value has been updated
-          cy.wrap(colorPicker).should('have.value', '#ff5733')
+          cy.wrap(colorPicker).should('have.css', 'background-color', 'rgb(169, 169, 169)')
         })
+      cy.get('.codap-inspector-palette-header-title').click() //close the color palette
 
       cy.log("checks the box Stroke same color as fill and check it")
-
       // Get the checkbox and check it
       cy.get('span.chakra-checkbox__control.css-4utxuo')
         .eq(0)
@@ -411,35 +409,31 @@ context("Graph UI", () => {
         .should('be.checked')
 
       // Use Cypress commands to get the first and second color picker elements
-      cy.get('input[type="color"].chakra-input.color-picker-thumb.css-12wa1y3')
-        .eq(0)
+      cy.get('.color-picker-row').eq(0).find('.color-picker-thumb-swatch')
         .as('fillColorPicker')
 
-      cy.get('input[type="color"].chakra-input.color-picker-thumb.css-12wa1y3')
-        .eq(1)
+      cy.get('.color-picker-row').eq(1).find('.color-picker-thumb-swatch')
         .as('strokeColorPicker')
 
       // Get the fill color value
-      cy.get('@fillColorPicker').invoke('val').then((fillColor) => {
+      cy.get('@fillColorPicker').invoke('css', 'background-color').then((fillColor) => {
         // Get the stroke color value and compare it to the fill color
         cy.get('@strokeColorPicker')
-        .should('have.value', fillColor)
+        .should('have.css', 'background-color', fillColor)
       })
 
       cy.log("changes the background color and verifies the change")
       // Use a more specific selector to find the background color input element
-      cy.get('input[type="color"].chakra-input.color-picker-thumb.css-12wa1y3')
-        .eq(2)
-        .should('have.value', '#ffffff') // Ensure background initially has the expected value
+      cy.get('.color-picker-row').eq(2).find('.color-picker-thumb-swatch')
+        // Ensure background initially has the expected value
+        .should('have.css', 'background-color', 'rgb(255, 255, 255)')
         .then((backgroundColorPicker) => {
           // Change the value of the background color picker
-          cy.wrap(backgroundColorPicker)
-            .invoke('val', '#ff5733')
-            .trigger('input')
-            .trigger('change')
+          cy.wrap(backgroundColorPicker).click()
+          cpp.getColorSettingSwatchCell().eq(4).click()
 
           // Verify the value has been updated
-          cy.wrap(backgroundColorPicker).should('have.value', '#ff5733')
+          cy.wrap(backgroundColorPicker).should('have.css', 'background-color', 'rgb(173, 35, 35)')
         })
 
       cy.log("finds the Transparent checkbox and verifies it can be checked")

v3/cypress/support/elements/color-picker-palette.ts

@@ -0,0 +1,53 @@
+export const ColorPickerPaletteElements = {
+  getColorPalette() {
+    return cy.get(".color-picker-palette")
+  },
+  getCategoricalColorSettingsGroup() {
+    return cy.get(".cat-color-setting")
+  },
+  getCategoricalColorSettingRow() {
+    return cy.get(".cat-color-setting .color-picker-row.cat-color-picker")
+  },
+  getCategoricalColorSettingLabel() {
+    return cy.get(".cat-color-setting .form-label.color-picker")
+  },
+  getCategoricalColorSettingButton() {
+    return cy.get(".cat-color-setting .color-picker-thumb")
+  },
+  getCategoricalColorSettingSwatch() {
+    return cy.get(".cat-color-setting .color-picker-thumb-swatch")
+  },
+  getColorSettingPalette() {
+    return cy.get(".color-picker-palette-container")
+  },
+  getColorSettingSwatchGrid() {
+    return cy.get(".color-swatch-grid")
+  },
+  getColorSettingSwatchCell() {
+    return cy.get(".color-swatch-cell")
+  },
+  getColorSettingSwatchRow() {
+    return cy.get(".color-swatch-row")
+  },
+  getSelectedSwatchCell() {
+    return cy.get(".selected")
+  },
+  getColorPickerToggleButton() {
+    return cy.get(".color-swatch-footer [data-testid=toggle-show-color-picker-button]")
+  },
+  getColorPicker() {
+    return cy.get(".color-picker-container")
+  },
+  getColorPickerSaturation() {
+    return cy.get(".react-colorful__saturation .react-colorful__interactive")
+  },
+  getColorPickerHue() {
+    return cy.get(".react-colorful__hue .react-colorful__interactive")
+  },
+  getSetColorButton() {
+    return cy.get(".color-picker-footer .set-color-button")
+  },
+  getCancelColorButton() {
+    return cy.get(".color-picker-footer .cancel-button")
+  }
+}

v3/src/components/data-display/components/legend/categorical-legend.tsx

@@ -91,13 +91,16 @@ export const CategoricalLegend = observer(
 
     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)
-        }))
+        const newCategoryData = categoriesRef.current.map((cat: string, index) => {
+          return (
+          {
+            category: cat,
+            color: dataConfiguration?.getLegendColorForCategory(cat) || missingColor,
+            column: index % layoutData.current.numColumns,
+            index,
+            row: Math.floor(index / layoutData.current.numColumns)
+          })
+        })
         categoryData.current = newCategoryData
       }
     }, [dataConfiguration])
@@ -149,7 +152,8 @@ export const CategoricalLegend = observer(
                   return dataConfiguration?.allCasesForCategoryAreSelected(catData[index].category) ??
                     false
                 })
-              .style('fill', (index: number) => catData[index].color || 'white')
+              .style('fill', (index: number) =>
+                                dataConfiguration?.getLegendColorForCategory(catData[index].category) || 'white')
               .transition().duration(duration.current)
               .on('end', () => {
                 duration.current = 0
@@ -327,6 +331,18 @@ export const CategoricalLegend = observer(
       return () => disposer()
     }, [refreshKeys, computeDesiredExtent, dataConfiguration, setupKeys, setDesiredExtent, layerIndex])
 
+    useEffect(function respondToLegendColorChange() {
+      const disposer = reaction(
+        () => {
+          return dataConfiguration?.categorySetForAttrRole('legend')?.colorHash
+        },
+        () => {
+          refreshKeys()
+        }, {fireImmediately: true}
+      )
+      return () => disposer()
+    }, [dataConfiguration, refreshKeys])
+
     useEffect(function setup() {
       if (keysElt.current && categoryData.current) {
         setupKeys()

v3/src/components/data-display/inspector/display-item-format-control.tsx

@@ -1,11 +1,10 @@
-import React, {ReactElement, useRef} from "react"
+import React, {useRef} from "react"
 import {observer} from "mobx-react-lite"
-import {
-  Checkbox, Flex, FormControl, FormLabel, Input, Slider, SliderThumb, SliderTrack
-} from "@chakra-ui/react"
+import {Checkbox, Flex, FormControl, FormLabel, Slider, SliderThumb, SliderTrack
+  } from "@chakra-ui/react"
+import { PointColorSetting } from "./point-color-setting"
 import {IDataConfigurationModel} from "../models/data-configuration-model"
 import {IDisplayItemDescriptionModel} from "../models/display-item-description-model"
-import {missingColor} from "../../../utilities/color-utils"
 import {t} from "../../../utilities/translation/translate"
 
 import "./inspector-panel.scss"
@@ -36,21 +35,33 @@ export const DisplayItemFormatControl = observer(function PointFormatControl(pro
     },  {
       undoStringKey: "DG.Undo.graph.changePointColor",
       redoStringKey: "DG.Redo.graph.changePointColor",
-      log: "Changed point color"
+      log: attrType === "categorical" ? "Changed categorical point color" : "Changed point color"
     })
   }
 
-  const catPointColorSettingArr: ReactElement[] = []
-  categoriesRef.current?.forEach(cat => {
-    catPointColorSettingArr.push(
-      <Flex direction="row" key={cat} className="palette-row color-picker-row cat-color-picker">
-        <FormLabel className="form-label color-picker">{cat}</FormLabel>
-        <Input type="color" className="color-picker-thumb categorical"
-               value={dataConfiguration?.getLegendColorForCategory(cat) || missingColor}
-               onChange={e => handlePointColorChange(e.target.value)}/>
-      </Flex>
+  const handleCatPointColorChange = (color: string, cat: string) => {
+    displayItemDescription.applyModelChange(
+      () => {
+        dataConfiguration.setLegendColorForCategory(cat, color)
+      },
+      {
+        undoStringKey: "DG.Undo.graph.changePointColor",
+        redoStringKey: "DG.Redo.graph.changePointColor",
+        log: "Changed categorical point color"
+      }
     )
-  })
+  }
+
+  const handlePointStrokeColorChange = (color: string) => {
+    displayItemDescription.applyModelChange(
+      () => displayItemDescription.setPointStrokeColor(color),
+      {
+        undoStringKey: "DG.Undo.graph.changeStrokeColor",
+        redoStringKey: "DG.Redo.graph.changeStrokeColor",
+        log: "Changed stroke color"
+      }
+    )
+  }
 
   const renderPlotControlsIfAny = () => {
     if (onBackgroundTransparencyChange && onBackgroundColorChange) {
@@ -59,8 +70,9 @@ export const DisplayItemFormatControl = observer(function PointFormatControl(pro
           <FormControl isDisabled={isTransparent}>
             <Flex className="palette-row color-picker-row">
               <FormLabel className="form-label color-picker">{t("DG.Inspector.backgroundColor")}</FormLabel>
-              <Input type="color" className="color-picker-thumb" value={plotBackgroundColor}
-                     onChange={e => onBackgroundColorChange(e.target.value)}/>
+              <PointColorSetting propertyLabel={t("DG.Inspector.backgroundColor")}
+                                onColorChange={(color) => onBackgroundColorChange(color)}
+                                swatchBackgroundColor={plotBackgroundColor ?? "#FFFFFF"}/>
             </Flex>
           </FormControl>
           <FormControl>
@@ -111,53 +123,47 @@ export const DisplayItemFormatControl = observer(function PointFormatControl(pro
 
       <FormControl isDisabled={displayItemDescription.pointStrokeSameAsFill} className="palette-form-control">
         <Flex className="palette-row color-picker-row">
-          <FormLabel className="form-label color-picker stroke">{t("DG.Inspector.stroke")}</FormLabel>
-          <Input type="color" className="color-picker-thumb" value={displayItemDescription.pointStrokeColor}
-                 onChange={(e) => {
-                   displayItemDescription.applyModelChange(
-                     () => displayItemDescription.setPointStrokeColor(e.target.value),
-                     {
-                       undoStringKey: "DG.Undo.graph.changeStrokeColor",
-                       redoStringKey: "DG.Redo.graph.changeStrokeColor",
-                       log: "Changed stroke color"
-                     }
-                   )
-                 }}/>
+          <FormLabel className="form-label color-picker">{t("DG.Inspector.stroke")}</FormLabel>
+          <PointColorSetting propertyLabel={t("DG.Inspector.stroke")}
+                            onColorChange={(color)=>handlePointStrokeColorChange(color)}
+                            swatchBackgroundColor={displayItemDescription.pointStrokeColor}/>
         </Flex>
         <>
-          { /*todo: The legend color controls are not in place yet*/ }
           {dataConfiguration.attributeID("legend") &&
-          attrType === "categorical"
-            ? <FormControl className="cat-color-setting">{catPointColorSettingArr}</FormControl>
-            : attrType === "numeric"
-              ?(
-              <FormControl className="num-color-setting">
+            attrType === "categorical"
+              ? <FormControl className="cat-color-setting">
+                {categoriesRef.current?.map(category => {
+                  return (
+                    <Flex direction="row" key={category} className="palette-row color-picker-row cat-color-picker">
+                      <FormLabel className="form-label color-picker">{category}</FormLabel>
+                      <PointColorSetting key={category} propertyLabel={category}
+                        onColorChange={(color) => handleCatPointColorChange(color, category)}
+                        swatchBackgroundColor={dataConfiguration.getLegendColorForCategory(category)}/>
+                    </Flex>
+                  )
+                })}
+                </FormControl>
+              : attrType === "numeric"
+                ? <FormControl className="num-color-setting">
+                    <Flex className="palette-row color-picker-row">
+                      <FormLabel className="form-label color-picker">{t("DG.Inspector.legendColor")}</FormLabel>
+                      {/* Sets the min and max colors for numeric legend. Currently not implemented so
+                                    this sets the same color for all the points*/}
+                      <PointColorSetting propertyLabel={t("DG.Inspector.legendColor")}
+                                        onColorChange={(color) => handlePointColorChange(color)}
+                                        swatchBackgroundColor={displayItemDescription.pointColor}/>
+                      <PointColorSetting propertyLabel={t("DG.Inspector.legendColor")}
+                                        onColorChange={(color) => handlePointColorChange(color)}
+                                        swatchBackgroundColor={displayItemDescription.pointColor}/>
+                    </Flex>
+                  </FormControl>
+                :(
                 <Flex className="palette-row color-picker-row">
-                  {/* Sets the min and max colors for numeric legend. Currently not implemented so
-                                this sets the same color for all the points*/}
-                  <FormLabel className="form-label color-picker legend">{t("DG.Inspector.legendColor")}</FormLabel>
-                  <Input type="color" className="color-picker-thumb" value={missingColor}
-                         onChange={e => displayItemDescription.setPointColor(e.target.value)}/>
-                  <Input type="color" className="color-picker-thumb" value={missingColor}
-                         onChange={e => displayItemDescription.setPointColor(e.target.value)}/>
-                </Flex>
-              </FormControl>)
-              :(
-              <Flex className="palette-row color-picker-row">
-                <FormLabel className="form-label color-picker color">{t("DG.Inspector.color")}</FormLabel>
-                <Input type="color" className="color-picker-thumb"
-                       value={displayItemDescription.pointColor}
-                       onChange={e => {
-                         displayItemDescription.applyModelChange(
-                           () => displayItemDescription.setPointColor(e.target.value),
-                           {
-                             undoStringKey: "DG.Undo.graph.changePointColor",
-                             redoStringKey: "DG.Redo.graph.changePointColor",
-                             log: attrType === "categorical" ? "Changed categorical point color" : "Changed point color"
-                           }
-                         )
-                       }}/>
-              </Flex>)
+                  <FormLabel className="form-label color-picker">{t("DG.Inspector.color")}</FormLabel>
+                  <PointColorSetting propertyLabel={t("DG.Inspector.color")}
+                                    onColorChange={(color) => handlePointColorChange(color)}
+                                    swatchBackgroundColor={displayItemDescription.pointColor}/>
+                </Flex>)
           }
         </>
       </FormControl>