diff --git a/quadratic-client/src/app/atoms/borderMenuAtom.ts b/quadratic-client/src/app/atoms/borderMenuAtom.ts new file mode 100644 index 0000000000..15b18f56c6 --- /dev/null +++ b/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); + }; + }, + ], +}); diff --git a/quadratic-client/src/app/ui/menus/TopBar/SubMenus/FormatMenu/useGetBorderMenu.tsx b/quadratic-client/src/app/ui/menus/TopBar/SubMenus/FormatMenu/useGetBorderMenu.tsx index d079350d50..5ef1a5169e 100644 --- a/quadratic-client/src/app/ui/menus/TopBar/SubMenus/FormatMenu/useGetBorderMenu.tsx +++ b/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,96 +16,42 @@ 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(); - const [borderSelection, setBorderSelection] = useState(); - const defaultColor = convertTintToString(colors.defaultBorderColor); - const [color, setColor] = useState(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 ( -
{ - if (!props.disabled) { - setBorderSelection(props.type); - handleChangeBorders(props.type, color); - } - }} - > - - {props.label} - -
- ); - }; - - const cursor = sheets.sheet.cursor; - if ((cursor.multiCursor && cursor.multiCursor.length > 1) || cursor.columnRow !== undefined) { + if (disabled) { return null; } @@ -111,44 +59,75 @@ export function useGetBorderMenu(): JSX.Element | null {
- } /> + } + onClick={handleChangeBorderSelection} + /> } - disabled={!multiCursor} + onClick={handleChangeBorderSelection} + /> + } + onClick={handleChangeBorderSelection} /> - } /> } - disabled={!multiCursor} + onClick={handleChangeBorderSelection} /> } - disabled={!multiCursor} + onClick={handleChangeBorderSelection} />
- } /> - } /> - } /> + } + onClick={handleChangeBorderSelection} + /> + } + onClick={handleChangeBorderSelection} + /> + } + onClick={handleChangeBorderSelection} + /> } + onClick={handleChangeBorderSelection} + /> + } + onClick={handleChangeBorderSelection} /> - } />
} + label={} > @@ -157,22 +136,22 @@ export function useGetBorderMenu(): JSX.Element | null { className="borderSubmenu" label={} > - handleChangeBorderType(e)}> + handleChangeBorderLine(e, 'line1')}>
- handleChangeBorderType(e, 'line2')}> + handleChangeBorderLine(e, 'line2')}>
- handleChangeBorderType(e, 'line3')}> + handleChangeBorderLine(e, 'line3')}>
- handleChangeBorderType(e, 'dashed')}> + handleChangeBorderLine(e, 'dashed')}>
- handleChangeBorderType(e, 'dotted')}> + handleChangeBorderLine(e, 'dotted')}>
- handleChangeBorderType(e, 'double')}> + handleChangeBorderLine(e, 'double')}>
@@ -180,3 +159,25 @@ export function useGetBorderMenu(): JSX.Element | null {
); } + +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 ( +
{ + if (!disabled) onClick(type); + }} + > + + {label} + +
+ ); +} diff --git a/quadratic-client/src/app/ui/menus/TopBar/SubMenus/useBorders.ts b/quadratic-client/src/app/ui/menus/TopBar/SubMenus/useBorders.ts index ef3e213d1b..8151b4a17e 100644 --- a/quadratic-client/src/app/ui/menus/TopBar/SubMenus/useBorders.ts +++ b/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 => { diff --git a/quadratic-client/src/app/web-workers/quadraticCore/worker/core.ts b/quadratic-client/src/app/web-workers/quadraticCore/worker/core.ts index d262f8105a..0f6b6ec559 100644 --- a/quadratic-client/src/app/web-workers/quadraticCore/worker/core.ts +++ b/quadratic-client/src/app/web-workers/quadraticCore/worker/core.ts @@ -672,14 +672,14 @@ class Core { y: number, width: number, height: number, - border: string, + selection: string, style: string | undefined, cursor: string ) { return new Promise((resolve) => { this.clientQueue.push(() => { if (!this.gridController) throw new Error('Expected gridController to be defined'); - this.gridController.setRegionBorders(sheetId, numbersToRect(x, y, width, height), border, style, cursor); + this.gridController.setRegionBorders(sheetId, numbersToRect(x, y, width, height), selection, style, cursor); resolve(undefined); }); }); diff --git a/quadratic-core/src/controller/operations/borders.rs b/quadratic-core/src/controller/operations/borders.rs index f036854ed7..d8bcc665af 100644 --- a/quadratic-core/src/controller/operations/borders.rs +++ b/quadratic-core/src/controller/operations/borders.rs @@ -19,7 +19,7 @@ impl GridController { let cur_borders = get_rect_borders(sheet, &sheet_rect.into()); let new_borders = generate_borders(sheet, &sheet_rect.into(), selections.clone(), style); let borders = if cur_borders.render_lookup == new_borders.render_lookup { - generate_borders(sheet, &sheet_rect.into(), selections.clone(), None) + generate_borders(sheet, &sheet_rect.into(), selections, None) } else { new_borders }; diff --git a/quadratic-core/src/controller/operations/formats.rs b/quadratic-core/src/controller/operations/formats.rs index 1a0c359f6b..978fd0c4dd 100644 --- a/quadratic-core/src/controller/operations/formats.rs +++ b/quadratic-core/src/controller/operations/formats.rs @@ -3,7 +3,7 @@ use crate::{ controller::GridController, grid::{ formats::{format_update::FormatUpdate, Formats}, - generate_borders, get_rect_borders, BorderSelection, Sheet, + generate_borders, BorderSelection, Sheet, }, selection::Selection, Rect, @@ -12,13 +12,7 @@ use crate::{ impl GridController { pub(crate) fn clear_border_op(sheet: &Sheet, rect: &Rect) -> Operation { let selections = vec![BorderSelection::Clear]; - let cur_borders = get_rect_borders(sheet, rect); - let new_borders = generate_borders(sheet, rect, selections.clone(), None); - let borders = if cur_borders.render_lookup == new_borders.render_lookup { - generate_borders(sheet, rect, selections.clone(), None) - } else { - new_borders - }; + let borders = generate_borders(sheet, rect, selections, None); Operation::SetBorders { sheet_rect: rect.to_sheet_rect(sheet.id), borders, diff --git a/quadratic-core/src/grid/borders/compute_indices.rs b/quadratic-core/src/grid/borders/compute_indices.rs index f22acb3e64..0a43dad8b2 100644 --- a/quadratic-core/src/grid/borders/compute_indices.rs +++ b/quadratic-core/src/grid/borders/compute_indices.rs @@ -3,7 +3,7 @@ use itertools::Itertools; use crate::grid::borders::style::BorderSelection; use crate::Rect; -pub(super) fn vertical(rect: &Rect, selections: Vec) -> Vec { +pub(super) fn vertical(rect: &Rect, selections: &[BorderSelection]) -> Vec { selections .iter() .flat_map(|&selection| vertical_selection(rect, selection)) @@ -26,7 +26,7 @@ fn vertical_selection(rect: &Rect, selection: BorderSelection) -> Vec { } } -pub(super) fn horizontal(rect: &Rect, selections: Vec) -> Vec { +pub(super) fn horizontal(rect: &Rect, selections: &[BorderSelection]) -> Vec { selections .iter() .flat_map(|&selection| horizontal_selection(rect, selection)) @@ -62,36 +62,33 @@ mod tests { let rect = Rect::new_span(Pos { x: 10, y: 20 }, Pos { x: 13, y: 23 }); assert_eq!( - horizontal(&rect, vec![BorderSelection::All]), + horizontal(&rect, &[BorderSelection::All]), vec![20, 21, 22, 23, 24] ); assert_eq!( - horizontal(&rect, vec![BorderSelection::Inner]), + horizontal(&rect, &[BorderSelection::Inner]), vec![21, 22, 23] ); + assert_eq!(horizontal(&rect, &[BorderSelection::Outer]), vec![20, 24]); assert_eq!( - horizontal(&rect, vec![BorderSelection::Outer]), - vec![20, 24] - ); - assert_eq!( - horizontal(&rect, vec![BorderSelection::Horizontal]), + horizontal(&rect, &[BorderSelection::Horizontal]), vec![21, 22, 23] ); - assert!(horizontal(&rect, vec![BorderSelection::Vertical]).is_empty()); - assert!(horizontal(&rect, vec![BorderSelection::Left]).is_empty()); - assert_eq!(horizontal(&rect, vec![BorderSelection::Top]), vec![20]); - assert!(horizontal(&rect, vec![BorderSelection::Right]).is_empty()); - assert_eq!(horizontal(&rect, vec![BorderSelection::Bottom]), vec![24]); + assert!(horizontal(&rect, &[BorderSelection::Vertical]).is_empty()); + assert!(horizontal(&rect, &[BorderSelection::Left]).is_empty()); + assert_eq!(horizontal(&rect, &[BorderSelection::Top]), vec![20]); + assert!(horizontal(&rect, &[BorderSelection::Right]).is_empty()); + assert_eq!(horizontal(&rect, &[BorderSelection::Bottom]), vec![24]); assert_eq!( - horizontal(&rect, vec![BorderSelection::Top, BorderSelection::Bottom]), + horizontal(&rect, &[BorderSelection::Top, BorderSelection::Bottom]), vec![20, 24] ); assert_eq!( - horizontal(&rect, vec![BorderSelection::Bottom, BorderSelection::Top]), + horizontal(&rect, &[BorderSelection::Bottom, BorderSelection::Top]), vec![20, 24] ); - assert!(horizontal(&rect, vec![BorderSelection::Left, BorderSelection::Right]).is_empty()); + assert!(horizontal(&rect, &[BorderSelection::Left, BorderSelection::Right]).is_empty()); } #[test] @@ -100,32 +97,29 @@ mod tests { let rect = Rect::new_span(Pos { x: 10, y: 20 }, Pos { x: 13, y: 23 }); assert_eq!( - vertical(&rect, vec![BorderSelection::All]), + vertical(&rect, &[BorderSelection::All]), vec![10, 11, 12, 13, 14] ); + assert_eq!(vertical(&rect, &[BorderSelection::Inner]), vec![11, 12, 13]); + assert_eq!(vertical(&rect, &[BorderSelection::Outer]), vec![10, 14]); + assert!(vertical(&rect, &[BorderSelection::Horizontal]).is_empty()); assert_eq!( - vertical(&rect, vec![BorderSelection::Inner]), - vec![11, 12, 13] - ); - assert_eq!(vertical(&rect, vec![BorderSelection::Outer]), vec![10, 14]); - assert!(vertical(&rect, vec![BorderSelection::Horizontal]).is_empty()); - assert_eq!( - vertical(&rect, vec![BorderSelection::Vertical]), + vertical(&rect, &[BorderSelection::Vertical]), vec![11, 12, 13] ); - assert_eq!(vertical(&rect, vec![BorderSelection::Left]), vec![10]); - assert!(vertical(&rect, vec![BorderSelection::Top]).is_empty()); - assert_eq!(vertical(&rect, vec![BorderSelection::Right]), vec![14]); - assert!(vertical(&rect, vec![BorderSelection::Bottom]).is_empty()); + assert_eq!(vertical(&rect, &[BorderSelection::Left]), vec![10]); + assert!(vertical(&rect, &[BorderSelection::Top]).is_empty()); + assert_eq!(vertical(&rect, &[BorderSelection::Right]), vec![14]); + assert!(vertical(&rect, &[BorderSelection::Bottom]).is_empty()); - assert!(vertical(&rect, vec![BorderSelection::Top, BorderSelection::Bottom]).is_empty()); + assert!(vertical(&rect, &[BorderSelection::Top, BorderSelection::Bottom]).is_empty()); assert_eq!( - vertical(&rect, vec![BorderSelection::Left, BorderSelection::Right]), + vertical(&rect, &[BorderSelection::Left, BorderSelection::Right]), vec![10, 14] ); assert_eq!( - vertical(&rect, vec![BorderSelection::Right, BorderSelection::Left]), + vertical(&rect, &[BorderSelection::Right, BorderSelection::Left]), vec![10, 14] ); } diff --git a/quadratic-core/src/grid/borders/sheet.rs b/quadratic-core/src/grid/borders/sheet.rs index 40c6eb6ef5..d9137f5e68 100644 --- a/quadratic-core/src/grid/borders/sheet.rs +++ b/quadratic-core/src/grid/borders/sheet.rs @@ -37,8 +37,8 @@ pub fn generate_borders_full( }; for style in styles.iter() { - let horizontal = compute_indices::horizontal(rect, selections.clone()); - let vertical = compute_indices::vertical(rect, selections.clone()); + let horizontal = compute_indices::horizontal(rect, &selections); + let vertical = compute_indices::vertical(rect, &selections); for &horizontal_border_index in &horizontal { let above_index = horizontal_border_index - 1;