Merge pull request #1619 from quadratichq/ayush/1388

feat: maintain border state

quadratic-client/src/app/atoms/borderMenuAtom.ts

@@ -0,0 +1,36 @@
+import { events } from '@/app/events/events';
+import { convertTintToString } from '@/app/helpers/convertColor';
+import { BorderSelection, CellBorderLine } from '@/app/quadratic-core-types';
+import { colors } from '@/app/theme/colors';
+import { atom, DefaultValue } from 'recoil';
+
+interface BorderMenuState {
+  selection?: BorderSelection;
+  color: string;
+  line: CellBorderLine;
+}
+
+const defaultBorderMenuState: BorderMenuState = {
+  selection: undefined,
+  color: convertTintToString(colors.defaultBorderColor),
+  line: 'line1',
+};
+
+export const borderMenuAtom = atom({
+  key: 'borderMenuState',
+  default: defaultBorderMenuState,
+  effects: [
+    ({ setSelf }) => {
+      const clearSelection = () => {
+        setSelf((prev) => {
+          if (prev instanceof DefaultValue) return defaultBorderMenuState;
+          return { ...prev, selection: undefined };
+        });
+      };
+      events.on('cursorPosition', clearSelection);
+      return () => {
+        events.off('cursorPosition', clearSelection);
+      };
+    },
+  ],
+});

quadratic-client/src/app/ui/menus/TopBar/SubMenus/FormatMenu/useGetBorderMenu.tsx

@@ -1,5 +1,7 @@
-import { events } from '@/app/events/events';
+import { borderMenuAtom } from '@/app/atoms/borderMenuAtom';
+import { convertReactColorToString } from '@/app/helpers/convertColor';
 import { BorderSelection, CellBorderLine } from '@/app/quadratic-core-types';
+import { QColorPicker } from '@/app/ui/components/qColorPicker';
 import {
   BorderAllIcon,
   BorderBottomIcon,
@@ -14,141 +16,118 @@ import {
   BorderTopIcon,
   BorderVerticalIcon,
 } from '@/app/ui/icons';
+import { useBorders } from '@/app/ui/menus/TopBar/SubMenus/useBorders';
 import { Tooltip } from '@mui/material';
 import { ClickEvent, MenuItem, SubMenu } from '@szhsin/react-menu';
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback } from 'react';
 import { ColorResult } from 'react-color';
-import { sheets } from '../../../../../grid/controller/Sheets';
-import { convertReactColorToString, convertTintToString } from '../../../../../helpers/convertColor';
-import { colors } from '../../../../../theme/colors';
-import { QColorPicker } from '../../../../components/qColorPicker';
-import { ChangeBorder, useBorders } from '../useBorders';
+import { useRecoilValue } from 'recoil';
 import './useGetBorderMenu.css';
 
 export function useGetBorderMenu(): JSX.Element | null {
-  const [lineStyle, setLineStyle] = useState<CellBorderLine | undefined>();
-  const [borderSelection, setBorderSelection] = useState<BorderSelection | undefined>();
-  const defaultColor = convertTintToString(colors.defaultBorderColor);
-  const [color, setColor] = useState<string>(defaultColor);
+  const { color } = useRecoilValue(borderMenuAtom);
+  const { disabled, changeBorders } = useBorders();
 
-  const { changeBorders } = useBorders();
-
-  const [multiCursor, setMultiCursor] = useState(!!sheets.sheet?.cursor.multiCursor);
-  const clearSelection = useCallback(() => {
-    setBorderSelection('clear');
-    setMultiCursor(!!sheets.sheet.cursor.multiCursor);
-  }, []);
-
-  // clear border type when changing selection
-  useEffect(() => {
-    events.on('cursorPosition', clearSelection);
-    return () => {
-      events.off('cursorPosition', clearSelection);
-    };
-  }, [clearSelection]);
-
-  const handleChangeBorders = useCallback(
-    (borderSelection: BorderSelection | undefined, color: string, lineStyle?: CellBorderLine): void => {
-      if (borderSelection === undefined) return;
-      const borders: ChangeBorder = { selection: borderSelection, type: lineStyle };
-      if (color !== defaultColor) borders.color = color;
-      changeBorders(borders);
+  const handleChangeBorderSelection = useCallback(
+    (selection: BorderSelection) => {
+      changeBorders({ selection });
     },
-    [changeBorders, defaultColor]
+    [changeBorders]
   );
 
   const handleChangeBorderColor = useCallback(
-    (change: ColorResult) => {
-      const converted = convertReactColorToString(change);
-      if (converted !== color) {
-        setColor(converted);
-      }
-      handleChangeBorders(borderSelection, converted, lineStyle);
+    (pickedColor: ColorResult) => {
+      const color = convertReactColorToString(pickedColor);
+      changeBorders({ color });
     },
-    [color, setColor, borderSelection, handleChangeBorders, lineStyle]
+    [changeBorders]
   );
 
-  const handleChangeBorderType = useCallback(
-    (e: ClickEvent, change?: CellBorderLine): void => {
+  const handleChangeBorderLine = useCallback(
+    (e: ClickEvent, line: CellBorderLine): void => {
       e.keepOpen = true;
-      if (change !== lineStyle) {
-        setLineStyle(change);
-      }
-      handleChangeBorders(borderSelection, color, change);
+      changeBorders({ line });
     },
-    [lineStyle, setLineStyle, borderSelection, color, handleChangeBorders]
+    [changeBorders]
   );
 
-  const BorderSelectionButton = (props: {
-    type: BorderSelection;
-    label: JSX.Element;
-    disabled?: boolean;
-    title: string;
-  }): JSX.Element => {
-    return (
-      <div
-        className={`borderMenuType ${props.disabled ? 'borderDisabled' : ''}`}
-        onClick={() => {
-          if (!props.disabled) {
-            setBorderSelection(props.type);
-            handleChangeBorders(props.type, color);
-          }
-        }}
-      >
-        <Tooltip title={props.title} arrow disableInteractive>
-          {props.label}
-        </Tooltip>
-      </div>
-    );
-  };
-
-  const cursor = sheets.sheet.cursor;
-  if ((cursor.multiCursor && cursor.multiCursor.length > 1) || cursor.columnRow !== undefined) {
+  if (disabled) {
     return null;
   }
 
   return (
     <div className="borderMenu">
       <div className="borderMenuLines">
         <div className="borderMenuLine">
-          <BorderSelectionButton type={'all'} title="All borders" label={<BorderAllIcon className="h-5 w-5" />} />
+          <BorderSelectionButton
+            type={'all'}
+            title="All borders"
+            label={<BorderAllIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
+          />
           <BorderSelectionButton
             type={'inner'}
             title="Inner borders"
             label={<BorderInnerIcon className="h-5 w-5" />}
-            disabled={!multiCursor}
+            onClick={handleChangeBorderSelection}
+          />
+          <BorderSelectionButton
+            type={'outer'}
+            title="Outer borders"
+            label={<BorderOuterIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
           />
-          <BorderSelectionButton type={'outer'} title="Outer borders" label={<BorderOuterIcon className="h-5 w-5" />} />
           <BorderSelectionButton
             type={'horizontal'}
             title="Horizontal borders"
             label={<BorderHorizontalIcon className="h-5 w-5" />}
-            disabled={!multiCursor}
+            onClick={handleChangeBorderSelection}
           />
           <BorderSelectionButton
             type={'vertical'}
             title="Vertical borders"
             label={<BorderVerticalIcon className="h-5 w-5" />}
-            disabled={!multiCursor}
+            onClick={handleChangeBorderSelection}
           />
         </div>
         <div className="borderMenuLine">
-          <BorderSelectionButton type={'left'} title="Left border" label={<BorderLeftIcon className="h-5 w-5" />} />
-          <BorderSelectionButton type={'top'} title="Top border" label={<BorderTopIcon className="h-5 w-5" />} />
-          <BorderSelectionButton type={'right'} title="Right border" label={<BorderRightIcon className="h-5 w-5" />} />
+          <BorderSelectionButton
+            type={'left'}
+            title="Left border"
+            label={<BorderLeftIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
+          />
+          <BorderSelectionButton
+            type={'top'}
+            title="Top border"
+            label={<BorderTopIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
+          />
+          <BorderSelectionButton
+            type={'right'}
+            title="Right border"
+            label={<BorderRightIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
+          />
           <BorderSelectionButton
             type={'bottom'}
             title="Bottom border"
             label={<BorderBottomIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
+          />
+          <BorderSelectionButton
+            type={'clear'}
+            title="Clear borders"
+            label={<BorderNoneIcon className="h-5 w-5" />}
+            onClick={handleChangeBorderSelection}
           />
-          <BorderSelectionButton type={'clear'} title="Clear borders" label={<BorderNoneIcon className="h-5 w-5" />} />
         </div>
       </div>
       <div className="borderMenuFormatting">
         <SubMenu
           className="borderSubmenu color-picker-submenu"
           id="FillBorderColorMenuID"
-          label={<BorderColorIcon className="mr-1 h-5 w-5"></BorderColorIcon>}
+          label={<BorderColorIcon className="mr-1 h-5 w-5" style={{ color }}></BorderColorIcon>}
         >
           <QColorPicker onChangeComplete={handleChangeBorderColor} />
         </SubMenu>
@@ -157,26 +136,48 @@ export function useGetBorderMenu(): JSX.Element | null {
           className="borderSubmenu"
           label={<BorderStyleIcon className="mr-1 h-5 w-5" />}
         >
-          <MenuItem onClick={(e) => handleChangeBorderType(e)}>
+          <MenuItem onClick={(e) => handleChangeBorderLine(e, 'line1')}>
             <div className="lineStyleBorder normalBorder"></div>
           </MenuItem>
-          <MenuItem onClick={(e) => handleChangeBorderType(e, 'line2')}>
+          <MenuItem onClick={(e) => handleChangeBorderLine(e, 'line2')}>
             <div className="lineStyleBorder doubleBorder"></div>
           </MenuItem>
-          <MenuItem onClick={(e) => handleChangeBorderType(e, 'line3')}>
+          <MenuItem onClick={(e) => handleChangeBorderLine(e, 'line3')}>
             <div className="lineStyleBorder tripleBorder"></div>
           </MenuItem>
-          <MenuItem onClick={(e) => handleChangeBorderType(e, 'dashed')}>
+          <MenuItem onClick={(e) => handleChangeBorderLine(e, 'dashed')}>
             <div className="lineStyleBorder dashedBorder"></div>
           </MenuItem>
-          <MenuItem onClick={(e) => handleChangeBorderType(e, 'dotted')}>
+          <MenuItem onClick={(e) => handleChangeBorderLine(e, 'dotted')}>
             <div className="lineStyleBorder dottedBorder"></div>
           </MenuItem>
-          <MenuItem onClick={(e) => handleChangeBorderType(e, 'double')}>
+          <MenuItem onClick={(e) => handleChangeBorderLine(e, 'double')}>
             <div className="lineStyleBorder twoLineBorder"></div>
           </MenuItem>
         </SubMenu>
       </div>
     </div>
   );
 }
+
+function BorderSelectionButton(props: {
+  type: BorderSelection;
+  title: string;
+  label: JSX.Element;
+  disabled?: boolean;
+  onClick: (type: BorderSelection) => void;
+}): JSX.Element {
+  const { type, title, label, disabled, onClick } = props;
+  return (
+    <div
+      className={`borderMenuType ${disabled ? 'borderDisabled' : ''}`}
+      onClick={() => {
+        if (!disabled) onClick(type);
+      }}
+    >
+      <Tooltip title={title} arrow disableInteractive>
+        {label}
+      </Tooltip>
+    </div>
+  );
+}

quadratic-client/src/app/ui/menus/TopBar/SubMenus/useBorders.ts

@@ -1,16 +1,17 @@
+import { borderMenuAtom } from '@/app/atoms/borderMenuAtom';
 import { events } from '@/app/events/events';
+import { sheets } from '@/app/grid/controller/Sheets';
+import { convertColorStringToTint, convertTintToArray } from '@/app/helpers/convertColor';
 import { BorderSelection, BorderStyle, CellBorderLine } from '@/app/quadratic-core-types';
 import { quadraticCore } from '@/app/web-workers/quadraticCore/quadraticCore';
 import { Rectangle } from 'pixi.js';
 import { useEffect, useState } from 'react';
-import { sheets } from '../../../../grid/controller/Sheets';
-import { convertColorStringToTint, convertTintToArray } from '../../../../helpers/convertColor';
-import { colors } from '../../../../theme/colors';
+import { useSetRecoilState } from 'recoil';
 
 export interface ChangeBorder {
   selection?: BorderSelection;
   color?: string;
-  type?: CellBorderLine;
+  line?: CellBorderLine;
 }
 
 export interface UseBordersResults {
@@ -20,29 +21,38 @@ export interface UseBordersResults {
 }
 
 export const useBorders = (): UseBordersResults => {
+  const setBorderMenuState = useSetRecoilState(borderMenuAtom);
+
   const changeBorders = (options: ChangeBorder): void => {
-    const cursor = sheets.sheet.cursor;
-    if (cursor.multiCursor && cursor.multiCursor.length > 1) {
-      console.log('TODO: implement multiCursor border support');
-      return;
-    }
-    const rectangle = cursor.multiCursor
-      ? cursor.multiCursor[0]
-      : new Rectangle(cursor.cursorPosition.x, cursor.cursorPosition.y, 1, 1);
-    const sheet = sheets.sheet;
-    const colorTint = options.color === undefined ? colors.defaultBorderColor : convertColorStringToTint(options.color);
-    const colorArray = convertTintToArray(colorTint);
-    const selection = options.selection === undefined ? 'all' : options.selection;
-    const style: BorderStyle = {
-      color: {
-        red: Math.floor(colorArray[0] * 255),
-        green: Math.floor(colorArray[1] * 255),
-        blue: Math.floor(colorArray[2] * 255),
-        alpha: 0xff,
-      },
-      line: options.type ?? 'line1',
-    };
-    quadraticCore.setRegionBorders(sheet.id, rectangle, selection, style);
+    setBorderMenuState((prev) => {
+      const selection = options.selection ?? prev.selection;
+      const color = options.color ?? prev.color;
+      const line = options.line ?? prev.line;
+      const cursor = sheets.sheet.cursor;
+      if (cursor.multiCursor && cursor.multiCursor.length > 1) {
+        console.log('TODO: implement multiCursor border support');
+      }
+      // apply border only on selection change, else only update border menu state
+      else if (options.selection !== undefined) {
+        const rectangle = cursor.multiCursor
+          ? cursor.multiCursor[0]
+          : new Rectangle(cursor.cursorPosition.x, cursor.cursorPosition.y, 1, 1);
+        const sheet = sheets.sheet;
+        const colorTint = convertColorStringToTint(color);
+        const colorArray = convertTintToArray(colorTint);
+        const style: BorderStyle = {
+          color: {
+            red: Math.floor(colorArray[0] * 255),
+            green: Math.floor(colorArray[1] * 255),
+            blue: Math.floor(colorArray[2] * 255),
+            alpha: 0xff,
+          },
+          line,
+        };
+        quadraticCore.setRegionBorders(sheet.id, rectangle, options.selection, style);
+      }
+      return { selection, color, line };
+    });
   };
 
   const clearBorders = (): void => {