From 5e37c16a6dce77775d5060aef526dcbb1f17f6f4 Mon Sep 17 00:00:00 2001 From: Benjamin Edward Niedzielski Date: Wed, 30 Sep 2026 09:47:46 -0700 Subject: [PATCH] fix(phone): improve search and navigation --- .../__tests__/phone-list-filter.test.ts | 42 +++- .../__tests__/phone-list-maintain.test.ts | 54 ++++- .../__tests__/phone-list-unit-table.test.ts | 108 +++++++++- .../Personnel/__tests__/phone-list.test.ts | 110 ++++++++++ .../__tests__/section-jump-links.test.ts | 4 +- .../svm-frequent-number-table.test.ts | 48 ++++- .../__tests__/svm-phone-section-table.test.ts | 46 ++++- .../__tests__/svm-phones-maintain.test.ts | 58 +++++- .../Personnel/__tests__/svm-phones.test.ts | 76 +++---- VueApp/src/Personnel/__tests__/test-utils.ts | 43 +++- VueApp/src/Personnel/assets/phone-list.css | 64 ++++++ .../Personnel/components/MobileCardList.vue | 79 ++++++-- .../components/MobileSortControl.vue | 2 +- .../Personnel/components/PhoneListFilter.vue | 60 +++++- .../components/PhoneListUnitTable.vue | 188 ++++++++++-------- .../components/SVMFrequentNumberTable.vue | 110 +++++----- .../components/SVMPhoneSectionTable.vue | 113 ++++++----- .../Personnel/components/SectionJumpLinks.vue | 25 ++- .../composables/section-jump-targets.ts | 73 +++++++ .../composables/use-section-table.ts | 40 ++++ VueApp/src/Personnel/pages/PhoneList.vue | 25 ++- .../src/Personnel/pages/PhoneListMaintain.vue | 12 +- VueApp/src/Personnel/pages/SVMPhones.vue | 51 ++--- .../src/Personnel/pages/SVMPhonesMaintain.vue | 18 +- VueApp/src/Personnel/personnel.ts | 2 + VueApp/src/styles/base.css | 12 -- 26 files changed, 1124 insertions(+), 339 deletions(-) create mode 100644 VueApp/src/Personnel/assets/phone-list.css create mode 100644 VueApp/src/Personnel/composables/section-jump-targets.ts create mode 100644 VueApp/src/Personnel/composables/use-section-table.ts diff --git a/VueApp/src/Personnel/__tests__/phone-list-filter.test.ts b/VueApp/src/Personnel/__tests__/phone-list-filter.test.ts index 70aca4059..61ab25d0c 100644 --- a/VueApp/src/Personnel/__tests__/phone-list-filter.test.ts +++ b/VueApp/src/Personnel/__tests__/phone-list-filter.test.ts @@ -2,9 +2,9 @@ import { mount } from "@vue/test-utils" import { Quasar } from "quasar" import PhoneListFilter from "../components/PhoneListFilter.vue" -function mountFilter(search = "") { +function mountFilter(search = "", noMatches?: boolean) { return mount(PhoneListFilter, { - props: { modelValue: search }, + props: { modelValue: search, noMatches }, global: { plugins: [Quasar] }, }) } @@ -38,19 +38,47 @@ describe("phoneListFilter.vue", () => { it("carries the class the sticky rule is keyed to", () => { expect.hasAssertions() - // The pinning itself is a media query, which jsdom does not apply; this only guards the - // hook it is attached to. Whether it looks right on a phone still needs a real browser. + // The pinning itself is CSS, which these tests do not load; this only guards the hook it + // is attached to. Whether it looks right still needs a real browser, at desktop and phone + // widths. expect(mountFilter().find(".phone-list-filter").exists()).toBeTruthy() }) - it("publishes its own height, which the scroll-margin offset is calculated from", () => { + it("publishes its own height, which the page's scroll padding is calculated from", () => { expect.hasAssertions() document.documentElement.style.removeProperty("--phone-list-filter-height") mountFilter() - // Jsdom reports 0 for every measurement, so this guards that the variable is published at - // all, not what it comes to. The value itself is only meaningful in a real browser. + // Happy-dom does no layout and reports 0 for every measurement, so this guards that the + // variable is published at all, not what it comes to. The value itself is only meaningful + // in a real browser. expect(document.documentElement.style.getPropertyValue("--phone-list-filter-height")).toBe("0px") }) }) + +describe("phoneListFilter.vue - no matches line", () => { + it("names the search that matched nothing", () => { + expect.hasAssertions() + + expect(mountFilter("smith", true).find("[role='status']").text()).toBe('No records match "smith".') + }) + + it("says nothing while the search has matches", () => { + expect.hasAssertions() + + expect(mountFilter("smith", false).find("[role='status']").text()).toBe("") + }) + + it("says nothing when the page does not set it, as the maintain pages do not", () => { + expect.hasAssertions() + + expect(mountFilter("smith").find("[role='status']").text()).toBe("") + }) + + it("keeps the live region rendered while it is silent, so its first message is announced", () => { + expect.hasAssertions() + + expect(mountFilter("smith", false).find("[role='status']").exists()).toBeTruthy() + }) +}) diff --git a/VueApp/src/Personnel/__tests__/phone-list-maintain.test.ts b/VueApp/src/Personnel/__tests__/phone-list-maintain.test.ts index 67904f758..9d37b72b5 100644 --- a/VueApp/src/Personnel/__tests__/phone-list-maintain.test.ts +++ b/VueApp/src/Personnel/__tests__/phone-list-maintain.test.ts @@ -6,7 +6,7 @@ import { getPhoneListData } from "../composables/phone-list-data-fetch.ts" import { phoneListService } from "../services/phone-list-service.ts" import { phoneListUnitService } from "../services/phone-list-unit-service.ts" import type { PhoneListUnit } from "../types/phone-list-phone-types" -import { apiError, apiResult } from "./test-utils" +import { apiError, apiResult, linkTargets, openJumpLinks, trackAttached, unmountAttached } from "./test-utils" /** * PhoneListMaintain shows a StatusBanner only when a delete/save action reports an error @@ -90,15 +90,20 @@ const personSelectorStub = { template: "
", } -function mountPage() { - return mount(PhoneListMaintain, { +/** Pass attach to mount into the document, which the jump links' portalled menu needs. */ +function mountPage(attach = false) { + const wrapper = mount(PhoneListMaintain, { global: { plugins: [[Quasar, { plugins: { Notify } }]], stubs: { PersonSelector: personSelectorStub }, }, + ...(attach ? { attachTo: document.body } : {}), }) + return attach ? trackAttached(wrapper) : wrapper } +afterEach(unmountAttached) + function findAddButton(wrapper: ReturnType) { return wrapper.findAllComponents({ name: "QBtn" }).find((btn) => btn.props("icon") === "add") } @@ -242,3 +247,46 @@ describe("phoneListMaintain.vue - error banner", () => { expect(vi.mocked(getPhoneListData).mock.calls.length).toBeGreaterThan(callsBeforeSave) }) }) + +describe("phoneListMaintain.vue - unit jump links", () => { + async function mountWithTwoUnits() { + stubListInfo() + vi.mocked(getPhoneListData).mockResolvedValue([ + unitWithDeletableRow(), + { ...unitWithDeletableRow(), id: 11, name: "Business Office" }, + ]) + const wrapper = mountPage(true) + await flushPromises() + return wrapper + } + + it("links to every unit", async () => { + expect.hasAssertions() + const wrapper = await mountWithTwoUnits() + + await expect(linkTargets(wrapper)).resolves.toStrictEqual(["Dean's Office", "Business Office"]) + }) + + it("keeps linking to units a search has emptied, since they stay shown with their add button", async () => { + expect.hasAssertions() + const wrapper = await mountWithTwoUnits() + + await wrapper.findComponent({ name: "QInput" }).setValue("nobody") + + await expect(linkTargets(wrapper)).resolves.toStrictEqual(["Dean's Office", "Business Office"]) + }) + + it("points each link at the heading of its own unit", async () => { + expect.hasAssertions() + const wrapper = await mountWithTwoUnits() + const links = await openJumpLinks(wrapper) + + expect(links).toHaveLength(2) + + for (const link of links) { + const id = link.getAttribute("href")!.slice(1) + + expect(wrapper.find(`h2#${id}`).exists()).toBeTruthy() + } + }) +}) diff --git a/VueApp/src/Personnel/__tests__/phone-list-unit-table.test.ts b/VueApp/src/Personnel/__tests__/phone-list-unit-table.test.ts index f76ea0096..a2a42069d 100644 --- a/VueApp/src/Personnel/__tests__/phone-list-unit-table.test.ts +++ b/VueApp/src/Personnel/__tests__/phone-list-unit-table.test.ts @@ -1,5 +1,5 @@ import { mount } from "@vue/test-utils" -import { Quasar } from "quasar" +import { Quasar, Screen } from "quasar" import PhoneListUnitTable from "../components/PhoneListUnitTable.vue" import type { PhoneListDisplayRecord, PhoneListUnit } from "../types/phone-list-phone-types" @@ -50,16 +50,20 @@ function makeUnit(rows: PhoneListDisplayRecord[]): PhoneListUnit { return { name: "Dean's Office", id: 10, cols, rows } } -function mountTable(props: { unit: PhoneListUnit; loading: boolean; isMaintain: boolean; search: string }) { +function mountTable(props: { + unit: PhoneListUnit + loading: boolean + isMaintain: boolean + search: string + anchorId?: string +}) { return mount(PhoneListUnitTable, { props, global: { plugins: [Quasar] }, }) } -// QTable always renders a .q-table__title element from its own :title prop, so it can't -// distinguish the custom top-left slot (the "add" button) from the base title - check for the -// button itself instead. +// The add button is found by its icon: its only text is an aria-label, so there is no label to find it by. function hasAddButton(wrapper: ReturnType): boolean { return wrapper.findAllComponents({ name: "QBtn" }).some((btn) => btn.props("icon") === "add") } @@ -106,6 +110,19 @@ describe("phoneListUnitTable.vue - isMaintain gating", () => { expect(wrapper.text()).toContain("Smith, Amy") }) + it("keeps the add button out of the unit headings, so its label is not part of their name", () => { + expect.hasAssertions() + const wrapper = mountTable({ unit: makeUnit([makeRow()]), loading: false, isMaintain: true, search: "" }) + const headings = wrapper.findAll("h2") + + // One for the desktop table, one for the mobile list. + expect(headings).toHaveLength(2) + for (const heading of headings) { + expect(heading.text()).toBe("Dean's Office") + expect(heading.find("button").exists()).toBeFalsy() + } + }) + it("shows a check icon only on the row where listFirst is true", () => { expect.hasAssertions() const wrapper = mountTable({ @@ -123,6 +140,87 @@ describe("phoneListUnitTable.vue - isMaintain gating", () => { }) }) +/** Whether the desktop table and its heading are left showing, rather than hidden by v-show. */ +function desktopShown(wrapper: ReturnType): boolean { + return (wrapper.find(".gt-sm").element as HTMLElement).style.display !== "none" +} + +describe("phoneListUnitTable.vue - desktop table visibility", () => { + it("shows the table while the search matches one of its rows", () => { + expect.hasAssertions() + const wrapper = mountTable({ unit: makeUnit([makeRow()]), loading: false, isMaintain: false, search: "smith" }) + + expect(desktopShown(wrapper)).toBeTruthy() + }) + + it("hides the table once a search leaves it with no rows, as the list does", () => { + expect.hasAssertions() + const wrapper = mountTable({ + unit: makeUnit([makeRow()]), + loading: false, + isMaintain: false, + search: "no match", + }) + + expect(desktopShown(wrapper)).toBeFalsy() + }) + + it("hides a unit with no rows before any search", () => { + expect.hasAssertions() + const wrapper = mountTable({ unit: makeUnit([]), loading: false, isMaintain: false, search: "" }) + + expect(desktopShown(wrapper)).toBeFalsy() + }) + + it("keeps the emptied table in maintain mode, where it carries the add button", () => { + expect.hasAssertions() + const wrapper = mountTable({ + unit: makeUnit([makeRow()]), + loading: false, + isMaintain: true, + search: "no match", + }) + + expect(desktopShown(wrapper)).toBeTruthy() + }) +}) + +describe("phoneListUnitTable.vue - jump link anchor", () => { + const initialLtMd = Screen.lt.md + + afterEach(() => { + Screen.lt.md = initialLtMd + }) + + function anchoredHeadings(ltMd: boolean) { + Screen.lt.md = ltMd + const wrapper = mountTable({ + unit: makeUnit([makeRow()]), + loading: false, + isMaintain: false, + search: "", + anchorId: "phone-unit-10", + }) + return wrapper.findAll("h2#phone-unit-10") + } + + it("anchors the desktop heading at desktop widths, where the mobile one is hidden", () => { + expect.hasAssertions() + const headings = anchoredHeadings(false) + + expect(headings).toHaveLength(1) + expect(headings[0].element.closest(".gt-sm")).not.toBeNull() + }) + + it("anchors the mobile heading below 1024px, where the desktop one is hidden", () => { + expect.hasAssertions() + const headings = anchoredHeadings(true) + + expect(headings).toHaveLength(1) + expect(headings[0].element.closest(".lt-md")).not.toBeNull() + }) +}) + /** The mobile list, which renders alongside the table and is shown by CSS below 1024px. */ function listItems(wrapper: ReturnType) { return wrapper.findComponent({ name: "QList" }).findAllComponents({ name: "QItem" }) diff --git a/VueApp/src/Personnel/__tests__/phone-list.test.ts b/VueApp/src/Personnel/__tests__/phone-list.test.ts index 66f941530..6e54c9035 100644 --- a/VueApp/src/Personnel/__tests__/phone-list.test.ts +++ b/VueApp/src/Personnel/__tests__/phone-list.test.ts @@ -4,6 +4,8 @@ import PhoneList from "../pages/PhoneList.vue" import { getPhoneListData } from "../composables/phone-list-data-fetch" import { phoneListModifiedDateService } from "../services/phone-list-modified-date-service.ts" import { phoneListService } from "../services/phone-list-service.ts" +import type { PhoneListDisplayRecord, PhoneListUnit } from "../types/phone-list-phone-types" +import { linkTargets, openJumpLinks, trackAttached, unmountAttached } from "./test-utils" /** * PhoneList is the read-only view of any unit list, resolved from the :code route param. It @@ -122,3 +124,111 @@ describe("phoneList.vue - loading, naming, and internal-use banner", () => { expect(getPhoneListData).not.toHaveBeenCalled() }) }) + +// Attached, so the jump links' portalled menu reaches the document. +function mountAttachedPage() { + return trackAttached( + mount(PhoneList, { + global: { plugins: [[Quasar, { plugins: { Notify } }]] }, + attachTo: document.body, + }), + ) +} + +afterEach(unmountAttached) + +const unitCols = [ + { name: "name", label: "Name", field: "name", align: "left" as const }, + { name: "office", label: "Office", field: "office", align: "left" as const }, +] + +// Only the fields the jump links read: the unit name, and enough of a row for the filter to match +// on. The rest of PhoneListDisplayRecord is irrelevant here. +function unit(id: number, name: string, row: { name: string; office?: string }): PhoneListUnit { + const { office = "Room 100" } = row + return { + name, + id, + cols: unitCols, + rows: [{ name: row.name, office, unitPersonId: id, employeeMailId: "" } as unknown as PhoneListDisplayRecord], + } +} + +describe("phoneList.vue - unit jump links", () => { + async function mountWithUnits(units: PhoneListUnit[]) { + stubListInfo(false) + vi.mocked(getPhoneListData).mockResolvedValue(units) + const wrapper = mountAttachedPage() + await flushPromises() + return wrapper + } + + it("links to every unit", async () => { + expect.hasAssertions() + const wrapper = await mountWithUnits([ + unit(1, "Dean's Office", { name: "Smith, Amy" }), + unit(2, "Business Office", { name: "Jones, Bo" }), + ]) + + await expect(linkTargets(wrapper)).resolves.toStrictEqual(["Dean's Office", "Business Office"]) + }) + + it("points each link at the heading of its own unit", async () => { + expect.hasAssertions() + const wrapper = await mountWithUnits([ + unit(1, "Dean's Office", { name: "Smith, Amy" }), + unit(2, "Business Office", { name: "Jones, Bo" }), + ]) + + // The links come from the portalled menu; the headings they point at stay in the page. + const links = await openJumpLinks(wrapper) + + expect(links).not.toHaveLength(0) + + for (const link of links) { + const id = link.getAttribute("href")!.slice(1) + + expect(wrapper.find(`h2#${id}`).exists()).toBeTruthy() + } + }) + + it("drops a unit from the links once a search leaves it with no rows", async () => { + expect.hasAssertions() + const wrapper = await mountWithUnits([ + unit(1, "Dean's Office", { name: "Smith, Amy", office: "Room 100" }), + unit(2, "Business Office", { name: "Jones, Bo", office: "Room 100" }), + unit(3, "Teaching Office", { name: "Lee, Cy", office: "Room 200" }), + ]) + + // A unit the search empties is hidden, so a link to it would go nowhere. + await wrapper.findComponent({ name: "QInput" }).setValue("room 100") + + await expect(linkTargets(wrapper)).resolves.toStrictEqual(["Dean's Office", "Business Office"]) + }) + + it("says once, below the filter, when a search leaves every unit empty", async () => { + expect.hasAssertions() + const wrapper = await mountWithUnits([ + unit(1, "Dean's Office", { name: "Smith, Amy" }), + unit(2, "Business Office", { name: "Jones, Bo" }), + ]) + const status = () => wrapper.find("[role='status']").text() + + expect(status()).toBe("") + + await wrapper.findComponent({ name: "QInput" }).setValue("nobody") + + expect(status()).toBe('No records match "nobody".') + + await wrapper.findComponent({ name: "QInput" }).setValue("jones") + + expect(status()).toBe("") + }) + + it("offers no jump links for a list with a single unit", async () => { + expect.hasAssertions() + const wrapper = await mountWithUnits([unit(1, "Dean's Office", { name: "Smith, Amy" })]) + + await expect(linkTargets(wrapper)).resolves.toStrictEqual([]) + }) +}) diff --git a/VueApp/src/Personnel/__tests__/section-jump-links.test.ts b/VueApp/src/Personnel/__tests__/section-jump-links.test.ts index 81d9de3a6..7aed59a2c 100644 --- a/VueApp/src/Personnel/__tests__/section-jump-links.test.ts +++ b/VueApp/src/Personnel/__tests__/section-jump-links.test.ts @@ -61,11 +61,11 @@ describe("sectionJumpLinks.vue", () => { ]) }) - it("uses plain anchors, so the browser's own fragment navigation applies the scroll margin", async () => { + it("uses plain anchors, so the browser's own fragment navigation applies the scroll padding", async () => { expect.hasAssertions() const wrapper = await openMenu() - // A router-link would position with window.scrollTo, which ignores scroll-margin-top and + // A router-link would position with window.scrollTo, which ignores scroll padding and // would drop the heading under the sticky header and filter bar. expect(wrapper.findComponent({ name: "RouterLink" }).exists()).toBeFalsy() }) diff --git a/VueApp/src/Personnel/__tests__/svm-frequent-number-table.test.ts b/VueApp/src/Personnel/__tests__/svm-frequent-number-table.test.ts index 3480ac031..ffe5345b2 100644 --- a/VueApp/src/Personnel/__tests__/svm-frequent-number-table.test.ts +++ b/VueApp/src/Personnel/__tests__/svm-frequent-number-table.test.ts @@ -5,9 +5,9 @@ import type { SVMFrequentNumberRecord } from "../types/svm-phone-types" const numbers: SVMFrequentNumberRecord[] = [{ label: "Front Desk", phone: "530-555-1000", entryId: 1 }] -function mountTable(editRecords: boolean, rows: SVMFrequentNumberRecord[] = numbers, search = "") { +function mountTable(editRecords: boolean, rows: SVMFrequentNumberRecord[] = numbers, search = "", loading = false) { return mount(SVMFrequentNumberTable, { - props: { frequentNumbers: rows, loading: false, editRecords, search }, + props: { frequentNumbers: rows, loading, editRecords, search }, global: { plugins: [Quasar] }, }) } @@ -46,8 +46,46 @@ describe("sVMFrequentNumberTable.vue - editRecords gating", () => { }) }) +/** Whether the desktop table and its heading are left showing, rather than hidden by v-show. */ +function desktopShown(wrapper: ReturnType): boolean { + return (wrapper.find(".gt-sm").element as HTMLElement).style.display !== "none" +} + +describe("sVMFrequentNumberTable.vue - desktop table visibility", () => { + it("shows the table while the search matches one of its numbers", () => { + expect.hasAssertions() + + expect(desktopShown(mountTable(false, numbers, "front"))).toBeTruthy() + }) + + it("hides the table once a search leaves it with no numbers, as the list does", () => { + expect.hasAssertions() + + expect(desktopShown(mountTable(false, numbers, "no match"))).toBeFalsy() + }) + + it("keeps the emptied table in edit mode, where it carries the add button", () => { + expect.hasAssertions() + + expect(desktopShown(mountTable(true, numbers, "no match"))).toBeTruthy() + }) + + it("keeps the table while its numbers are still loading, so the loading bar shows", () => { + expect.hasAssertions() + + expect(desktopShown(mountTable(false, [], "", true))).toBeTruthy() + }) + + it("heads the table whether or not it can be edited, so a jump link has somewhere to land", () => { + expect.hasAssertions() + + expect(mountTable(false).find(".gt-sm h2").text()).toBe("Frequently Called Numbers") + expect(mountTable(true).find(".gt-sm h2").text()).toBe("Frequently Called Numbers") + }) +}) + describe("sVMFrequentNumberTable.vue - narrow screen list", () => { - it("heads each entry with its location and carries the number beneath it, unlabelled", () => { + it("heads each entry with its location and carries the number beneath it, unlabeled", () => { expect.hasAssertions() const [location, number] = listItems(mountTable(false))[0].findAll(".q-item__label") @@ -69,8 +107,8 @@ describe("sVMFrequentNumberTable.vue - narrow screen list", () => { it("shows the section heading whether or not the list can be edited", () => { expect.hasAssertions() - expect(mountTable(false).find("h2").text()).toContain("Frequently Called Numbers") - expect(mountTable(true).find("h2").text()).toContain("Frequently Called Numbers") + expect(mountTable(false).find(".lt-md h2").text()).toContain("Frequently Called Numbers") + expect(mountTable(true).find(".lt-md h2").text()).toContain("Frequently Called Numbers") }) it("applies the search box to the list, the way the filter prop does for the table", () => { diff --git a/VueApp/src/Personnel/__tests__/svm-phone-section-table.test.ts b/VueApp/src/Personnel/__tests__/svm-phone-section-table.test.ts index f685fd68d..108ebe4da 100644 --- a/VueApp/src/Personnel/__tests__/svm-phone-section-table.test.ts +++ b/VueApp/src/Personnel/__tests__/svm-phone-section-table.test.ts @@ -92,14 +92,52 @@ describe("sVMPhoneSectionTable.vue - isModify gating", () => { }) }) +/** Whether the desktop table and its heading are left showing, rather than hidden by v-show. */ +function desktopShown(wrapper: ReturnType): boolean { + return (wrapper.find(".gt-sm").element as HTMLElement).style.display !== "none" +} + +describe("sVMPhoneSectionTable.vue - desktop table visibility", () => { + it("shows the table while the search matches one of its rows", () => { + expect.hasAssertions() + + expect( + desktopShown(mountTable(false, { sectionCols: readOnlyCols, rows: [makeRow()], search: "dean" })), + ).toBeTruthy() + }) + + it("hides the table once a search leaves it with no rows, as the list does", () => { + expect.hasAssertions() + + expect( + desktopShown(mountTable(false, { sectionCols: readOnlyCols, rows: [makeRow()], search: "no match" })), + ).toBeFalsy() + }) + + it("keeps the emptied table in modify mode, where it carries the add button", () => { + expect.hasAssertions() + + expect( + desktopShown(mountTable(true, { sectionCols: readOnlyCols, rows: [makeRow()], search: "no match" })), + ).toBeTruthy() + }) + + it("heads the table whether or not it can be edited, so a jump link has somewhere to land", () => { + expect.hasAssertions() + + expect(mountTable(false).find(".gt-sm h2").text()).toBe("VMDO") + expect(mountTable(true).find(".gt-sm h2").text()).toBe("VMDO") + }) +}) + describe("sVMPhoneSectionTable.vue - narrow screen list", () => { it("shows the section heading whether or not the list can be edited", () => { expect.hasAssertions() - // The table only titles itself in maintain mode; the cards have no column headers to - // identify the section, so the heading is always rendered here. - expect(mountTable(false).find("h2").text()).toContain("VMDO") - expect(mountTable(true).find("h2").text()).toContain("VMDO") + // The cards have no column headers to identify the section, so the heading is always + // rendered here. + expect(mountTable(false).find(".lt-md h2").text()).toContain("VMDO") + expect(mountTable(true).find(".lt-md h2").text()).toContain("VMDO") }) it("renders one list item per row, headed by the unit name", () => { diff --git a/VueApp/src/Personnel/__tests__/svm-phones-maintain.test.ts b/VueApp/src/Personnel/__tests__/svm-phones-maintain.test.ts index a5800203b..4f3286db0 100644 --- a/VueApp/src/Personnel/__tests__/svm-phones-maintain.test.ts +++ b/VueApp/src/Personnel/__tests__/svm-phones-maintain.test.ts @@ -8,7 +8,7 @@ import { getFrequentlyCalledNumbers, getSVMData } from "../composables/svm-data- import { svmUnitService } from "../services/svm-unit-service" import { svmFrequentNumberService } from "../services/svm-frequent-number-service.ts" import type { SVMFrequentNumberRecord, SVMPhoneDisplayRecord, SVMPhoneSection } from "../types/svm-phone-types" -import { apiError, apiResult } from "./test-utils" +import { apiError, apiResult, linkTargets, openJumpLinks, trackAttached, unmountAttached } from "./test-utils" /** * SVMPhonesMaintain reports delete outcomes as toasts rather than as a page banner, since the @@ -112,6 +112,8 @@ async function mountPage( sections?: SVMPhoneSection[] frequentNumbers?: SVMFrequentNumberRecord[] loadError?: string | null + /** Mount into the document, which the jump links' portalled menu needs. */ + attach?: boolean } = {}, ) { vi.mocked(getSVMData).mockResolvedValue({ @@ -131,11 +133,17 @@ async function mountPage( plugins: [[Quasar, { plugins: { Notify } }]], stubs: { PersonSelector: personSelectorStub }, }, + ...(data.attach ? { attachTo: document.body } : {}), }) + if (data.attach) { + trackAttached(wrapper) + } await flushPromises() return wrapper } +afterEach(unmountAttached) + type Page = Awaited> function findAddButton(root: Pick) { @@ -151,9 +159,8 @@ async function clickAction(root: Pick, action: "edit" } /** - * Per-test reset, called as the first line of each test rather than from a beforeEach, since - * vitest/no-hooks is on and the rest of the suite keeps its setup inside the test body. - * Pass false to decline the confirmation dialog. + * Per-test reset, called as the first line of each test. A call rather than a beforeEach, because + * the confirmation answer differs between tests: pass false to decline the confirmation dialog. */ function resetMocks(confirmed = true) { mockConfirmAction.mockResolvedValue(confirmed) @@ -470,3 +477,46 @@ describe("sVMPhonesMaintain.vue - frequently called numbers", () => { expect(filterParent?.childElementCount).toBeGreaterThan(1) }) }) + +describe("sVMPhonesMaintain.vue - section jump links", () => { + function mountWithTwoSections() { + return mountPage({ + sections: [sectionWithDeletableRow(), { ...sectionWithDeletableRow(), id: 2, title: "VMTH" }], + frequentNumbers: [frequentNumber], + attach: true, + }) + } + + it("links to every section, and to the frequently called numbers alongside them", async () => { + expect.hasAssertions() + resetMocks() + const wrapper = await mountWithTwoSections() + + await expect(linkTargets(wrapper)).resolves.toStrictEqual(["VMDO", "VMTH", "Frequently Called Numbers"]) + }) + + it("keeps linking to sections a search has emptied, since they stay shown with their add button", async () => { + expect.hasAssertions() + resetMocks() + const wrapper = await mountWithTwoSections() + + await wrapper.findComponent({ name: "QInput" }).setValue("nobody") + + await expect(linkTargets(wrapper)).resolves.toStrictEqual(["VMDO", "VMTH", "Frequently Called Numbers"]) + }) + + it("points each link at the heading of its own section", async () => { + expect.hasAssertions() + resetMocks() + const wrapper = await mountWithTwoSections() + const links = await openJumpLinks(wrapper) + + expect(links).toHaveLength(3) + + for (const link of links) { + const id = link.getAttribute("href")!.slice(1) + + expect(wrapper.find(`h2#${id}`).exists()).toBeTruthy() + } + }) +}) diff --git a/VueApp/src/Personnel/__tests__/svm-phones.test.ts b/VueApp/src/Personnel/__tests__/svm-phones.test.ts index 304f223a8..0d820ee17 100644 --- a/VueApp/src/Personnel/__tests__/svm-phones.test.ts +++ b/VueApp/src/Personnel/__tests__/svm-phones.test.ts @@ -4,6 +4,7 @@ import SVMPhones from "../pages/SVMPhones.vue" import { getFrequentlyCalledNumbers, getSVMData } from "../composables/svm-data-fetch" import { svmModifiedDateService } from "../services/svm-modified-date-service.ts" import type { SVMFrequentNumberRecord, SVMPhoneDisplayRecord, SVMPhoneSection } from "../types/svm-phone-types" +import { linkTargets, openJumpLinks, trackAttached, unmountAttached } from "./test-utils" /** * SVMPhones hides its "Updated" line while the initial fetch is in flight (v-if="!loading"). @@ -25,25 +26,17 @@ function neverResolves(): Promise { return new Promise(() => {}) } -// The jump links render through a portal, so they only reach the document when the page is -// attached to it, and they outlive the wrapper unless it is unmounted. -const mounted: ReturnType[] = [] - +// Attached, so the jump links' portalled menu reaches the document. function mountPage() { - const wrapper = mount(SVMPhones, { - global: { plugins: [Quasar] }, - attachTo: document.body, - }) - mounted.push(wrapper) - return wrapper + return trackAttached( + mount(SVMPhones, { + global: { plugins: [Quasar] }, + attachTo: document.body, + }), + ) } -afterEach(() => { - for (const wrapper of mounted.splice(0)) { - wrapper.unmount() - } - document.body.innerHTML = "" -}) +afterEach(unmountAttached) function stubDataServices(): void { vi.mocked(getFrequentlyCalledNumbers).mockResolvedValue({ rows: [], error: null }) @@ -80,23 +73,6 @@ describe("sVMPhones.vue - loading", () => { }) }) -/** - * The links sit behind a menu trigger, out of the sticky bar's flow, so they have to be opened - * before they exist anywhere to query. No trigger means too few sections to navigate between. - */ -async function openJumpLinks(wrapper: ReturnType): Promise { - const trigger = wrapper.findComponent({ name: "SectionJumpLinks" }).find("button") - if (trigger.exists()) { - await trigger.trigger("click") - } - return [...document.querySelectorAll(".q-menu a[href^='#']")] -} - -async function linkTargets(wrapper: ReturnType): Promise { - const links = await openJumpLinks(wrapper) - return links.map((link) => link.textContent?.trim() ?? "") -} - const sectionCols = [ { name: "unitName", label: "Unit", field: "unitName", align: "left" as const }, { name: "location", label: "Location", field: "officeLocation", align: "left" as const }, @@ -176,13 +152,32 @@ describe("sVMPhones.vue - section jump links", () => { ], }) - // The filtered-out sections still render, carrying their "no records" line, so a link to - // one would send the reader somewhere empty. + // A section the search empties is hidden, so a link to one would go nowhere. await wrapper.findComponent({ name: "QInput" }).setValue("room 100") await expect(linkTargets(wrapper)).resolves.toStrictEqual(["VMDO", "VMTH"]) }) + it("says once, below the filter, when a search leaves every section empty", async () => { + expect.hasAssertions() + const wrapper = await mountWithData({ + sections: [section(1, "VMDO", { unitName: "Dean's Office" })], + frequentNumbers: [{ label: "Front Desk", phone: "530-555-1000", entryId: 7 }], + }) + const status = () => wrapper.find("[role='status']").text() + + expect(status()).toBe("") + + await wrapper.findComponent({ name: "QInput" }).setValue("nobody") + + expect(status()).toBe('No records match "nobody".') + + // Matching only a frequent number still counts as a match. + await wrapper.findComponent({ name: "QInput" }).setValue("front desk") + + expect(status()).toBe("") + }) + it("keeps the frequently called numbers link when the search matches one of its numbers", async () => { expect.hasAssertions() const wrapper = await mountWithData({ @@ -249,4 +244,15 @@ describe("sVMPhones.vue - partial load failure", () => { expect(links.map((link) => link.textContent?.trim())).toStrictEqual(["VMDO", "VMTH"]) }) + + it("does not blame the search for an empty page after a failed load", async () => { + expect.hasAssertions() + const wrapper = await mountWithFailedFrequentNumbers() + + await wrapper.findComponent({ name: "QInput" }).setValue("nobody") + + // The error banner explains what is missing; "no records match" would suggest the search + // was the reason. + expect(wrapper.find("[role='status']").text()).toBe("") + }) }) diff --git a/VueApp/src/Personnel/__tests__/test-utils.ts b/VueApp/src/Personnel/__tests__/test-utils.ts index c0bb86f73..21895a08a 100644 --- a/VueApp/src/Personnel/__tests__/test-utils.ts +++ b/VueApp/src/Personnel/__tests__/test-utils.ts @@ -1,3 +1,4 @@ +import type { VueWrapper } from "@vue/test-utils" import type { Result } from "@/composables/ViperFetch" /** @@ -23,4 +24,44 @@ function apiError(errors: string[]): Result { return apiResult({ success: false, errors, status: 400 }) } -export { apiResult, apiError } +const attached: { unmount(): void }[] = [] + +/** + * Registers a page mounted with attachTo: document.body, which the jump links need: their menu + * renders through a portal, and outlives the wrapper unless it is unmounted. Call + * unmountAttached from an afterEach to clean up. + */ +function trackAttached(wrapper: W): W { + attached.push(wrapper) + return wrapper +} + +/** Unmounts every page registered with trackAttached, and clears what they portalled out. */ +function unmountAttached(): void { + for (const wrapper of attached.splice(0)) { + wrapper.unmount() + } + document.body.innerHTML = "" +} + +/** + * Opens a page's section jump links and returns them. The links sit behind a menu trigger, out of + * the sticky bar's flow, so they have to be opened before they exist anywhere to query. No trigger + * means too few sections to navigate between. The menu renders through a portal, so the page + * must be mounted with attachTo: document.body for the links to reach the document. + */ +async function openJumpLinks(wrapper: Pick): Promise { + const trigger = wrapper.findComponent({ name: "SectionJumpLinks" }).find("button") + if (trigger.exists()) { + await trigger.trigger("click") + } + return [...document.querySelectorAll(".q-menu a[href^='#']")] +} + +/** The labels of a page's section jump links, in order. */ +async function linkTargets(wrapper: Pick): Promise { + const links = await openJumpLinks(wrapper) + return links.map((link) => link.textContent?.trim() ?? "") +} + +export { apiResult, apiError, linkTargets, openJumpLinks, trackAttached, unmountAttached } diff --git a/VueApp/src/Personnel/assets/phone-list.css b/VueApp/src/Personnel/assets/phone-list.css new file mode 100644 index 000000000..45c2caf9e --- /dev/null +++ b/VueApp/src/Personnel/assets/phone-list.css @@ -0,0 +1,64 @@ +/* Styles shared by the phone list components, kept with the Personnel area rather than in the + app-wide base.css. Imported once by personnel.ts. */ + +/* Heads each of the stacked tables on the phone list pages. Deliberately below the 1.2rem in-app + heading size: a page repeats it once per unit, so at full size the headings outweigh the short + tables under them. Still bold and still an h2, so the page keeps its heading structure. The + .q-page-container prefix outweighs the global h2 rule in base.css, whichever loads first. */ +.q-page-container .table-section-heading { + font-size: 1rem; + line-height: 1.25; + margin: 0; + min-width: 0; + overflow-wrap: anywhere; + + /* As tall as the row it shares with taller controls (an add button, a sort select), with its + text still centered. A jump link lines up the heading's top edge below the pinned bar, so a + heading shorter than its neighbors would leave their tops tucked under the bar. */ + align-self: stretch; + display: flex; + align-items: center; +} + +/* The heading with its add button beside it rather than inside it, where the button's label would + become part of the heading's accessible name. */ +.table-section-header { + display: flex; + align-items: center; + gap: 0.25rem; +} + +/* The add button is xs, which alone is a small hit area. 1.5rem makes it 24px at 768px and up, the + WCAG 2.2 SC 2.5.8 minimum, but 21px below that, where the root font-size is 14px. There it + passes on the criterion's spacing exception instead: no other target sits within the 24px + circle around it. Keep that clearance if anything is placed beside it. */ +.table-section-header .q-btn { + flex: none; + min-width: 1.5rem; + min-height: 1.5rem; +} + +/* Clears the fixed app header and the pinned filter bar on the phone list pages (WCAG technique + C43). Anything the browser scrolls into view - a jump link's heading, or a table cell or card + reached with Tab or Shift+Tab - would otherwise land underneath them, hiding the focus ring + (SC 2.4.11). Padding on the page's scroller covers every such target, where a scroll-margin + would have to be put on each one. Scoped by :has to the pages that show the bar, as this file + loads for the whole Personnel area. + + Both heights are measured and published by the components that own them - ViperLayout for the + header, PhoneListFilter for the bar - because neither is a constant. The header is fixed at + every height; the bar only within PhoneListFilter.vue's own query, so the two must stay on the + same breakpoint. + + The extra 0.5rem stops a target short of the bar rather than flush against it: a focus ring is + drawn outside the element's box (the browser's ring by 2-3px, the app's halo by 0.25rem), so a + flush target would have the top of its ring under the bar. */ +html:has(.phone-list-filter) { + scroll-padding-top: calc(var(--viper-header-height, 86px) + 0.5rem); +} + +@media (height >= 400px) { + html:has(.phone-list-filter) { + scroll-padding-top: calc(var(--viper-header-height, 86px) + var(--phone-list-filter-height, 56px) + 0.5rem); + } +} diff --git a/VueApp/src/Personnel/components/MobileCardList.vue b/VueApp/src/Personnel/components/MobileCardList.vue index 0c610d224..b33887edf 100644 --- a/VueApp/src/Personnel/components/MobileCardList.vue +++ b/VueApp/src/Personnel/components/MobileCardList.vue @@ -1,23 +1,33 @@ @@ -29,9 +40,15 @@ import { useElementSize } from "@vueuse/core" // native input's aria-label, so no separate aria-label is needed or wanted here. const search = defineModel({ required: true }) -// Published rather than guessed at, so .sticky-filter-offset can be exactly this bar plus the -// header above it. The height is not a constant: the field is a fixed 40px but the padding is in -// rem, and the root font-size steps from 14px to 16px at 768px. +defineProps<{ + /** Set by a read-only page when the search has emptied every table. Maintain pages leave it + * unset: their tables stay shown, each with its own empty line. */ + noMatches?: boolean +}>() + +// Enables the scroll padding in assets/phone-list.css to be exactly this bar plus the header +// above it. The height is not a constant since the padding is in rem and the root font-size changes +// at 768px, and the controls either share a line or stack depending on the width. const barRef = useTemplateRef("bar") const { height } = useElementSize(barRef, undefined, { box: "border-box" }) watchEffect(() => { @@ -41,18 +58,39 @@ watchEffect(() => { diff --git a/VueApp/src/Personnel/composables/section-jump-targets.ts b/VueApp/src/Personnel/composables/section-jump-targets.ts new file mode 100644 index 000000000..85d6f6fb8 --- /dev/null +++ b/VueApp/src/Personnel/composables/section-jump-targets.ts @@ -0,0 +1,73 @@ +import { buildFrequentNumberColumns } from "./svm-phone-columns" +import { filterRows } from "./use-mobile-table-rows" +import type { QTableColumn } from "quasar" +import type { JumpTarget } from "../components/SectionJumpLinks.vue" +import type { PhoneListUnit } from "../types/phone-list-phone-types" +import type { SVMFrequentNumberRecord, SVMPhoneSection } from "../types/svm-phone-types" + +/** + * The section jump links for the phone list pages, shared by the read-only and maintain pages of + * each list so the two build their links, and the heading ids those links point at, the same way. + */ + +/** A section as far as a jump link is concerned: where it lands, and what the search sees. */ +type JumpSection = { id: string; label: string; cols: QTableColumn[] | undefined; rows: unknown[] } + +const FREQUENT_NUMBERS_ANCHOR_ID = "phone-section-frequent-numbers" + +// Matched on the data columns only, which are the same whether or not the list can be edited. +const frequentNumberColumns = buildFrequentNumberColumns(false) + +function unitAnchorId(unitId: number): string { + return `phone-unit-${unitId}` +} + +function sectionAnchorId(sectionId: number): string { + return `phone-section-${sectionId}` +} + +function phoneListJumpSections(units: PhoneListUnit[]): JumpSection[] { + return units.map((unit) => ({ id: unitAnchorId(unit.id), label: unit.name, cols: unit.cols, rows: unit.rows })) +} + +/** Every SVM section, followed by the frequently called numbers, in page order. */ +function svmJumpSections(sections: SVMPhoneSection[], frequentNumbers: SVMFrequentNumberRecord[]): JumpSection[] { + return [ + ...sections.map((section) => ({ + id: sectionAnchorId(section.id), + label: section.title, + cols: section.cols, + rows: section.rows, + })), + { + id: FREQUENT_NUMBERS_ANCHOR_ID, + label: "Frequently Called Numbers", + cols: frequentNumberColumns, + rows: frequentNumbers, + }, + ] +} + +/** For a maintain page, where every section stays shown, with its add button, whatever the search. */ +function allJumpTargets(sections: JumpSection[]): JumpTarget[] { + return sections.map(({ id, label }) => ({ id, label })) +} + +/** + * For a read-only page, which hides a section the search empties: a link to one would go nowhere. + * Uses the same filterRows the lists themselves do, so a link is offered if and only if that + * section still has rows. + */ +function matchingJumpTargets(sections: JumpSection[], search: string): JumpTarget[] { + return allJumpTargets(sections.filter((section) => filterRows(section.cols, section.rows, search).length > 0)) +} + +export { + FREQUENT_NUMBERS_ANCHOR_ID, + allJumpTargets, + matchingJumpTargets, + phoneListJumpSections, + sectionAnchorId, + svmJumpSections, + unitAnchorId, +} diff --git a/VueApp/src/Personnel/composables/use-section-table.ts b/VueApp/src/Personnel/composables/use-section-table.ts new file mode 100644 index 000000000..c4d4e6790 --- /dev/null +++ b/VueApp/src/Personnel/composables/use-section-table.ts @@ -0,0 +1,40 @@ +import { computed, ref, toValue } from "vue" +import { Screen } from "quasar" +import { filterRows } from "./use-mobile-table-rows" +import type { MaybeRefOrGetter } from "vue" +import type { QTableColumn, QTableProps } from "quasar" + +/** + * Ensures the anchor ID is applied to the correct heading depending on screen size. + * Both the desktop and the mobile headings exist in the DOM, but only one is visible at a time, + * with lt.md as a breakpoint. Since IDs are unique, they must be applied only to the visible heading. + */ +function useHeadingAnchorIds(anchorId: () => string | undefined) { + return { + desktopAnchorId: computed(() => (Screen.lt.md ? undefined : anchorId())), + mobileAnchorId: computed(() => (Screen.lt.md ? anchorId() : undefined)), + } +} + +/** + * The state every phone list table keeps across its two renderings - the desktop QTable and the + * mobile card list - kept in one place so the tables cannot drift apart on it. + */ +function useSectionTable(options: { + columns: MaybeRefOrGetter + rows: MaybeRefOrGetter + search: MaybeRefOrGetter + anchorId: () => string | undefined +}) { + // Bound to the table, and shared with the card list's sort control. + const pagination = ref({ rowsPerPage: 0, sortBy: null, descending: false }) + + // The same filterRows the card list and the page's jump links use, so all three agree on it. + const hasMatches = computed( + () => filterRows(toValue(options.columns), toValue(options.rows), toValue(options.search)).length > 0, + ) + + return { pagination, hasMatches, ...useHeadingAnchorIds(options.anchorId) } +} + +export { useSectionTable } diff --git a/VueApp/src/Personnel/pages/PhoneList.vue b/VueApp/src/Personnel/pages/PhoneList.vue index d4cbd83cf..8d7af61e3 100644 --- a/VueApp/src/Personnel/pages/PhoneList.vue +++ b/VueApp/src/Personnel/pages/PhoneList.vue @@ -22,7 +22,13 @@ Updated {{ formatDate(updatedDate?.toString() ?? "") || "Never" }}
Click on a name to send an email
- + + + +