fix: harden favorite row picker

src/content/market/favorite-row-picker.ts

@@ -2,6 +2,10 @@ import type { FavoriteCreatorInput, FavoriteFolder } from "./favorites-store";
 
 const ACTION_SELECTOR = 'button[data-sces-favorite-row-action="button"]';
 const PICKER_ROOT_SELECTOR = '[data-sces-favorite-row-picker="root"]';
+const PICKER_ROOT_ID = "sces-favorite-row-picker";
+const PICKER_VIEWPORT_PADDING = 8;
+const PICKER_FALLBACK_HEIGHT = 120;
+const PICKER_FALLBACK_WIDTH = 240;
 
 export interface FavoriteRowPickerRow extends FavoriteCreatorInput {
   actionCell?: HTMLElement;
@@ -28,6 +32,7 @@ export function syncFavoriteRowPickers(options: {
 
     const button = ensureFavoriteActionButton(row.actionCell);
     const authorId = row.authorId.trim();
+    const pendingOperation = state.pendingOperations.get(button);
     const context: FavoritePickerContext = {
       button,
       creator: {
@@ -38,15 +43,22 @@ export function syncFavoriteRowPickers(options: {
       folders: [...options.folders],
       onCreateFolder: options.onCreateFolder,
       onSetCreatorFolderIds: options.onSetCreatorFolderIds,
-      pending: false,
       selectedFolderIds: new Set(
-        authorId ? options.folderIdsByAuthorId.get(authorId) ?? [] : []
+        pendingOperation
+          ? pendingOperation.confirmedFolderIds
+          : authorId
+            ? options.folderIdsByAuthorId.get(authorId) ?? []
+            : []
       )
     };
 
     currentButtons.add(button);
     state.contextByButton.set(button, context);
-    syncFavoriteActionButton(context);
+    syncFavoriteActionButton(
+      context,
+      Boolean(pendingOperation),
+      state.openPicker?.button === button
+    );
     button.onclick = () => {
       openFavoritePicker(options.document, state, button);
     };
@@ -57,6 +69,7 @@ export function syncFavoriteRowPickers(options: {
       continue;
     }
     state.contextByButton.delete(button);
+    state.pendingOperations.delete(button);
     button.onclick = null;
   }
 
@@ -83,10 +96,13 @@ type FavoritePickerContext = {
     creator: FavoriteCreatorInput,
     folderIds: string[]
   ) => Promise<void>;
-  pending: boolean;
   selectedFolderIds: Set<string>;
 };
 
+type FavoritePickerOperation = {
+  confirmedFolderIds: Set<string>;
+};
+
 type OpenFavoritePicker = {
   button: HTMLButtonElement;
   root: HTMLElement;
@@ -97,6 +113,7 @@ type FavoritePickerDocumentState = {
   onDocumentKeydown: (event: KeyboardEvent) => void;
   onDocumentPointerDown: (event: PointerEvent) => void;
   openPicker: OpenFavoritePicker | null;
+  pendingOperations: Map<HTMLButtonElement, FavoritePickerOperation>;
 };
 
 const pickerDocumentStates = new WeakMap<Document, FavoritePickerDocumentState>();
@@ -111,7 +128,8 @@ function getPickerDocumentState(document: Document): FavoritePickerDocumentState
     contextByButton: new Map<HTMLButtonElement, FavoritePickerContext>(),
     onDocumentKeydown: () => {},
     onDocumentPointerDown: () => {},
-    openPicker: null
+    openPicker: null,
+    pendingOperations: new Map<HTMLButtonElement, FavoritePickerOperation>()
   };
   state.onDocumentKeydown = (event) => {
     if (event.key === "Escape") {
@@ -156,7 +174,11 @@ function ensureFavoriteActionButton(actionCell: HTMLElement): HTMLButtonElement
   return button;
 }
 
-function syncFavoriteActionButton(context: FavoritePickerContext): void {
+function syncFavoriteActionButton(
+  context: FavoritePickerContext,
+  isPending = false,
+  isExpanded = false
+): void {
   const authorId = context.creator.authorId.trim();
   const count = context.selectedFolderIds.size;
   const isSaved = count > 0;
@@ -166,6 +188,9 @@ function syncFavoriteActionButton(context: FavoritePickerContext): void {
     ? "bookmark-filled"
     : "bookmark-outline";
   context.button.dataset.scesFavoriteState = isSaved ? "saved" : "empty";
+  context.button.setAttribute("aria-controls", PICKER_ROOT_ID);
+  context.button.setAttribute("aria-expanded", String(isExpanded));
+  context.button.setAttribute("aria-haspopup", "dialog");
 
   if (!authorId) {
     context.button.disabled = true;
@@ -175,7 +200,7 @@ function syncFavoriteActionButton(context: FavoritePickerContext): void {
     return;
   }
 
-  context.button.disabled = false;
+  context.button.disabled = isPending;
   context.button.title = isSaved ? `已收藏至 ${count} 个收藏夹` : "加入收藏夹";
   context.button.setAttribute("aria-label", context.button.title);
   context.button.textContent = isSaved ? "已收藏" : "收藏";
@@ -197,20 +222,22 @@ function openFavoritePicker(
 
     const root = document.createElement("div");
     root.dataset.scesFavoriteRowPicker = "root";
+    root.id = PICKER_ROOT_ID;
     root.setAttribute("role", "dialog");
     root.setAttribute("aria-label", "选择收藏夹");
     root.style.position = "fixed";
     root.style.zIndex = "2147483647";
-    const rect = button.getBoundingClientRect();
-    root.style.left = `${Math.round(rect.left)}px`;
-    root.style.top = `${Math.round(rect.bottom + 4)}px`;
     (document.body ?? document.documentElement).appendChild(root);
     state.openPicker = { button, root };
+    button.setAttribute("aria-expanded", "true");
     document.addEventListener("pointerdown", state.onDocumentPointerDown);
     document.addEventListener("keydown", state.onDocumentKeydown);
   }
 
   renderFavoritePicker(document, state, button);
+  if (state.openPicker?.button === button) {
+    focusFavoritePickerControl(state.openPicker.root);
+  }
 }
 
 function closeFavoritePicker(
@@ -226,6 +253,10 @@ function closeFavoritePicker(
   openPicker.root.remove();
   document.removeEventListener("pointerdown", state.onDocumentPointerDown);
   document.removeEventListener("keydown", state.onDocumentKeydown);
+  openPicker.button.setAttribute("aria-expanded", "false");
+  if (openPicker.button.isConnected) {
+    openPicker.button.focus();
+  }
 }
 
 function removeFavoritePickerRoots(document: Document): void {
@@ -244,6 +275,7 @@ function renderFavoritePicker(
   }
 
   const root = openPicker.root;
+  const isPending = state.pendingOperations.has(button);
   const folderList = document.createElement("div");
   context.folders.forEach((folder) => {
     const label = document.createElement("label");
@@ -251,11 +283,10 @@ function renderFavoritePicker(
     checkbox.type = "checkbox";
     checkbox.dataset.scesFavoriteFolderId = folder.id;
     checkbox.checked = context.selectedFolderIds.has(folder.id);
-    checkbox.disabled = context.pending;
+    checkbox.disabled = isPending;
     checkbox.addEventListener("change", () => {
       const nextFolderIds = readCheckedFolderIds(root, context);
-      renderFavoritePicker(document, state, button);
+      beginFavoriteFolderSave(document, state, context, nextFolderIds);
-      void saveFavoriteFolderIds(document, state, context, nextFolderIds);
     });
 
     label.append(checkbox, document.createTextNode(folder.name));
@@ -266,12 +297,13 @@ function renderFavoritePicker(
   createButton.type = "button";
   createButton.dataset.scesFavoriteRowPicker = "create-folder";
   createButton.textContent = "新建收藏夹";
-  createButton.disabled = context.pending;
+  createButton.disabled = isPending;
   createButton.addEventListener("click", () => {
     void createAndSaveFavoriteFolder(document, state, context);
   });
 
   root.replaceChildren(folderList, createButton);
+  positionFavoritePicker(document, root, button);
 }
 
 function readCheckedFolderIds(
@@ -293,32 +325,39 @@ function readCheckedFolderIds(
   return [...selectedFolderIds];
 }
 
-async function saveFavoriteFolderIds(
+function beginFavoriteFolderSave(
   document: Document,
   state: FavoritePickerDocumentState,
   context: FavoritePickerContext,
-  nextFolderIds: string[],
+  nextFolderIds: string[]
-  createdFolder?: FavoriteFolder
+): void {
-): Promise<void> {
+  const operation = beginFavoritePickerOperation(state, context);
-  context.pending = true;
+  syncFavoriteActionButton(context, true, state.openPicker?.button === context.button);
   renderFavoritePicker(document, state, context.button);
 
+  void completeFavoriteFolderSave(
+    document,
+    state,
+    context,
+    operation,
+    nextFolderIds
+  );
+}
+
+async function completeFavoriteFolderSave(
+  document: Document,
+  state: FavoritePickerDocumentState,
+  context: FavoritePickerContext,
+  operation: FavoritePickerOperation,
+  nextFolderIds: string[]
+): Promise<void> {
   try {
     await context.onSetCreatorFolderIds(context.creator, nextFolderIds);
-    if (state.contextByButton.get(context.button) !== context) {
+    finishFavoritePickerOperation(document, state, context.button, operation, {
-      return;
+      folderIds: nextFolderIds
-    }
+    });
-
-    context.selectedFolderIds = new Set(nextFolderIds);
-    if (createdFolder && !context.folders.some((folder) => folder.id === createdFolder.id)) {
-      context.folders = [...context.folders, createdFolder];
-    }
-    syncFavoriteActionButton(context);
   } catch {
-    // The picker keeps the last confirmed selection when persistence fails.
+    finishFavoritePickerOperation(document, state, context.button, operation);
-  } finally {
-    context.pending = false;
-    renderFavoritePicker(document, state, context.button);
   }
 }
 
@@ -327,33 +366,127 @@ async function createAndSaveFavoriteFolder(
   state: FavoritePickerDocumentState,
   context: FavoritePickerContext
 ): Promise<void> {
-  if (context.pending) {
+  if (state.pendingOperations.has(context.button)) {
     return;
   }
 
-  context.pending = true;
+  const operation = beginFavoritePickerOperation(state, context);
+  syncFavoriteActionButton(context, true, state.openPicker?.button === context.button);
   renderFavoritePicker(document, state, context.button);
   try {
     const folder = await context.onCreateFolder();
     if (!folder) {
+      finishFavoritePickerOperation(document, state, context.button, operation);
       return;
     }
 
-    const nextFolderIds = [...context.selectedFolderIds, folder.id];
+    const nextFolderIds = [...new Set([...context.selectedFolderIds, folder.id])];
     await context.onSetCreatorFolderIds(context.creator, nextFolderIds);
-    if (state.contextByButton.get(context.button) !== context) {
+    finishFavoritePickerOperation(document, state, context.button, operation, {
-      return;
+      createdFolder: folder,
-    }
+      folderIds: nextFolderIds
-
+    });
-    context.selectedFolderIds = new Set(nextFolderIds);
-    if (!context.folders.some((existingFolder) => existingFolder.id === folder.id)) {
-      context.folders = [...context.folders, folder];
-    }
-    syncFavoriteActionButton(context);
   } catch {
-    // The picker keeps the last confirmed selection when creating or saving fails.
+    finishFavoritePickerOperation(document, state, context.button, operation);
-  } finally {
-    context.pending = false;
-    renderFavoritePicker(document, state, context.button);
   }
 }
+
+function beginFavoritePickerOperation(
+  state: FavoritePickerDocumentState,
+  context: FavoritePickerContext
+): FavoritePickerOperation {
+  const operation: FavoritePickerOperation = {
+    confirmedFolderIds: new Set(context.selectedFolderIds)
+  };
+  state.pendingOperations.set(context.button, operation);
+  return operation;
+}
+
+function finishFavoritePickerOperation(
+  document: Document,
+  state: FavoritePickerDocumentState,
+  button: HTMLButtonElement,
+  operation: FavoritePickerOperation,
+  result?: {
+    createdFolder?: FavoriteFolder;
+    folderIds: string[];
+  }
+): void {
+  if (state.pendingOperations.get(button) !== operation) {
+    return;
+  }
+
+  state.pendingOperations.delete(button);
+  const currentContext = state.contextByButton.get(button);
+  if (!currentContext || !button.isConnected) {
+    return;
+  }
+
+  if (result) {
+    currentContext.selectedFolderIds = new Set(result.folderIds);
+    if (
+      result.createdFolder &&
+      !currentContext.folders.some((folder) => folder.id === result.createdFolder?.id)
+    ) {
+      currentContext.folders = [...currentContext.folders, result.createdFolder];
+    }
+  }
+
+  syncFavoriteActionButton(
+    currentContext,
+    false,
+    state.openPicker?.button === button
+  );
+  renderFavoritePicker(document, state, button);
+}
+
+function focusFavoritePickerControl(root: HTMLElement): void {
+  const firstCheckbox = Array.from(
+    root.querySelectorAll<HTMLInputElement>("input[data-sces-favorite-folder-id]")
+  ).find((checkbox) => !checkbox.disabled);
+  const createButton = root.querySelector<HTMLButtonElement>(
+    '[data-sces-favorite-row-picker="create-folder"]'
+  );
+  (firstCheckbox ?? (createButton && !createButton.disabled ? createButton : null))?.focus();
+}
+
+function positionFavoritePicker(
+  document: Document,
+  root: HTMLElement,
+  button: HTMLButtonElement
+): void {
+  const triggerRect = button.getBoundingClientRect();
+  const pickerRect = root.getBoundingClientRect();
+  const pickerWidth = pickerRect.width || root.offsetWidth || PICKER_FALLBACK_WIDTH;
+  const pickerHeight = pickerRect.height || root.offsetHeight || PICKER_FALLBACK_HEIGHT;
+  const viewportWidth =
+    document.documentElement.clientWidth ||
+    document.defaultView?.innerWidth ||
+    pickerWidth + PICKER_VIEWPORT_PADDING * 2;
+  const viewportHeight =
+    document.documentElement.clientHeight ||
+    document.defaultView?.innerHeight ||
+    pickerHeight + PICKER_VIEWPORT_PADDING * 2;
+  const maxLeft = Math.max(
+    PICKER_VIEWPORT_PADDING,
+    viewportWidth - pickerWidth - PICKER_VIEWPORT_PADDING
+  );
+  const maxTop = Math.max(
+    PICKER_VIEWPORT_PADDING,
+    viewportHeight - pickerHeight - PICKER_VIEWPORT_PADDING
+  );
+  const belowTop = triggerRect.bottom + PICKER_VIEWPORT_PADDING;
+  const preferredTop =
+    belowTop + pickerHeight > viewportHeight - PICKER_VIEWPORT_PADDING
+      ? triggerRect.top - PICKER_VIEWPORT_PADDING - pickerHeight
+      : belowTop;
+  const left = clamp(triggerRect.left, PICKER_VIEWPORT_PADDING, maxLeft);
+  const top = clamp(preferredTop, PICKER_VIEWPORT_PADDING, maxTop);
+
+  root.style.left = `${Math.round(left)}px`;
+  root.style.top = `${Math.round(top)}px`;
+}
+
+function clamp(value: number, minimum: number, maximum: number): number {
+  return Math.min(Math.max(value, minimum), maximum);
+}

tests/favorite-row-picker.test.ts

@@ -50,6 +50,44 @@ function readPopover(): HTMLElement | null {
   return document.querySelector('[data-sces-favorite-row-picker="root"]');
 }
 
+function createDeferred(): {
+  promise: Promise<void>;
+  resolve: () => void;
+} {
+  let resolve!: () => void;
+  const promise = new Promise<void>((nextResolve) => {
+    resolve = nextResolve;
+  });
+  return { promise, resolve };
+}
+
+function createRect(options: {
+  bottom?: number;
+  height?: number;
+  left?: number;
+  top?: number;
+  width?: number;
+}): DOMRect {
+  const bottom = options.bottom ?? (options.top ?? 0) + (options.height ?? 0);
+  const height = options.height ?? bottom - (options.top ?? 0);
+  const left = options.left ?? 0;
+  const top = options.top ?? bottom - height;
+  const width = options.width ?? 0;
+  return {
+    bottom,
+    height,
+    left,
+    right: left + width,
+    toJSON() {
+      return {};
+    },
+    top,
+    width,
+    x: left,
+    y: top
+  } as DOMRect;
+}
+
 function sync(options: {
   actionCell?: HTMLElement;
   authorId?: string;
@@ -208,6 +246,50 @@ describe("favorite-row-picker", () => {
     expect(readFavoriteButton(actionCell).dataset.scesFavoriteCount).toBe("2");
   });
 
+  test("retains a confirmed save through a resync while it is pending", async () => {
+    const actionCell = createActionCell();
+    const deferredSave = createDeferred();
+    const onSetCreatorFolderIds = vi.fn(() => deferredSave.promise);
+
+    sync({ actionCell, onSetCreatorFolderIds });
+    readFavoriteButton(actionCell).click();
+    const firstCheckbox = document.querySelector(
+      `input[data-sces-favorite-folder-id="${firstFolder.id}"]`
+    ) as HTMLInputElement;
+    firstCheckbox.checked = true;
+    firstCheckbox.dispatchEvent(new Event("change", { bubbles: true }));
+
+    sync({ actionCell, onSetCreatorFolderIds });
+    expect(readFavoriteButton(actionCell).disabled).toBe(true);
+    expect(
+      (
+        document.querySelector(
+          `input[data-sces-favorite-folder-id="${firstFolder.id}"]`
+        ) as HTMLInputElement
+      ).disabled
+    ).toBe(true);
+
+    deferredSave.resolve();
+    await flushAsyncEvents();
+
+    expect(readFavoriteButton(actionCell).disabled).toBe(false);
+    expect(readFavoriteButton(actionCell).dataset.scesFavoriteCount).toBe("1");
+    expect(
+      (
+        document.querySelector(
+          `input[data-sces-favorite-folder-id="${firstFolder.id}"]`
+        ) as HTMLInputElement
+      ).checked
+    ).toBe(true);
+    expect(
+      (
+        document.querySelector(
+          `input[data-sces-favorite-folder-id="${firstFolder.id}"]`
+        ) as HTMLInputElement
+      ).disabled
+    ).toBe(false);
+  });
+
   test("adds a newly created folder to the creator selection", async () => {
     const actionCell = createActionCell();
     const createdFolder = createFolder("folder-3", "重点");
@@ -273,6 +355,90 @@ describe("favorite-row-picker", () => {
     expect(readPopover()).toBeNull();
   });
 
+  test("clamps a bottom-right picker within the viewport", () => {
+    const actionCell = createActionCell();
+    const originalWidth = Object.getOwnPropertyDescriptor(
+      document.documentElement,
+      "clientWidth"
+    );
+    const originalHeight = Object.getOwnPropertyDescriptor(
+      document.documentElement,
+      "clientHeight"
+    );
+    Object.defineProperty(document.documentElement, "clientWidth", {
+      configurable: true,
+      value: 300
+    });
+    Object.defineProperty(document.documentElement, "clientHeight", {
+      configurable: true,
+      value: 200
+    });
+    const rectSpy = vi
+      .spyOn(HTMLElement.prototype, "getBoundingClientRect")
+      .mockImplementation(function mockPickerRect(this: HTMLElement): DOMRect {
+        if (this.dataset.scesFavoriteRowAction === "button") {
+          return createRect({ bottom: 190, height: 20, left: 280, top: 170, width: 20 });
+        }
+        if (this.dataset.scesFavoriteRowPicker === "root") {
+          return createRect({ height: 60, width: 120 });
+        }
+        return createRect({});
+      });
+
+    try {
+      sync({ actionCell });
+      readFavoriteButton(actionCell).click();
+
+      const root = readPopover() as HTMLElement;
+      expect(Number.parseFloat(root.style.left)).toBeGreaterThanOrEqual(8);
+      expect(Number.parseFloat(root.style.left)).toBeLessThanOrEqual(172);
+      expect(Number.parseFloat(root.style.top)).toBeGreaterThanOrEqual(8);
+      expect(Number.parseFloat(root.style.top)).toBeLessThanOrEqual(132);
+      expect(root.style.top).toBe("102px");
+    } finally {
+      rectSpy.mockRestore();
+      if (originalWidth) {
+        Object.defineProperty(document.documentElement, "clientWidth", originalWidth);
+      } else {
+        delete (document.documentElement as { clientWidth?: number }).clientWidth;
+      }
+      if (originalHeight) {
+        Object.defineProperty(document.documentElement, "clientHeight", originalHeight);
+      } else {
+        delete (document.documentElement as { clientHeight?: number }).clientHeight;
+      }
+    }
+  });
+
+  test("sets trigger dialog state, focuses the picker, and restores focus on Escape", () => {
+    const actionCell = createActionCell();
+    sync({ actionCell });
+
+    const button = readFavoriteButton(actionCell);
+    expect(button.getAttribute("aria-haspopup")).toBe("dialog");
+    expect(button.getAttribute("aria-expanded")).toBe("false");
+    const pickerId = button.getAttribute("aria-controls");
+    expect(pickerId).not.toBeNull();
+
+    button.focus();
+    button.click();
+    const root = readPopover() as HTMLElement;
+    expect(root.id).toBe(pickerId);
+    expect(root.getAttribute("role")).toBe("dialog");
+    expect(root.getAttribute("aria-label")).toBe("选择收藏夹");
+    expect(button.getAttribute("aria-expanded")).toBe("true");
+    expect(document.activeElement).toBe(
+      document.querySelector(
+        `input[data-sces-favorite-folder-id="${firstFolder.id}"]`
+      )
+    );
+
+    document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" }));
+    expect(readPopover()).toBeNull();
+    expect(button.getAttribute("aria-expanded")).toBe("false");
+    expect(document.activeElement).toBe(button);
+  });
+
   test("resyncs one control with fresh callbacks and closes an obsolete popover", async () => {
     const actionCell = createActionCell();
     const staleCallback = vi.fn(async () => {});