From 95d89a751a01c5dcf528fef6d7e5901a72a31f35 Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Thu, 15 May 2025 13:22:26 +0200 Subject: [PATCH] refactor: decouple radio button selection from `.buttonList` wrapper (#9528) * refactor: decouple radio button selection from `.buttonList` * fix --- .../components/ExampleApp.scss | 2 +- .../excalidraw/actions/actionProperties.tsx | 726 +++++++++--------- packages/excalidraw/components/Actions.tsx | 2 +- .../excalidraw/components/ButtonSelect.tsx | 30 - .../components/FontPicker/FontPicker.tsx | 16 +- ...uttonIconSelect.tsx => RadioSelection.tsx} | 7 +- packages/excalidraw/css/styles.scss | 14 +- 7 files changed, 393 insertions(+), 404 deletions(-) delete mode 100644 packages/excalidraw/components/ButtonSelect.tsx rename packages/excalidraw/components/{ButtonIconSelect.tsx => RadioSelection.tsx} (88%) diff --git a/examples/with-script-in-browser/components/ExampleApp.scss b/examples/with-script-in-browser/components/ExampleApp.scss index e41a77ccc..77b921ea8 100644 --- a/examples/with-script-in-browser/components/ExampleApp.scss +++ b/examples/with-script-in-browser/components/ExampleApp.scss @@ -52,7 +52,7 @@ transform: none; } -.excalidraw .panelColumn { +.excalidraw .selected-shape-actions { text-align: left; } diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index c379de7f8..654676e90 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -76,7 +76,7 @@ import type { Scene } from "@excalidraw/element"; import type { CaptureUpdateActionType } from "@excalidraw/element"; import { trackEvent } from "../analytics"; -import { ButtonIconSelect } from "../components/ButtonIconSelect"; +import { RadioSelection } from "../components/RadioSelection"; import { ColorPicker } from "../components/ColorPicker/ColorPicker"; import { FontPicker } from "../components/FontPicker/FontPicker"; import { IconPicker } from "../components/IconPicker"; @@ -421,50 +421,52 @@ export const actionChangeFillStyle = register({ return (
{t("labels.fill")} - element.fillStyle, - (element) => element.hasOwnProperty("fillStyle"), - (hasSelection) => - hasSelection ? null : appState.currentItemFillStyle, - )} - onClick={(value, event) => { - const nextValue = - event.altKey && - value === "hachure" && - selectedElements.every((el) => el.fillStyle === "hachure") - ? "zigzag" - : value; +
+ element.fillStyle, + (element) => element.hasOwnProperty("fillStyle"), + (hasSelection) => + hasSelection ? null : appState.currentItemFillStyle, + )} + onClick={(value, event) => { + const nextValue = + event.altKey && + value === "hachure" && + selectedElements.every((el) => el.fillStyle === "hachure") + ? "zigzag" + : value; - updateData(nextValue); - }} - /> + updateData(nextValue); + }} + /> +
); }, @@ -488,38 +490,40 @@ export const actionChangeStrokeWidth = register({ PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.strokeWidth")} - element.strokeWidth, - (element) => element.hasOwnProperty("strokeWidth"), - (hasSelection) => - hasSelection ? null : appState.currentItemStrokeWidth, - )} - onChange={(value) => updateData(value)} - /> +
+ element.strokeWidth, + (element) => element.hasOwnProperty("strokeWidth"), + (hasSelection) => + hasSelection ? null : appState.currentItemStrokeWidth, + )} + onChange={(value) => updateData(value)} + /> +
), }); @@ -543,35 +547,37 @@ export const actionChangeSloppiness = register({ PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.sloppiness")} - element.roughness, - (element) => element.hasOwnProperty("roughness"), - (hasSelection) => - hasSelection ? null : appState.currentItemRoughness, - )} - onChange={(value) => updateData(value)} - /> +
+ element.roughness, + (element) => element.hasOwnProperty("roughness"), + (hasSelection) => + hasSelection ? null : appState.currentItemRoughness, + )} + onChange={(value) => updateData(value)} + /> +
), }); @@ -594,35 +600,37 @@ export const actionChangeStrokeStyle = register({ PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.strokeStyle")} - element.strokeStyle, - (element) => element.hasOwnProperty("strokeStyle"), - (hasSelection) => - hasSelection ? null : appState.currentItemStrokeStyle, - )} - onChange={(value) => updateData(value)} - /> +
+ element.strokeStyle, + (element) => element.hasOwnProperty("strokeStyle"), + (hasSelection) => + hasSelection ? null : appState.currentItemStrokeStyle, + )} + onChange={(value) => updateData(value)} + /> +
), }); @@ -661,63 +669,65 @@ export const actionChangeFontSize = register({ PanelComponent: ({ elements, appState, updateData, app }) => (
{t("labels.fontSize")} - { - if (isTextElement(element)) { - return element.fontSize; - } - const boundTextElement = getBoundTextElement( - element, - app.scene.getNonDeletedElementsMap(), - ); - if (boundTextElement) { - return boundTextElement.fontSize; - } - return null; - }, - (element) => - isTextElement(element) || - getBoundTextElement( - element, - app.scene.getNonDeletedElementsMap(), - ) !== null, - (hasSelection) => - hasSelection - ? null - : appState.currentItemFontSize || DEFAULT_FONT_SIZE, - )} - onChange={(value) => updateData(value)} - /> +
+ { + if (isTextElement(element)) { + return element.fontSize; + } + const boundTextElement = getBoundTextElement( + element, + app.scene.getNonDeletedElementsMap(), + ); + if (boundTextElement) { + return boundTextElement.fontSize; + } + return null; + }, + (element) => + isTextElement(element) || + getBoundTextElement( + element, + app.scene.getNonDeletedElementsMap(), + ) !== null, + (hasSelection) => + hasSelection + ? null + : appState.currentItemFontSize || DEFAULT_FONT_SIZE, + )} + onChange={(value) => updateData(value)} + /> +
), }); @@ -1189,52 +1199,54 @@ export const actionChangeTextAlign = register({ return (
{t("labels.textAlign")} - - group="text-align" - options={[ - { - value: "left", - text: t("labels.left"), - icon: TextAlignLeftIcon, - testId: "align-left", - }, - { - value: "center", - text: t("labels.center"), - icon: TextAlignCenterIcon, - testId: "align-horizontal-center", - }, - { - value: "right", - text: t("labels.right"), - icon: TextAlignRightIcon, - testId: "align-right", - }, - ]} - value={getFormValue( - elements, - app, - (element) => { - if (isTextElement(element)) { - return element.textAlign; - } - const boundTextElement = getBoundTextElement( - element, - elementsMap, - ); - if (boundTextElement) { - return boundTextElement.textAlign; - } - return null; - }, - (element) => - isTextElement(element) || - getBoundTextElement(element, elementsMap) !== null, - (hasSelection) => - hasSelection ? null : appState.currentItemTextAlign, - )} - onChange={(value) => updateData(value)} - /> +
+ + group="text-align" + options={[ + { + value: "left", + text: t("labels.left"), + icon: TextAlignLeftIcon, + testId: "align-left", + }, + { + value: "center", + text: t("labels.center"), + icon: TextAlignCenterIcon, + testId: "align-horizontal-center", + }, + { + value: "right", + text: t("labels.right"), + icon: TextAlignRightIcon, + testId: "align-right", + }, + ]} + value={getFormValue( + elements, + app, + (element) => { + if (isTextElement(element)) { + return element.textAlign; + } + const boundTextElement = getBoundTextElement( + element, + elementsMap, + ); + if (boundTextElement) { + return boundTextElement.textAlign; + } + return null; + }, + (element) => + isTextElement(element) || + getBoundTextElement(element, elementsMap) !== null, + (hasSelection) => + hasSelection ? null : appState.currentItemTextAlign, + )} + onChange={(value) => updateData(value)} + /> +
); }, @@ -1277,54 +1289,56 @@ export const actionChangeVerticalAlign = register({ PanelComponent: ({ elements, appState, updateData, app }) => { return (
- - group="text-align" - options={[ - { - value: VERTICAL_ALIGN.TOP, - text: t("labels.alignTop"), - icon: , - testId: "align-top", - }, - { - value: VERTICAL_ALIGN.MIDDLE, - text: t("labels.centerVertically"), - icon: , - testId: "align-middle", - }, - { - value: VERTICAL_ALIGN.BOTTOM, - text: t("labels.alignBottom"), - icon: , - testId: "align-bottom", - }, - ]} - value={getFormValue( - elements, - app, - (element) => { - if (isTextElement(element) && element.containerId) { - return element.verticalAlign; - } - const boundTextElement = getBoundTextElement( - element, - app.scene.getNonDeletedElementsMap(), - ); - if (boundTextElement) { - return boundTextElement.verticalAlign; - } - return null; - }, - (element) => - isTextElement(element) || - getBoundTextElement( - element, - app.scene.getNonDeletedElementsMap(), - ) !== null, - (hasSelection) => (hasSelection ? null : VERTICAL_ALIGN.MIDDLE), - )} - onChange={(value) => updateData(value)} - /> +
+ + group="text-align" + options={[ + { + value: VERTICAL_ALIGN.TOP, + text: t("labels.alignTop"), + icon: , + testId: "align-top", + }, + { + value: VERTICAL_ALIGN.MIDDLE, + text: t("labels.centerVertically"), + icon: , + testId: "align-middle", + }, + { + value: VERTICAL_ALIGN.BOTTOM, + text: t("labels.alignBottom"), + icon: , + testId: "align-bottom", + }, + ]} + value={getFormValue( + elements, + app, + (element) => { + if (isTextElement(element) && element.containerId) { + return element.verticalAlign; + } + const boundTextElement = getBoundTextElement( + element, + app.scene.getNonDeletedElementsMap(), + ); + if (boundTextElement) { + return boundTextElement.verticalAlign; + } + return null; + }, + (element) => + isTextElement(element) || + getBoundTextElement( + element, + app.scene.getNonDeletedElementsMap(), + ) !== null, + (hasSelection) => (hasSelection ? null : VERTICAL_ALIGN.MIDDLE), + )} + onChange={(value) => updateData(value)} + /> +
); }, @@ -1372,32 +1386,38 @@ export const actionChangeRoundness = register({ return (
{t("labels.edges")} - - hasLegacyRoundness ? null : element.roundness ? "round" : "sharp", - (element) => - !isArrowElement(element) && element.hasOwnProperty("roundness"), - (hasSelection) => - hasSelection ? null : appState.currentItemRoundness, - )} - onChange={(value) => updateData(value)} - /> +
+ + hasLegacyRoundness + ? null + : element.roundness + ? "round" + : "sharp", + (element) => + !isArrowElement(element) && element.hasOwnProperty("roundness"), + (hasSelection) => + hasSelection ? null : appState.currentItemRoundness, + )} + onChange={(value) => updateData(value)} + /> +
); }, @@ -1760,48 +1780,50 @@ export const actionChangeArrowType = register({ return (
{t("labels.arrowtypes")} - { - if (isArrowElement(element)) { - return element.elbowed - ? ARROW_TYPE.elbow - : element.roundness - ? ARROW_TYPE.round - : ARROW_TYPE.sharp; - } +
+ { + if (isArrowElement(element)) { + return element.elbowed + ? ARROW_TYPE.elbow + : element.roundness + ? ARROW_TYPE.round + : ARROW_TYPE.sharp; + } - return null; - }, - (element) => isArrowElement(element), - (hasSelection) => - hasSelection ? null : appState.currentItemArrowType, - )} - onChange={(value) => updateData(value)} - /> + return null; + }, + (element) => isArrowElement(element), + (hasSelection) => + hasSelection ? null : appState.currentItemArrowType, + )} + onChange={(value) => updateData(value)} + /> +
); }, diff --git a/packages/excalidraw/components/Actions.tsx b/packages/excalidraw/components/Actions.tsx index 4f3782048..60dab78f4 100644 --- a/packages/excalidraw/components/Actions.tsx +++ b/packages/excalidraw/components/Actions.tsx @@ -154,7 +154,7 @@ export const SelectedShapeActions = ({ !isSingleElementBoundContainer && alignActionsPredicate(appState, app); return ( -
+
{canChangeStrokeColor(appState, targetElements) && renderAction("changeStrokeColor")} diff --git a/packages/excalidraw/components/ButtonSelect.tsx b/packages/excalidraw/components/ButtonSelect.tsx deleted file mode 100644 index c47ff65e7..000000000 --- a/packages/excalidraw/components/ButtonSelect.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import clsx from "clsx"; - -export const ButtonSelect = ({ - options, - value, - onChange, - group, -}: { - options: { value: T; text: string }[]; - value: T | null; - onChange: (value: T) => void; - group: string; -}) => ( -
- {options.map((option) => ( - - ))} -
-); diff --git a/packages/excalidraw/components/FontPicker/FontPicker.tsx b/packages/excalidraw/components/FontPicker/FontPicker.tsx index 546e1fa34..118c6fac3 100644 --- a/packages/excalidraw/components/FontPicker/FontPicker.tsx +++ b/packages/excalidraw/components/FontPicker/FontPicker.tsx @@ -6,7 +6,7 @@ import { FONT_FAMILY } from "@excalidraw/common"; import type { FontFamilyValues } from "@excalidraw/element/types"; import { t } from "../../i18n"; -import { ButtonIconSelect } from "../ButtonIconSelect"; +import { RadioSelection } from "../RadioSelection"; import { ButtonSeparator } from "../ButtonSeparator"; import { FontFamilyCodeIcon, @@ -82,12 +82,14 @@ export const FontPicker = React.memo( return (
- - type="button" - options={defaultFonts} - value={selectedFontFamily} - onClick={onSelectCallback} - /> +
+ + type="button" + options={defaultFonts} + value={selectedFontFamily} + onClick={onSelectCallback} + /> +
diff --git a/packages/excalidraw/components/ButtonIconSelect.tsx b/packages/excalidraw/components/RadioSelection.tsx similarity index 88% rename from packages/excalidraw/components/ButtonIconSelect.tsx rename to packages/excalidraw/components/RadioSelection.tsx index 45665e4ca..9cdc47e48 100644 --- a/packages/excalidraw/components/ButtonIconSelect.tsx +++ b/packages/excalidraw/components/RadioSelection.tsx @@ -4,8 +4,7 @@ import { ButtonIcon } from "./ButtonIcon"; import type { JSX } from "react"; -// TODO: It might be "clever" to add option.icon to the existing component -export const ButtonIconSelect = ( +export const RadioSelection = ( props: { options: { value: T; @@ -28,7 +27,7 @@ export const ButtonIconSelect = ( } ), ) => ( -
+ <> {props.options.map((option) => props.type === "button" ? ( ( ), )} -
+ ); diff --git a/packages/excalidraw/css/styles.scss b/packages/excalidraw/css/styles.scss index 6f1d9cd48..f8486083e 100644 --- a/packages/excalidraw/css/styles.scss +++ b/packages/excalidraw/css/styles.scss @@ -140,7 +140,7 @@ body.excalidraw-cursor-resize * { justify-content: space-between; } - .panelColumn { + .selected-shape-actions { display: flex; flex-direction: column; row-gap: 0.75rem; @@ -245,10 +245,6 @@ body.excalidraw-cursor-resize * { left: 0; right: 0; --bar-padding: calc(4 * var(--space-factor)); - padding-top: #{"max(var(--bar-padding), var(--sat,0))"}; - padding-right: var(--sar, 0); - padding-bottom: var(--sab, 0); - padding-left: var(--sal, 0); z-index: 4; display: flex; align-items: flex-end; @@ -263,10 +259,6 @@ body.excalidraw-cursor-resize * { display: flex; flex-direction: column; pointer-events: var(--ui-pointerEvents); - - .panelColumn { - padding: 8px 8px 0 8px; - } } } @@ -302,6 +294,10 @@ body.excalidraw-cursor-resize * { overflow-y: auto; box-sizing: border-box; margin-bottom: var(--bar-padding); + + .selected-shape-actions { + padding: 8px 8px 0 8px; + } } .App-menu {