fix switching between different popovers for texts

This commit is contained in:
Ryan Di
2025-08-28 16:43:42 +10:00
parent ab3e2d7074
commit 8325f4bd3a
5 changed files with 119 additions and 21 deletions

View File

@@ -1172,19 +1172,19 @@ export const actionChangeFontFamily = register({
openPopup: "fontFamily",
});
} else {
// close, use the cache and clear it afterwards
// close immediately to avoid racing with other popovers opening
const data = {
openPopup: null,
currentHoveredFontFamily: null,
cachedElements: new Map(cachedElementsRef.current),
resetAll: true,
} as ChangeFontFamilyData;
if (isUnmounted.current) {
// in case the component was unmounted by the parent, trigger the update directly
updateData({ ...batchedData, ...data });
} else {
setBatchedData(data);
// apply immediately instead of batching
updateData({ ...batchedData, ...data });
setBatchedData({});
}
cachedElementsRef.current.clear();

View File

@@ -1,5 +1,5 @@
import clsx from "clsx";
import { useState } from "react";
import { useState, useEffect } from "react";
import * as Popover from "@radix-ui/react-popover";
import {
@@ -353,6 +353,14 @@ export const CompactShapeActions = ({
const isRTL = document.documentElement.getAttribute("dir") === "rtl";
// Close local popovers any time a global popup (appState.openPopup) opens/switches
useEffect(() => {
if (appState.openPopup) {
setStrokePopoverOpen(false);
setOtherActionsPopoverOpen(false);
}
}, [appState.openPopup]);
return (
<div className="compact-shape-actions">
{/* Stroke Color */}
@@ -394,6 +402,17 @@ export const CompactShapeActions = ({
type="button"
className="compact-action-button"
title={t("labels.stroke")}
onPointerDown={(e) => {
e.preventDefault();
setStrokePopoverOpen((open) => {
const next = !open;
if (next) {
setOtherActionsPopoverOpen(false);
setAppState({ openPopup: null });
}
return next;
});
}}
>
{resizeIcon}
</button>
@@ -453,6 +472,16 @@ export const CompactShapeActions = ({
type="button"
className="compact-action-button"
title={t("labels.arrowtypes")}
onPointerDown={(e) => {
e.preventDefault();
if (appState.openPopup === "arrowProperties") {
setAppState({ openPopup: null });
} else {
setStrokePopoverOpen(false);
setOtherActionsPopoverOpen(false);
setAppState({ openPopup: "arrowProperties" });
}
}}
>
{(() => {
// Show an icon based on the current arrow type
@@ -488,7 +517,11 @@ export const CompactShapeActions = ({
<PropertiesPopover
container={container}
style={{ maxWidth: "13rem" }}
onClose={() => setAppState({ openPopup: null })}
onClose={() => {
if (appState.openPopup === "arrowProperties") {
setAppState({ openPopup: null });
}
}}
>
{renderAction("changeArrowProperties")}
</PropertiesPopover>
@@ -530,6 +563,16 @@ export const CompactShapeActions = ({
type="button"
className="compact-action-button"
title={t("labels.textAlign")}
onPointerDown={(e) => {
e.preventDefault();
if (appState.openPopup === "textAlign") {
setAppState({ openPopup: null });
} else {
setStrokePopoverOpen(false);
setOtherActionsPopoverOpen(false);
setAppState({ openPopup: "textAlign" });
}
}}
>
{TextSizeIcon}
</button>
@@ -539,7 +582,11 @@ export const CompactShapeActions = ({
className={CLASSES.SHAPE_ACTIONS_THEME_SCOPE}
container={container}
style={{ maxWidth: "13rem" }}
onClose={() => setAppState({ openPopup: null })}
onClose={() => {
if (appState.openPopup === "textAlign") {
setAppState({ openPopup: null });
}
}}
>
<div className="selected-shape-actions">
{(appState.activeTool.type === "text" ||
@@ -593,6 +640,17 @@ export const CompactShapeActions = ({
type="button"
className="compact-action-button"
title={t("labels.actions")}
onPointerDown={(e) => {
e.preventDefault();
setOtherActionsPopoverOpen((open) => {
const next = !open;
if (next) {
setStrokePopoverOpen(false);
setAppState({ openPopup: null });
}
return next;
});
}}
>
{settingsPlusIcon}
</button>

View File

@@ -1,6 +1,6 @@
import * as Popover from "@radix-ui/react-popover";
import clsx from "clsx";
import { useRef } from "react";
import { useRef, useEffect } from "react";
import {
COLOR_OUTLINE_CONTRAST_THRESHOLD,
@@ -78,6 +78,7 @@ const ColorPickerPopupContent = ({
elements,
palette = COLOR_PALETTE,
updateData,
getOpenPopup,
}: Pick<
ColorPickerProps,
| "type"
@@ -87,7 +88,9 @@ const ColorPickerPopupContent = ({
| "elements"
| "palette"
| "updateData"
>) => {
> & {
getOpenPopup: () => AppState["openPopup"];
}) => {
const { container } = useExcalidrawContainer();
const [, setActiveColorPickerSection] = useAtom(activeColorPickerSectionAtom);
@@ -132,7 +135,10 @@ const ColorPickerPopupContent = ({
}
}}
onClose={() => {
updateData({ openPopup: null });
// only clear if we're still the active popup (avoid racing with switch)
if (getOpenPopup() === type) {
updateData({ openPopup: null });
}
setActiveColorPickerSection(null);
}}
>
@@ -169,6 +175,7 @@ const ColorPickerPopupContent = ({
if (eyeDropperState) {
setEyeDropperState(null);
} else {
// close explicitly on Escape
updateData({ openPopup: null });
}
}}
@@ -215,7 +222,12 @@ const ColorPickerTrigger = ({
? t("labels.showStroke")
: t("labels.showBackground")
}
onClick={onToggle}
data-openpopup={type}
onPointerDown={(e) => {
// use pointerdown so we run before outside-close logic
e.preventDefault();
onToggle();
}}
>
<div className="color-picker__button-outline">{!color && slashIcon}</div>
{compactMode && color && (
@@ -261,6 +273,10 @@ export const ColorPicker = ({
appState,
compactMode = false,
}: ColorPickerProps) => {
const openRef = useRef(appState.openPopup);
useEffect(() => {
openRef.current = appState.openPopup;
}, [appState.openPopup]);
return (
<div>
<div
@@ -284,8 +300,6 @@ export const ColorPicker = ({
onOpenChange={(open) => {
if (open) {
updateData({ openPopup: type });
} else if (appState.openPopup === type) {
updateData({ openPopup: null });
}
}}
>
@@ -297,10 +311,18 @@ export const ColorPicker = ({
compactMode={compactMode}
mode={type === "elementStroke" ? "stroke" : "background"}
onToggle={() => {
// toggle to this type (if already open, close; if another is open, switch)
updateData({
openPopup: appState.openPopup === type ? null : type,
});
// atomic switch: if another popup is open, close it first, then open this one next tick
if (appState.openPopup === type) {
// toggle off
updateData({ openPopup: null });
} else if (appState.openPopup) {
// switching
updateData({ openPopup: null });
setTimeout(() => updateData({ openPopup: type }), 0);
} else {
// open this one
updateData({ openPopup: type });
}
}}
/>
{/* popup content */}
@@ -313,6 +335,7 @@ export const ColorPicker = ({
elements={elements}
palette={palette}
updateData={updateData}
getOpenPopup={() => openRef.current}
/>
)}
</Popover.Root>

View File

@@ -103,7 +103,18 @@ export const FontPicker = React.memo(
)}
{!compactMode && <ButtonSeparator />}
<Popover.Root open={isOpened} onOpenChange={onPopupChange}>
<FontPickerTrigger selectedFontFamily={selectedFontFamily} />
<FontPickerTrigger
selectedFontFamily={selectedFontFamily}
onTrigger={() => {
if (isOpened) {
onPopupChange(false);
} else {
// switch from any open popup: close then open next tick
onPopupChange(false);
setTimeout(() => onPopupChange(true), 0);
}
}}
/>
{isOpened && (
<FontPickerList
selectedFontFamily={selectedFontFamily}

View File

@@ -11,10 +11,12 @@ import { isDefaultFont } from "./FontPicker";
interface FontPickerTriggerProps {
selectedFontFamily: FontFamilyValues | null;
onTrigger?: (event: React.SyntheticEvent) => void;
}
export const FontPickerTrigger = ({
selectedFontFamily,
onTrigger,
}: FontPickerTriggerProps) => {
const isTriggerActive = useMemo(
() => Boolean(selectedFontFamily && !isDefaultFont(selectedFontFamily)),
@@ -24,7 +26,7 @@ export const FontPickerTrigger = ({
return (
<Popover.Trigger asChild>
{/* Empty div as trigger so it's stretched 100% due to different button sizes */}
<div>
<div data-openpopup="fontFamily">
<ButtonIcon
standalone
icon={TextIcon}
@@ -32,8 +34,12 @@ export const FontPickerTrigger = ({
className="properties-trigger"
testId={"font-family-show-fonts"}
active={isTriggerActive}
// no-op
onClick={() => {}}
onClick={(e) => {
if (onTrigger) {
e.preventDefault();
onTrigger(e);
}
}}
style={{
border: "none",
}}