Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions frontend/src/components/browser/toolbars/top/filter-by-select.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
:items="bookmarkChoices"
:menu-props="{
contentClass: filterMenuClass,
contentProps: { onKeydownCapture: onMenuKeydownCapture },
maxHeight: undefined,
closeOnContentClick: false,
}"
Expand Down Expand Up @@ -93,6 +94,14 @@ import ToolbarSelect from "@/components/toolbar-select.vue";
import { useAuthStore } from "@/stores/auth";
import { useBrowserStore } from "@/stores/browser";

const ARROW_STEPS = Object.freeze({ ArrowDown: 1, ArrowUp: -1 });
/*
* Vuetify focuses list rows programmatically with ``tabindex="-2"`` and
* parks the list itself at ``-1``, so excluding only ``-1`` selects exactly
* the rows its own focus walk would visit.
*/
const FOCUSABLE_ROW = '[tabindex]:not([tabindex="-1"]):not([disabled])';

export default {
name: "BrowserFilterBySelect",
components: {
Expand Down Expand Up @@ -244,6 +253,39 @@ export default {
this.loadAvailableFilterChoices();
}
},
/*
* Vuetify 4.2 moved select keyboard navigation into ``useScrolling``,
* which wraps from the last bookmark row straight back to the first and
* calls ``stopImmediatePropagation()``. That pre-empts VList's own focus
* walk, which is what used to carry the user into the rows this menu
* adds through the prepend/append slots ("Clear All Filters",
* "Favorites Only" and the filter sub-menus), leaving them mouse-only.
* A capture listener on the overlay content runs before the list's, so
* stepping off either end of the bookmark rows lands on the adjacent
* slot row instead of wrapping. Everywhere else the event is left alone
* and Vuetify still owns the navigation.
*/
onMenuKeydownCapture(event) {
const step = ARROW_STEPS[event.key];
const content = event.currentTarget;
const row = document.activeElement?.closest?.("[aria-posinset]");
if (!step || !row || !content.contains(row)) {
return;
}
// Only the true ends of the bookmark list wrap; mid-list rows are fine.
const end = step > 0 ? Number(row.getAttribute("aria-setsize")) : 1;
if (Number(row.getAttribute("aria-posinset")) !== end) {
return;
}
const rows = [...content.querySelectorAll(FOCUSABLE_ROW)];
const target = rows[rows.indexOf(row) + step];
if (!target || target.hasAttribute("aria-posinset")) {
return;
}
event.preventDefault();
event.stopPropagation();
target.focus();
},
},
};
</script>
Expand Down
5 changes: 0 additions & 5 deletions frontend/src/components/reader/book-change-drawer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
:location="drawerLocation"
:model-value="isDrawerOpen"
:scrim="false"
:class="{ drawerActivated: isDrawerOpen }"
temporary
touchless
>
Expand Down Expand Up @@ -94,10 +93,6 @@ export default {
.bookChangeDrawer {
opacity: 0.75 !important;
z-index: 15 !important;
}

.drawerActivated {
// Deactivated drawers with custom width don't move off the screen enough
width: col.$change-column-width !important;
}

Expand Down
6 changes: 6 additions & 0 deletions frontend/src/components/settings/button.vue
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,11 @@ export default {
<style scoped lang="scss">
#settingsDrawerButton {
padding-right: max(10px, calc(env(safe-area-inset-right) / 2));
/*
* Vuetify 4.2 clips .v-btn (fix(variant) #22992). The admin librarian
* progress ring is larger than the compact button box on xs, so without
* this it loses its left and bottom arcs on phones.
*/
overflow: visible;
}
</style>
108 changes: 108 additions & 0 deletions frontend/tests/unit/book-change-drawer.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Tests for ``book-change-drawer.vue`` — the prev/next book slide-outs.
*
* The drawer is 33vw, not Vuetify's 256px default. Vuetify 4.1 parked an
* inactive layout item at ``translateX(-(width prop + 1)px)``, which left a
* 33vw drawer partly on screen, so the width used to be applied only while
* open (commit 24f8d1bd6). Vuetify 4.2 parks it at ``calc(±100% ± 1px)`` of
* its own rendered box, so the width is now unconditional. These tests pin
* the offscreen transform that makes that safe.
*/
import { createTestingPinia } from "@pinia/testing";
import { flushPromises, mount } from "@vue/test-utils";
import { afterEach, describe, expect, test } from "vitest";

import { VApp } from "vuetify/components";

import BookChangeDrawer from "@/components/reader/book-change-drawer.vue";
import vuetify from "@/plugins/vuetify";

const MAX_PAGE = 10;

let wrappers = [];

async function mountDrawer(direction, { bookChange } = {}) {
const pinia = createTestingPinia({
// The drawer's location, icon and visibility all come from store actions.
stubActions: false,
initialState: {
reader: {
// Each drawer only shows at its own end of the book.
page: direction === "prev" ? 0 : MAX_PAGE,
bookChange,
books: {
current: { maxPage: MAX_PAGE },
prev: { pk: 1 },
next: { pk: 3 },
},
routes: {
books: { prev: { pk: 1, page: 0 }, next: { pk: 3, page: 0 } },
},
},
},
});
/*
* vite-plugin-vuetify's autoImport only rewrites SFC templates, so a
* runtime-compiled one has to register VApp itself. The drawer is a
* layout item and throws without it.
*/
const Host = {
components: { BookChangeDrawer, VApp },
props: { direction: { type: String, required: true } },
template: `
<VApp>
<BookChangeDrawer :direction="direction" />
</VApp>
`,
};
const wrapper = mount(Host, {
attachTo: document.body,
props: { direction },
global: { plugins: [pinia, vuetify], stubs: { RouterLink: true } },
});
wrappers.push(wrapper);
await flushPromises();
return wrapper.find(".v-navigation-drawer");
}

afterEach(() => {
for (const wrapper of wrappers) {
wrapper.unmount();
}
wrappers = [];
});

describe("BookChangeDrawer — offscreen when closed", () => {
test("the previous-book drawer parks a full width to the left", async () => {
const drawer = await mountDrawer("prev");

expect(drawer.exists()).toBe(true);
expect(drawer.attributes("style")).toContain(
"translateX(calc(-100% + -1px))",
);
});

test("the next-book drawer parks a full width to the right", async () => {
const drawer = await mountDrawer("next");

expect(drawer.attributes("style")).toContain(
"translateX(calc(100% + 1px))",
);
});

test("an open drawer is not translated", async () => {
const drawer = await mountDrawer("prev", { bookChange: "prev" });

expect(drawer.attributes("style")).toContain("translateX(0px)");
});

test("the width class is applied regardless of open state", async () => {
const closed = await mountDrawer("prev");
const open = await mountDrawer("prev", { bookChange: "prev" });

for (const drawer of [closed, open]) {
expect(drawer.classes()).toContain("bookChangeDrawer");
expect(drawer.classes()).not.toContain("drawerActivated");
}
});
});
151 changes: 151 additions & 0 deletions frontend/tests/unit/filter-by-select.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
* Tests for ``filter-by-select.vue`` — the browser's "filter by" menu.
*
* Both cases guard behavior that Vuetify 4.2 changed underneath this
* component:
* - ``useSelectionMenu.closeOnSelect()`` now bails when ``menuProps``
* carries ``closeOnContentClick: false``, so ``onSubMenuSelected`` is
* the only thing left that closes this menu after a pick.
* - the list keydown capture added by ``useScrolling`` wraps from either
* end of the bookmark rows and stops propagation, which used to strand
* prepend/append slot rows ("Clear All Filters", "Favorites Only", the
* filter sub-menus) with no keyboard route in. ``onMenuKeydownCapture``
* hands those two edge steps to the adjacent slot row instead.
*
* Real Vuetify is mounted so the overlay, list and its keyboard handlers
* are the ones that ship; store actions are stubbed by createTestingPinia.
*/
import { createTestingPinia } from "@pinia/testing";
import { flushPromises, mount } from "@vue/test-utils";
import { afterEach, beforeAll, describe, expect, test } from "vitest";

import FilterBySelect from "@/components/browser/toolbars/top/filter-by-select.vue";
import vuetify from "@/plugins/vuetify";
import { useBrowserStore } from "@/stores/browser";

beforeAll(() => {
/*
* VOverlay's connected location strategy reads the bare global; happy-dom
* has no visual viewport, so the menu never positions without this.
*/
globalThis.visualViewport ??= {
width: 1024,
height: 768,
offsetLeft: 0,
offsetTop: 0,
scale: 1,
addEventListener() {},
removeEventListener() {},
};
});

let wrappers = [];

async function mountOpenMenu({ bookmark = "UNREAD", loggedIn = true } = {}) {
const pinia = createTestingPinia({
initialState: {
auth: { user: loggedIn ? { pk: 1 } : undefined },
browser: {
filterMode: "base",
// A non-default bookmark makes the "Clear All Filters" row render.
settings: { filters: { bookmark } },
choices: { dynamic: { characters: true } },
},
},
});
const wrapper = mount(FilterBySelect, {
attachTo: document.body,
global: { plugins: [pinia, vuetify] },
});
wrappers.push(wrapper);
wrapper.vm.menu = true;
await flushPromises();
const content = document.querySelector(".v-overlay__content");
const rows = [...content.querySelectorAll("[aria-posinset]")];
return { wrapper, content, rows, browserStore: useBrowserStore() };
}

afterEach(() => {
for (const wrapper of wrappers) {
wrapper.unmount();
}
wrappers = [];
});

describe("BrowserFilterBySelect — closing on select", () => {
test("picking a bookmark applies it and closes the menu", async () => {
const { wrapper, rows, browserStore } = await mountOpenMenu();
const inProgress = rows.find((row) =>
row.textContent.includes("In Progress"),
);

inProgress.click();
await flushPromises();

expect(browserStore.setSettings).toHaveBeenCalledWith({
filters: { bookmark: "IN_PROGRESS" },
});
expect(wrapper.vm.menu).toBe(false);
});
});

describe("BrowserFilterBySelect — keyboard reach into the slot rows", () => {
const arrow = (el, key) =>
el.dispatchEvent(
new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }),
);

test("ArrowDown off the last bookmark row lands on Favorites Only", async () => {
const { content, rows } = await mountOpenMenu();
const last = rows.at(-1);

last.focus();
arrow(last, "ArrowDown");

expect(document.activeElement).toBe(
content.querySelector(".favoritesOnly"),
);
});

test("ArrowUp off the first bookmark row lands on Clear All Filters", async () => {
const { content, rows } = await mountOpenMenu();
const first = rows[0];

first.focus();
arrow(first, "ArrowUp");

expect(document.activeElement).toBe(content.querySelector(".clearFilter"));
});

test("mid-list rows are left to Vuetify", async () => {
const { content, rows } = await mountOpenMenu();
const middle = rows[1];
let reached = 0;
middle.addEventListener("keydown", () => (reached += 1));

middle.focus();
arrow(middle, "ArrowDown");

/*
* Interception stops the event at the overlay content, so an untouched
* event is one that still reaches the row Vuetify navigates from.
*/
expect(reached).toBe(1);
expect(document.activeElement).not.toBe(
content.querySelector(".favoritesOnly"),
);
});

test("logged out, the last row steps to the first filter sub-menu", async () => {
const { content, rows } = await mountOpenMenu({ loggedIn: false });
const last = rows.at(-1);

last.focus();
arrow(last, "ArrowDown");

// Not a bookmark row: it stepped past the list instead of wrapping.
expect(content.querySelector(".favoritesOnly")).toBeNull();
expect(content.contains(document.activeElement)).toBe(true);
expect(document.activeElement.hasAttribute("aria-posinset")).toBe(false);
});
});