feat: leave Escape to a confirm or modal opened above an open dropdown in useClickOutside, drop ImageMenu's duplicate Escape guard, and restore the useDismissAdvisorFinding and useDismissMailboxImport mutation names the hook rename caught

This commit is contained in:
Matthew Meszaros
2026-10-01 10:14:55 -07:00
parent 76f43b9796
commit 8faefb88be
7 changed files with 32 additions and 20 deletions
@@ -39,7 +39,7 @@ import {
import { useConfirm } from "@/hooks/context/confirm";
import {
useAdvisorFeedback,
useClickOutsideAdvisorFinding,
useDismissAdvisorFinding,
useSnoozeAdvisorFinding,
useUndoAdvisorFinding,
} from "@/lib/api/hooks/app/advisor/useAdvisor";
@@ -63,7 +63,7 @@ export default function AdvisorCard({ finding, onFix, compact = false, defaultOp
const confirm = useConfirm();
const snooze = useSnoozeAdvisorFinding();
const dismiss = useClickOutsideAdvisorFinding();
const dismiss = useDismissAdvisorFinding();
const undo = useUndoAdvisorFinding();
const feedback = useAdvisorFeedback();
@@ -59,20 +59,6 @@ export function ImageMenu({ editor }: { editor: Editor }) {
const del = useDeleteEmailImage();
const confirm = useConfirm();
React.useEffect(() => {
if (!open) return;
const onKey = (e: KeyboardEvent) => {
if (e.key !== "Escape") return;
// The delete confirmation sits above this menu, so Escape belongs
// to it first: only the innermost layer closes.
if (document.querySelector("[role='alertdialog']")) return;
e.stopPropagation();
setOpen(false);
};
document.addEventListener("keydown", onKey, true);
return () => document.removeEventListener("keydown", onKey, true);
}, [open]);
const pick = async (files: FileList | File[] | null) => {
const file = Array.from(files ?? [])[0];
if (!file) return;
@@ -11,7 +11,7 @@ import {
PopoverMenuTrigger,
} from "@/components/ui/popover-menu";
import useMailboxImports from "@/lib/api/hooks/app/emails/useMailboxImports";
import { useClickOutsideMailboxImport } from "@/lib/api/hooks/app/emails/useMailboxImportActions";
import { useDismissMailboxImport } from "@/lib/api/hooks/app/emails/useMailboxImportActions";
import { type MailboxImport } from "@/lib/api/models/app/emails/MailboxImport";
import { vendorLabel } from "@/lib/api/models/app/emails/MailboxSources";
import { useConfirm } from "@/hooks/context/confirm";
@@ -143,7 +143,7 @@ export default function MailboxImportsMenu() {
function ImportEntry({ job, onOpen }: { job: MailboxImport; onOpen: () => void }) {
const confirm = useConfirm();
const dismiss = useClickOutsideMailboxImport();
const dismiss = useDismissMailboxImport();
const st = jobState(job);
const live = job.status === "running";
+22
View File
@@ -85,6 +85,28 @@ describe("useClickOutside", () => {
document.removeEventListener("keydown", onKey);
});
it("leaves Escape to a confirm opened above it, and still takes it inside a modal", () => {
const { unmount } = render(
<>
<Drop name="A" />
<div role="alertdialog" />
</>,
);
toggle("A");
fireEvent.keyDown(document.body, { key: "Escape" });
expect(isOpen("A")).toBe(true);
unmount();
render(
<div aria-modal="true">
<Drop name="B" />
</div>,
);
toggle("B");
fireEvent.keyDown(document.body, { key: "Escape" });
expect(isOpen("B")).toBe(false);
});
it("closes the open dropdown when another one opens", () => {
render(
<>
+4
View File
@@ -60,6 +60,10 @@ export default function useClickOutside(open: boolean, onClose: () => void, insi
// drawer holding it stays open. Focus inside goes back to where it was opened.
const onKey = (e: KeyboardEvent) => {
if (e.key !== "Escape" || layers[layers.length - 1] !== self) return;
// A modal opened above this layer (the confirm, a dialog) owns Escape first.
const modals = document.querySelectorAll('[role="alertdialog"], [aria-modal="true"]');
const above = Array.from(modals).some((m) => !refs.current.some((r) => r.current && m.contains(r.current)));
if (above) return;
e.stopPropagation();
const active = document.activeElement;
const origin = refs.current[0]?.current;
@@ -133,7 +133,7 @@ export function useSnoozeAdvisorFinding() {
});
}
export function useClickOutsideAdvisorFinding() {
export function useDismissAdvisorFinding() {
const invalidate = useAdvisorInvalidator();
return useMutation({
mutationFn: ({ id, reason }: { id: string; reason: string }) => dismissAdvisorFinding(id, reason),
@@ -51,7 +51,7 @@ export function useCancelMailboxImport(id: string | null) {
return useJobMutation<void>(id, (i) => cancelMailboxImport(i));
}
export function useClickOutsideMailboxImport() {
export function useDismissMailboxImport() {
const qc = useQueryClient();
return useMutation({
mutationFn: (id: string) => dismissMailboxImport(id),