Merge pull request #37 from codebridger/CU-86exzbh61_Fix-bundle-selector-dropdown-in-save-phrase-modal-wont-close-overflows-wrapper

Fix SelectPhraseBundleV2 dropdown close in modal context

Author: navidshad

src/console-crane/components/SelectPhraseBundleV2.vue

@@ -1,7 +1,8 @@
 <template>
-  <div class="relative w-full">
+  <div ref="rootRef" class="relative w-full">
     <Select v-model="selected" :options="options" multiple custom labelKey="title" valueKey="_id"
-      :placeholder="showSuggestion ? '' : 'Select Phrase Bundles to save...'">
+      :placeholder="showSuggestion ? '' : 'Select Phrase Bundles to save...'" @open="onDropdownOpen"
+      @close="onDropdownClose">
       <template #selected="{
         selectedOption,
         selectedOptions,
@@ -108,6 +109,11 @@ const isCreating = ref(false);
 const searchedBundleName = ref("");
 const options = ref<PhraseBundleType[]>([]);
 
+// Component root — boundary for our own outside-click close (see below).
+const rootRef = ref<HTMLElement | null>(null);
+// Mirrors pilotui Select's internal open state via its open/close events.
+const isDropdownOpen = ref(false);
+
 // In-field suggested bundle (shown only when nothing is selected yet).
 const isEditingSuggested = ref(false);
 const editBuffer = ref("");
@@ -273,19 +279,133 @@ watch(searchedBundleName, () => {
   }, 300); // 300ms debounce
 });
 
+/**
+ * Close the pilotui Select dropdown.
+ *
+ * pilotui's Select owns its open state internally and exposes no close method or
+ * `open` prop — the only outside-driven close it offers is its own document
+ * click handler, which bails whenever the click lands inside ANY `.relative`
+ * ancestor (Select.vue `handleClickOutside`). Inside the ConsoleCrane modal —
+ * where almost everything sits under a Tailwind `relative` wrapper — that guard
+ * matches on nearly every click, so the dropdown effectively never closes.
+ *
+ * We drive pilotui's own close path instead by dispatching the Escape key its
+ * trigger button already handles (`handleKeydown` → `closeDropdown`). It's
+ * idempotent: closing an already-closed dropdown is a no-op, so this can't
+ * accidentally re-open. Used both for our outside-click handler and by
+ * SaveWordSectionV2 after a successful save.
+ */
+function closeDropdown() {
+  const trigger = rootRef.value?.querySelector<HTMLButtonElement>(
+    'button[aria-haspopup="true"]'
+  );
+  trigger?.dispatchEvent(
+    new KeyboardEvent("keydown", { key: "Escape", bubbles: true })
+  );
+}
+
+/**
+ * Close the dropdown when the user clicks anywhere outside this component.
+ * Replaces pilotui's `.relative`-based outside detection, which misfires inside
+ * the modal (see closeDropdown). We only act while open and only for clicks that
+ * land outside our root, so in-dropdown interactions (multi-select toggles,
+ * search, create, chip removal, suggestion edit) are untouched.
+ */
+function handleOutsidePointer(event: Event) {
+  if (!isDropdownOpen.value) return;
+  const root = rootRef.value;
+  if (root && !root.contains(event.target as Node)) {
+    closeDropdown();
+  }
+}
+
+/**
+ * Size and place the open dropdown so it never runs past the modal frame
+ * (ClickUp 86exzbh61 follow-up). The save section sits near the bottom of the
+ * ConsoleCrane modal, so pilotui's fixed downward max-h-96 panel spilled below
+ * the visible frame — forcing a modal scroll on top of the list's own scroll.
+ *
+ * pilotui has no placement API, so on open we measure the trigger against the
+ * nearest scroll frame and set inline styles on its absolutely-positioned panel:
+ * flip it upward when it can't fully open downward and there's more room above,
+ * and cap its height to the available space (minus a gap) in whichever direction
+ * it opens. The list scrolls internally within that cap (see scoped styles).
+ */
+function positionDropdown() {
+  const root = rootRef.value;
+  if (!root) return;
+  const panel = root.querySelector<HTMLElement>('[role="listbox"]');
+  const trigger = root.querySelector<HTMLElement>('button[aria-haspopup="true"]');
+  if (!panel || !trigger) return;
+
+  const GAP = 12; // breathing room between the panel and the frame edge
+  const MAX = 336; // pilotui's max-h-96 (24rem → 336px after the rem→px rewrite)
+
+  const frameEl =
+    root.closest<HTMLElement>(".overflow-y-auto") ?? document.documentElement;
+  const frame = frameEl.getBoundingClientRect();
+  const t = trigger.getBoundingClientRect();
+
+  const below = frame.bottom - t.bottom - GAP;
+  const above = t.top - frame.top - GAP;
+
+  // Flip up only when the list can't fully open downward and there's more room
+  // above; otherwise keep the natural downward placement.
+  const openUp = below < MAX && above > below;
+  const avail = Math.max(0, openUp ? above : below);
+
+  panel.style.maxHeight = `${Math.min(MAX, avail)}px`;
+  if (openUp) {
+    panel.style.top = "auto";
+    panel.style.bottom = "calc(100% + 4px)";
+    panel.style.marginTop = "0";
+  } else {
+    // Clear any prior upward placement (panel is reused across reposition runs).
+    panel.style.top = "";
+    panel.style.bottom = "";
+    panel.style.marginTop = "";
+  }
+}
+
+// Reposition the open dropdown when the viewport or modal body changes size /
+// scrolls; torn down again on close so the listeners only live while open.
+let removeReposition: (() => void) | null = null;
+
+function onDropdownOpen() {
+  isDropdownOpen.value = true;
+  nextTick(positionDropdown);
+
+  if (removeReposition) return;
+  const handler = () => positionDropdown();
+  const frameEl: Window | HTMLElement =
+    rootRef.value?.closest<HTMLElement>(".overflow-y-auto") ?? window;
+  window.addEventListener("resize", handler);
+  frameEl.addEventListener("scroll", handler, { passive: true });
+  removeReposition = () => {
+    window.removeEventListener("resize", handler);
+    frameEl.removeEventListener("scroll", handler);
+  };
+}
+
+function onDropdownClose() {
+  isDropdownOpen.value = false;
+  removeReposition?.();
+  removeReposition = null;
+}
+
 onMounted(() => {
   fetchOptions();
+  document.addEventListener("pointerdown", handleOutsidePointer);
 });
 
 onBeforeUnmount(() => {
   if (searchDebounceTimer) {
     clearTimeout(searchDebounceTimer);
   }
+  document.removeEventListener("pointerdown", handleOutsidePointer);
+  removeReposition?.();
 });
 
-// Expose method for compatibility (Select manages its own open state)
-function closeDropdown() { }
-
 defineExpose({
   closeDropdown,
 });
@@ -324,4 +444,26 @@ defineExpose({
 .relative.w-full :deep(.flex.flex-col > .relative > .relative > button) {
   flex: 1 1 auto;
 }
+
+/*
+  Keep the open dropdown panel inside its own max-height instead of spilling the
+  bundle list out of the modal (ClickUp 86exzbh61). pilotui only gives the
+  option list an internal scroll in `confirm` mode; in the `custom` mode we use
+  here the list container is a plain `flex-1` with no overflow, so a long list
+  grows past the panel's max-height and out of the modal. Make the panel a flex
+  column and let the list region scroll within it.
+
+  Targets pilotui's internal markup (the absolutely-positioned listbox panel and
+  its `flex-1` body); a pilotui nesting change would make this a cosmetic
+  regression, not a functional break.
+*/
+.relative.w-full :deep([role="listbox"]) {
+  display: flex;
+  flex-direction: column;
+}
+
+.relative.w-full :deep([role="listbox"] > .flex-1) {
+  min-height: 0;
+  overflow-y: auto;
+}
 </style>

tests/select-phrase-bundle.test.ts

@@ -0,0 +1,119 @@
+import { describe, it, expect, beforeEach, vi } from "vitest";
+import { mount, flushPromises } from "@vue/test-utils";
+import { createPinia } from "pinia";
+import { nextTick } from "vue";
+import { Select } from "pilotui";
+
+/**
+ * Regression net for ClickUp 86exzbh61: the bundle selector dropdown in the save
+ * modal wouldn't close. pilotui's Select only closes from outside via its own
+ * `.relative`-based document handler, which misfires inside the ConsoleCrane
+ * modal (almost everything sits under a Tailwind `relative` wrapper) and exposes
+ * no close method/prop. SelectPhraseBundleV2 now runs its own outside-click
+ * close and a working `closeDropdown()` (also used by SaveWordSectionV2 after a
+ * save), both of which drive pilotui's own close path by dispatching the Escape
+ * key its trigger button handles.
+ *
+ * These tests assert that mechanism: the Escape keydown reaches the Select's
+ * trigger on an outside click / on closeDropdown(), and is NOT fired for clicks
+ * inside the dropdown (so multi-select toggles keep it open). They deliberately
+ * don't rely on pilotui actually rendering its open panel in happy-dom — that
+ * round trip is covered by the real-browser flow; here we pin our own logic.
+ */
+
+const { BUNDLES } = vi.hoisted(() => ({
+  BUNDLES: Array.from({ length: 14 }, (_, i) => ({
+    _id: `b${i}`,
+    title: `Bundle ${i}`,
+  })),
+}));
+
+vi.mock("@modular-rest/client", () => ({
+  dataProvider: {
+    find: vi.fn().mockResolvedValue(BUNDLES),
+    insertOne: vi.fn().mockResolvedValue({ _id: "new" }),
+  },
+  authentication: { user: { id: "u1" } },
+}));
+
+import SelectPhraseBundleV2 from "../src/console-crane/components/SelectPhraseBundleV2.vue";
+
+async function mountSelector() {
+  const wrapper = mount(SelectPhraseBundleV2, {
+    attachTo: document.body,
+    props: { selectedBundles: [] as string[] },
+    global: { plugins: [createPinia()] },
+  });
+  await flushPromises(); // fetchOptions() resolves
+
+  const triggerEl = wrapper
+    .find('button[aria-haspopup="true"]')
+    .element as HTMLButtonElement;
+
+  // Spy on the Escape keydown our close path dispatches at the trigger — this is
+  // what reaches pilotui's own closeDropdown handler.
+  const escapes: string[] = [];
+  triggerEl.addEventListener("keydown", (e) => {
+    escapes.push((e as KeyboardEvent).key);
+  });
+
+  // Mirror pilotui opening its dropdown (we listen to its `open` event).
+  const openDropdown = () => wrapper.findComponent(Select).vm.$emit("open");
+
+  return { wrapper, triggerEl, escapes, openDropdown };
+}
+
+describe("SelectPhraseBundleV2 dropdown close behaviour", () => {
+  let outside: HTMLElement;
+
+  beforeEach(() => {
+    document.body.innerHTML = "";
+    outside = document.createElement("div");
+    document.body.appendChild(outside);
+  });
+
+  it("closes on an outside click while open", async () => {
+    const { wrapper, escapes, openDropdown } = await mountSelector();
+    openDropdown();
+    await nextTick();
+
+    outside.dispatchEvent(new Event("pointerdown", { bubbles: true }));
+    await nextTick();
+
+    expect(escapes).toContain("Escape");
+    wrapper.unmount();
+  });
+
+  it("does nothing on an outside click while already closed", async () => {
+    const { wrapper, escapes } = await mountSelector();
+    // No openDropdown() — isDropdownOpen stays false.
+    outside.dispatchEvent(new Event("pointerdown", { bubbles: true }));
+    await nextTick();
+
+    expect(escapes).toHaveLength(0);
+    wrapper.unmount();
+  });
+
+  it("stays open when interacting inside the dropdown (multi-select)", async () => {
+    const { wrapper, triggerEl, escapes, openDropdown } = await mountSelector();
+    openDropdown();
+    await nextTick();
+
+    // A pointer press inside our root (e.g. toggling an option) must not be
+    // treated as an outside click.
+    triggerEl.dispatchEvent(new Event("pointerdown", { bubbles: true }));
+    await nextTick();
+
+    expect(escapes).toHaveLength(0);
+    wrapper.unmount();
+  });
+
+  it("exposes a working closeDropdown() for the post-save path", async () => {
+    const { wrapper, escapes } = await mountSelector();
+    (wrapper.vm as unknown as { closeDropdown: () => void }).closeDropdown();
+    await nextTick();
+
+    expect(escapes).toContain("Escape");
+    wrapper.unmount();
+  });
+});