From 70728413a7906eda7683716d724a19b7bb68c839 Mon Sep 17 00:00:00 2001 From: swithek <52840391+swithek@users.noreply.github.com> Date: Sun, 6 Sep 2026 12:45:09 +0200 Subject: [PATCH] add tag deletion modal --- web/app/components/AppSidebar.nuxt.test.ts | 30 ++++- web/app/components/AppSidebar.vue | 27 ++--- .../components/TagDeletionModal.nuxt.test.ts | 114 ++++++++++++++++++ web/app/components/TagDeletionModal.vue | 104 ++++++++++++++++ .../editor/DocumentTagList.nuxt.test.ts | 54 +++++++++ web/app/components/editor/DocumentTagList.vue | 38 ++++-- .../[[organizationSlug]]/[[documentSlug]].vue | 20 +++ web/i18n/locales/en/sidebar.json | 8 +- 8 files changed, 366 insertions(+), 29 deletions(-) create mode 100644 web/app/components/TagDeletionModal.nuxt.test.ts create mode 100644 web/app/components/TagDeletionModal.vue diff --git a/web/app/components/AppSidebar.nuxt.test.ts b/web/app/components/AppSidebar.nuxt.test.ts index 2b2c3934..884bdaad 100644 --- a/web/app/components/AppSidebar.nuxt.test.ts +++ b/web/app/components/AppSidebar.nuxt.test.ts @@ -400,6 +400,25 @@ describe("", { concurrent: false }, () => { expect(sidebar(wrapper).emitted("initial-load-complete")).toHaveLength(1) }) + it("announces that every section has loaded with every tag hidden", async ({ + expect, + }) => { + // a reload hands the cache its answers before the sidebar is built, + // so the tags section is there from the first render — and with the + // last visible tag hidden it has no rows to report + stubQueries() + seedQueryData(["documents", "tree"], [treeElement()]) + seedQueryData( + ["tags", "tree"], + [{ ...tagElement(TAG_A, "Production", "#22c55e"), hidden: true }], + ) + + const wrapper = await mountSidebar() + await settleMutations() + + expect(sidebar(wrapper).emitted("initial-load-complete")).toHaveLength(1) + }) + describe("next steps", { concurrent: false }, () => { it("invites the user to add team members while alone", async ({ expect, @@ -714,7 +733,9 @@ describe("", { concurrent: false }, () => { expect(calls[0]?.body).toEqual({ hidden: true }) }) - it("deletes a tag from its options menu", async ({ expect }) => { + it("asks the page to delete a tag from its options menu", async ({ + expect, + }) => { stubQueries({ tags: seededTags() }) const calls = mockEndpoint("DELETE", `/api/tags/${TAG_A}`, () => ({})) const wrapper = await mountSidebar() @@ -724,7 +745,12 @@ describe("", { concurrent: false }, () => { menuItem(t("sidebar.item-dropdown-menu-buttons.delete-tag")).click() await settleMutations() - expect(calls).toHaveLength(1) + // the modal outlives the sidebar's own drawer on a narrow screen, + // so the page owns it and the deletion with it + expect(sidebar(wrapper).emitted("delete-tag")).toEqual([ + [{ id: TAG_A, name: "Production" }], + ]) + expect(calls).toHaveLength(0) }) // the tree lists a document under a tag by its default branch, so diff --git a/web/app/components/AppSidebar.vue b/web/app/components/AppSidebar.vue index c607a311..6d73e969 100644 --- a/web/app/components/AppSidebar.vue +++ b/web/app/components/AppSidebar.vue @@ -21,18 +21,14 @@ const emit = defineEmits<{ (e: "initial-load-complete" | "toggle-notifications"): void (e: "open-settings", target: "org-members" | null): void (e: "delete-document" | "duplicate-document", id: string): void + (e: "delete-tag", tag: { id: string; name: string }): void (e: "create-document", parentId: string | null): void }>() const { t } = useI18n({ useScope: "global" }) const { fetchDocumentTree, updateDocumentTree } = useDocumentAPI() -const { - fetchTagTree, - updateTagTree, - updateTagVisibility, - deleteTag, - unassignBranchTag, -} = useTagAPI() +const { fetchTagTree, updateTagTree, updateTagVisibility, unassignBranchTag } = + useTagAPI() const { fetchOrganization, safeSignOut } = useAuthSession() const { fetchGitHubConnectionStatus, gitHubConfigured, fetchGitHubInstallURL } = useGitHubAPI() @@ -131,6 +127,9 @@ const sections = computed(() => { rootItems.push({ id: "tags", heading: t("sidebar.sections.tags.heading"), + // the tree has answered by the time this section exists, so no + // rows means every tag is hidden rather than still on its way + isEmptyAfterLoad: true, emptyMessage: t("sidebar.sections.tags.all-hidden"), items: tagItems, onUpdateLocation: handleTagLocationUpdate, @@ -351,8 +350,8 @@ function tagActions(tag: TagTreeElement): SidebarItemAction[] { id: "delete-tag", name: t("sidebar.item-dropdown-menu-buttons.delete-tag"), icon: "lucide:trash-2", - fn: async () => { - await handleTagDelete(tag.id) + fn: () => { + emit("delete-tag", { id: tag.id, name: tag.tagName }) }, }, ] @@ -383,14 +382,6 @@ async function handleTagVisibilityUpdate(id: string, req: { hidden: boolean }) { } } -async function handleTagDelete(id: string) { - try { - await deleteTag.mutateAsync(id) - } catch { - showToastMessage("error", t("sidebar.errors.delete-tag-failed")) - } -} - // the tree lists a document under a tag by its default branch, so that is // the branch the row's action detaches async function handleDocumentTagRemoval( @@ -506,7 +497,7 @@ async function installSlack() {
{{ section.emptyMessage }}
diff --git a/web/app/components/TagDeletionModal.nuxt.test.ts b/web/app/components/TagDeletionModal.nuxt.test.ts new file mode 100644 index 00000000..e59b4101 --- /dev/null +++ b/web/app/components/TagDeletionModal.nuxt.test.ts @@ -0,0 +1,114 @@ +import { mountSuspended } from "@nuxt/test-utils/runtime" +import { beforeEach, describe, it, vi } from "vitest" +import TagDeletionModal from "./TagDeletionModal.vue" +import { clearTeleportedOverlays, t, teleportedButton } from "./test-helpers" + +function mountModal( + target: { deleteTag: () => Promise; name: string } | null, +) { + return mountSuspended(TagDeletionModal, { + props: { modelValue: target }, + }) +} + +// the dialog body is teleported into , so it is the only place the +// modal's own markup can be read from +function dialog() { + return document.body.querySelector("[data-slot='dialog-content']") +} + +// the dialog body is teleported into the shared and the delete flow +// is driven by the global fake timers, so these tests cannot interleave +// t() reaches for the nuxt app context, so the label can only be +// resolved inside a test — not once at module scope +function confirmButton() { + return teleportedButton(t("sidebar.tag-deletion-modal.confirm-button")) +} + +describe("", { concurrent: false }, () => { + beforeEach(clearTeleportedOverlays) + + it("stays closed while there is no deletion target", async ({ expect }) => { + await mountModal(null) + + expect(dialog()).toBeNull() + }) + + it("names the tag it is about to delete", async ({ expect }) => { + await mountModal({ name: "Production", deleteTag: vi.fn() }) + + expect(dialog()?.textContent).toContain("Production") + }) + + it("deletes the tag when the confirm button is pressed", async ({ + expect, + }) => { + vi.useFakeTimers() + const deleteTag = vi.fn().mockResolvedValue(undefined) + const wrapper = await mountModal({ name: "Production", deleteTag }) + + confirmButton().click() + await vi.advanceTimersByTimeAsync(300) + + expect(deleteTag).toHaveBeenCalledTimes(1) + expect(wrapper.emitted("update:modelValue")?.at(-1)).toEqual([null]) + }) + + it("shows a spinner and disables both buttons while deleting", async ({ + expect, + }) => { + vi.useFakeTimers() + const deleteTag = vi.fn().mockResolvedValue(undefined) + await mountModal({ name: "Production", deleteTag }) + + confirmButton().click() + await nextTick() + + expect( + dialog()?.querySelector(".i-svg-spinners\\:blocks-shuffle-3"), + ).not.toBeNull() + expect(confirmButton().disabled).toBe(true) + expect( + teleportedButton(t("sidebar.tag-deletion-modal.cancel-button")).disabled, + ).toBe(true) + }) + + it("closes without deleting when the cancel button is pressed", async ({ + expect, + }) => { + const deleteTag = vi.fn() + const wrapper = await mountModal({ name: "Production", deleteTag }) + + teleportedButton(t("sidebar.tag-deletion-modal.cancel-button")).click() + await nextTick() + + expect(deleteTag).toHaveBeenCalledTimes(0) + expect(wrapper.emitted("update:modelValue")?.at(-1)).toEqual([null]) + }) + + it("closes without deleting when the close button is pressed", async ({ + expect, + }) => { + const deleteTag = vi.fn() + const wrapper = await mountModal({ name: "Production", deleteTag }) + + teleportedButton(t("general.modal-close-screen-reader-hint")).click() + await nextTick() + + expect(deleteTag).toHaveBeenCalledTimes(0) + expect(wrapper.emitted("update:modelValue")?.at(-1)).toEqual([null]) + }) + + it("keeps showing the tag name while the dialog animates shut", async ({ + expect, + }) => { + const wrapper = await mountModal({ + name: "Production", + deleteTag: vi.fn(), + }) + + await wrapper.setProps({ modelValue: null }) + + expect(document.body.textContent).toContain("Production") + }) +}) diff --git a/web/app/components/TagDeletionModal.vue b/web/app/components/TagDeletionModal.vue new file mode 100644 index 00000000..a73b91dd --- /dev/null +++ b/web/app/components/TagDeletionModal.vue @@ -0,0 +1,104 @@ + + diff --git a/web/app/components/editor/DocumentTagList.nuxt.test.ts b/web/app/components/editor/DocumentTagList.nuxt.test.ts index 2ba354e4..b57583a5 100644 --- a/web/app/components/editor/DocumentTagList.nuxt.test.ts +++ b/web/app/components/editor/DocumentTagList.nuxt.test.ts @@ -4,6 +4,7 @@ import { clearQueryCache, disposeMockEndpoints, makeXid, + mockDeferredEndpoint, mockEndpoint, runInApp, } from "~/composables/api/test-helpers" @@ -447,6 +448,22 @@ describe("", { concurrent: false }, () => { expect(pickerRows().filter((row) => row.includes("Production"))).toEqual([]) }) + it("reopens with the search box empty", async ({ expect }) => { + stubTags([ + makeTag(TAG_A, "Production", "#1a9e4a"), + makeTag(TAG_B, "Staging", "#e8760c"), + ]) + const wrapper = await mountTags() + await openPicker(wrapper) + await search("stag") + + await openPicker(wrapper) + await openPicker(wrapper) + + expect(pickerInput().value).toBe("") + expect(pickerRows()).toEqual(["Production", "Staging"]) + }) + it("hints at creation while nothing is typed", async ({ expect }) => { stubTags([makeTag(TAG_A, "Production", "#1a9e4a")]) const wrapper = await mountTags() @@ -512,6 +529,43 @@ describe("", { concurrent: false }, () => { expect(assigned[0]?.body).toEqual({ tagId: TAG_B }) }) + it("keeps the other tags listed while the new one is still in flight", async ({ + expect, + }) => { + // the create settles only once the refetch behind it has, so a slow + // request leaves the list on screen for as long as it takes + stubTags([makeTag(TAG_A, "Production", "#1a9e4a")]) + const created = mockDeferredEndpoint("POST", "/api/tags") + const wrapper = await mountTags() + await openPicker(wrapper) + await search("Rollout") + + pressEnter() + await created.reached + await nextTick() + + expect(pickerRows()).toEqual(["Production", "Rollout"]) + }) + + it("scrolls the list down to the tag it just appended", async ({ + expect, + }) => { + // happy-dom lays nothing out, so the list reports a height for the + // scroll to aim at + vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue(500) + stubTags([makeTag(TAG_A, "Production", "#1a9e4a")]) + mockEndpoint("POST", "/api/tags", () => ({ id: TAG_B })) + mockEndpoint("POST", BRANCH_TAGS_URL, () => ({})) + const wrapper = await mountTags() + await openPicker(wrapper) + await search("Rollout") + + pressEnter() + await nextTick() + + expect(pickerItems()[0]?.parentElement?.scrollTop).toBe(500) + }) + it("creates nothing on enter for a name that already exists", async ({ expect, }) => { diff --git a/web/app/components/editor/DocumentTagList.vue b/web/app/components/editor/DocumentTagList.vue index 31dcfd09..d5f46535 100644 --- a/web/app/components/editor/DocumentTagList.vue +++ b/web/app/components/editor/DocumentTagList.vue @@ -49,6 +49,7 @@ const documentTags = computed(() => { const rowElem = useTemplateRef("tag-row") const triggerElem = useTemplateRef("tag-trigger") +const listElem = useTemplateRef("tag-list") const rowParentElem = computed(() => rowElem.value?.parentElement ?? null) // the count settles by trying one more pill until the row overflows, then // stepping back. tooMany remembers the count that overflowed, so the two @@ -115,14 +116,16 @@ useResizeObserver([rowElem, rowParentElem], () => { void measureOnResize() }) +// the search is emptied on the way in rather than on the way out: the +// list is still on screen while the menu fades, and clearing it then +// shows the rows springing back to their full length mid-close watch(pickerOpen, (isOpen) => { - if (isOpen) { - newColor.value = suggestedColor() - + if (!isOpen) { return } query.value = "" + newColor.value = suggestedColor() }) // the swatches come from theme variables, which resolve to oklch, while a @@ -205,21 +208,31 @@ async function toggleTag(tag: TagTreeElement) { async function createAndAssign() { const documentId = editorStore.activeDocumentId const branchId = editorStore.activeBranchId - if (!documentId || !branchId || !trimmedQuery.value) { + const tagName = trimmedQuery.value + if (!documentId || !branchId || !tagName) { return } + // the search box is emptied before the request rather than after it. + // mutateAsync settles only once the refetch its success triggers has, + // and until then the typed name narrows the list to the one tag that + // matches it — the one just created + query.value = "" + try { - const created = await createTag.mutateAsync({ - tagName: trimmedQuery.value, + const creating = createTag.mutateAsync({ + tagName: tagName, // the swatches resolve from theme variables, which are oklch; the // stored colour is hex so it survives a theme change color: colorToHex( newColor.value ?? chartStyles().selectableColors.default, ), }) + // the tag is appended, so the row it adds is below the fold on a + // list long enough to scroll. onMutate has already put it there + void nextTick(scrollListToEnd) - query.value = "" + const created = await creating // the tag just created holds a colour now, so the next one drawn has // to weigh it in — the menu stays open across several creations newColor.value = suggestedColor() @@ -234,6 +247,15 @@ async function createAndAssign() { } } +function scrollListToEnd() { + const list = listElem.value + if (!list) { + return + } + + list.scrollTop = list.scrollHeight +} + // the create row is the only thing enter can act on: an existing tag is // toggled from its own row, and a name already taken leaves nothing to // create @@ -307,7 +329,7 @@ function handleSearchEnter() { />