fix: tag counts, chip focus, upload naming, share links and unsized clips hold after round 2

- A tag counts the followed channels the Subscriptions tab shows: hidden ones are left out, and a
  subscribe, unsubscribe, hide or tag toggle refreshes the count. The Tag manager's row and its
  hover list count the same channels as the chip; /api/tags sends both counts under their names.
- A chip's count keeps the chip's colour after a dot (a dimmed count fell under 4.5:1). A pick in
  the open "+N" fold closes it; a chip that leaves the list hands the focus to the one in its place.
  The fold is a dialog of toggle buttons, not an empty menu.
- Upload: a folder name left by a press is made once the press is over, if the dialog is still
  open, instead of guessing a dismissal from Modal's markup. While the folder is made, its place
  holds the focus and a live region says so.
- Share: an expired link has no Copy and a warn badge; the link made on open goes into the cached
  list, so a reopen makes no second one; the New link form survives the link made on open.
- Watch: a repeated Enter sends one unlock while one is on its way. See all buttons are named with
  their rail.
- An unsized clip of a rotated file gets the upright size its crop cut.
This commit is contained in:
2026-11-06 02:20:52 +01:00
parent 2baad2ebae
commit 4d1cf66d96
30 changed files with 526 additions and 128 deletions
+10
View File
@@ -201,6 +201,16 @@ def _ratio(v: object) -> float | None:
return n / d if n > 0 and d > 0 else None
def quarter_turned(probe: dict) -> bool | None:
"""Whether the first video stream of an ffprobe `-show_streams` JSON is stored turned a quarter
(ffmpeg stands it upright before any filter, so its filters see width and height swapped). None
when there is no video stream."""
for s in probe.get("streams") or []:
if s.get("codec_type") == "video":
return _quarter_turned(s)
return None
def _quarter_turned(stream: dict) -> bool:
rotation: object = (stream.get("tags") or {}).get("rotate")
for sd in stream.get("side_data_list") or []:
+4 -3
View File
@@ -89,9 +89,10 @@ def crop_filter(crop: dict) -> str:
def crop_stored_size(crop: dict, src_w: int | None, src_h: int | None) -> tuple[int, int] | None:
"""The stored size `crop_filter` cuts out of a `src_w` x `src_h` file, evened as `EVEN_FRAME`
evens it — the clip's size when ffprobe cannot tell. A crop counted in the shown picture is a
fraction of the file's frame, not its own pixel count; None when the source size is unknown."""
"""The size `crop_filter` cuts out of a `src_w` x `src_h` frame, evened as `EVEN_FRAME` evens it —
the clip's size when ffprobe cannot tell. The frame is the one ffmpeg's filters see: a rotated
file's stood upright, its stored size turned (the caller's to turn). A crop counted in the shown
picture is a fraction of that frame, not its own pixel count; None when the size is unknown."""
if crop.get("frame_w") and crop.get("frame_h"):
if not src_w or not src_h:
return None
+18 -1
View File
@@ -837,6 +837,18 @@ def _fail_edit(
)
def _filter_frame(
path: Path, width: int | None, height: int | None
) -> tuple[int | None, int | None]:
"""A file's frame as ffmpeg's filters see it: the stored size, turned when the stream is stored
turned a quarter (ffmpeg stands it upright first). Unknown when the file cannot be read."""
data = probe.probe_json(path)
turned = clipmod.quarter_turned(data) if data else None
if turned is None:
return None, None
return (height, width) if turned else (width, height)
def _process_edit(job_id: int, asset_id: int) -> None:
"""Produce a per-user trim/crop clip from the source asset's downloaded file via ffmpeg."""
with SessionLocal() as db:
@@ -905,6 +917,12 @@ def _process_edit(job_id: int, asset_id: int) -> None:
size = final.stat().st_size if final.exists() else None
# The clip's real frame: a re-encode evens an odd crop or source (`editmod.EVEN_FRAME`).
_, _, out_w, out_h, _ = probe.probe_media(final)
# Unsized, a crop's own size stands in, cut from the frame the filters saw.
cut = (
editmod.crop_stored_size(edit_spec["crop"], *_filter_frame(src_path, src_w, src_h))
if crop and not (out_w and out_h)
else None
)
with SessionLocal() as db:
asset = db.get(MediaAsset, asset_id)
@@ -914,7 +932,6 @@ def _process_edit(job_id: int, asset_id: int) -> None:
asset.size_bytes = size
asset.container = out_ext
asset.duration_s = total_s or asset.duration_s
cut = editmod.crop_stored_size(edit_spec["crop"], src_w, src_h) if crop else None
if out_w and out_h:
asset.width, asset.height = out_w, out_h
elif cut:
+5 -3
View File
@@ -44,9 +44,11 @@ def video(db: Session, video_id: str) -> Video | None:
def tags_with_counts(db: Session, user: User) -> list[tuple[Tag, int, int]]:
"""The tags this user sees (the shared system ones and their own), by category then name, each
with how many channel links it has and how many of the user's own links sit on a channel they
subscribe to — the rows the Channels page's tag filter shows. A link outlives an unsubscribe,
so the two differ."""
subscribed = select(Subscription.channel_id).where(owned_by(Subscription.user_id, user))
subscribe to and have not hidden — the rows the Channels page's tag filter shows (a hidden
channel has its own tab). A link outlives an unsubscribe and a hide, so the two differ."""
subscribed = select(Subscription.channel_id).where(
owned_by(Subscription.user_id, user), Subscription.hidden.is_(False)
)
on_subscribed = dict(
db.execute(
select(ChannelTag.tag_id, func.count(ChannelTag.id))
+21
View File
@@ -178,6 +178,27 @@ def test_a_clip_ffprobe_cannot_size_gets_the_size_its_crop_cut(
assert (asset.width, asset.height) == (160, 120)
def test_a_rotated_clip_ffprobe_cannot_size_gets_the_upright_size_its_crop_cut(
db, make_user, tmp_path, monkeypatch
):
"""The frame was drawn on the upright picture, which is the stored one turned: so is its cut."""
monkeypatch.setattr(settings, "files_root", str(tmp_path))
src = tmp_path / "in.mp4"
_source(src, "320x240", "x=0:y=0:w=160:h=240", sar=1, rotate=90)
def probe_fails():
monkeypatch.setattr(pipeline.probe, "probe_media", lambda _p: (None,) * 5)
asset = _cut_asset(
db, make_user, src,
{"crop": {"x": 0, "y": 160, "w": 240, "h": 160, "frame_w": 240, "frame_h": 320}},
then=probe_fails,
) # fmt: skip
# What ffmpeg cut (the rotated test above): 240 x 160, not the stored 320 x 120.
assert (asset.width, asset.height) == (240, 160)
@pytest.mark.parametrize("key", ["frame_w", "x"])
def test_an_infinite_number_in_a_crop_is_dropped_not_a_crash(key):
crop = {"x": 4, "y": 6, "w": 100, "h": 50, "frame_w": 640, "frame_h": 240}
+20 -6
View File
@@ -66,22 +66,36 @@ def test_a_system_tag_is_nobodys_to_edit(db, make_user):
# --- tags ------------------------------------------------------------------------------------
def test_a_tag_counts_apart_the_links_on_channels_the_user_still_follows(db, make_user):
"""An unsubscribe keeps the channel's tag links; the Channels page's filter lists only followed
channels, so its count must not hold the others."""
def _music_on_four(db, make_user):
"""A tag on four channels: two followed, one followed but hidden, one no longer followed."""
user, other = make_user(), make_user()
tag = service.create_tag(db, user, "Music", "other", None)
for cid in ("UCa", "UCb", "UCgone"):
for cid in ("UCa", "UCb", "UChid", "UCgone"):
db.add(Channel(id=cid, title=cid))
db.flush()
for cid in ("UCa", "UCb"):
db.add(Subscription(user_id=user.id, channel_id=cid))
db.add(Subscription(user_id=user.id, channel_id="UChid", hidden=True))
db.add(Subscription(user_id=other.id, channel_id="UCgone"))
for cid in ("UCa", "UCb", "UCgone"):
for cid in ("UCa", "UCb", "UChid", "UCgone"):
db.add(ChannelTag(channel_id=cid, tag_id=tag.id, user_id=user.id, source="user"))
db.commit()
return user, tag
def test_a_tag_counts_apart_the_links_on_channels_the_user_still_follows(db, make_user):
"""An unsubscribe and a hide keep the channel's tag links; the Channels page's filter lists only
the followed channels not hidden, so its count must not hold the others."""
user, tag = _music_on_four(db, make_user)
[(_, count, subscribed)] = [r for r in queries.tags_with_counts(db, user) if r[0].id == tag.id]
assert (count, subscribed) == (3, 2)
assert (count, subscribed) == (4, 2)
def test_the_tag_list_sends_each_count_under_its_own_name(db, make_user, as_user):
"""Both counts are ints, so a swap would type-check: the answer itself pins which is which."""
user, tag = _music_on_four(db, make_user)
[row] = [t for t in as_user(user).get("/api/tags").json() if t["id"] == tag.id]
assert (row["channel_count"], row["subscribed_count"]) == (4, 2)
def test_a_tag_is_made_once_per_name_and_an_unknown_category_is_other(db, make_user):
+3
View File
@@ -866,3 +866,6 @@ of a full-width panel and nothing on the right.
| 2026-11-04 | Tag chip counts (owner D5a) | A Channels tag chip shows how many followed channels it holds: a muted number after the name, no badge | The number in brackets after the name | The owner's pick; the number is a detail of the name, so it takes the name's own text, muted, rather than a `CountBadge` |
| 2026-11-05 | Picked tag chips lead the row (owner c) | On the Channels tag row the picked chips come first, in the tags' order, then the rest | Keep the order and fold the unpicked first; mark the "+N" when it holds a pick | The row folds from its end, so a picked tag late in the list sat only inside "+N" with nothing on the row saying a filter was on |
| 2026-11-05 | A toggled tag chip keeps the focus (owner a) | A chip that moves when toggled takes the keyboard focus with it: to its new place, or to "+N" when it went into the closed fold | Keep a picked chip in the "+N" popover until it closes | The row always shows the filters as they are; the focus, not the order, was what got lost |
| 2026-11-06 | Tag chip count after a dot (owner a) | The count keeps the chip's own colour, set off from the name by a "·" | The number dimmed (it fell under the 4.5:1 text floor, resting and pressed); the number in a tinted inner badge | Contrast holds in every scheme without a new token, and the dot still reads the number as a detail of the name |
| 2026-11-06 | A pick in the open fold closes it (owner a) | Toggling a chip inside the "+N" popover closes the popover; the focus goes with the chip | Keep the popover open and focus the chip now in the picked one's place | The focus never sits outside a popover left open, and a picked chip leads the row anyway |
| 2026-11-06 | An expired share link offers no Copy (owner a) | An expired row in the share dialog has a warn badge "expired <date>" and its address struck through; Open and Revoke stay, Copy goes | The warn badge alone, Copy kept | An expired link opens nothing, so it is no link to hand out; Open still lets the owner check it |
@@ -9,7 +9,13 @@ import { act, render, screen, fireEvent, cleanup } from "@testing-library/react"
vi.mock("react-i18next", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-i18next")>();
return { ...actual, useTranslation: () => ({ t: (k: string) => k }) };
return {
...actual,
useTranslation: () => ({
t: (k: string, o?: { action?: string; title?: string }) =>
o?.title ? `${o.action}: ${o.title}` : k,
}),
};
});
import { Carousel } from "./Carousel";
@@ -54,7 +60,10 @@ describe("Carousel", () => {
);
expect(screen.getByText("card-a")).toBeTruthy();
expect(screen.getByRole("heading", { name: "Recently added" })).toBeTruthy();
fireEvent.click(screen.getByRole("button", { name: "common.seeAll" }));
// Named with its rail, the visible words first: a page of rails has a See all on each.
const seeAll = screen.getByRole("button", { name: "common.seeAll: Recently added" });
expect(seeAll.textContent).toBe("common.seeAll");
fireEvent.click(seeAll);
expect(onSeeAll).toHaveBeenCalledOnce();
});
+15 -1
View File
@@ -109,7 +109,21 @@ export function Carousel({
<div className="mb-3 flex items-baseline gap-2">
<h2 className="text-sm font-semibold">{title}</h2>
{onSeeAll && (
<Button variant="link-muted" onClick={onSeeAll} className="ml-auto gap-0.5 text-xs">
<Button
variant="link-muted"
onClick={onSeeAll}
// Named with its rail: a page of rails has a See all on each, and a screen reader's
// list of buttons (or a voice command) must tell them apart. The visible words lead.
aria-label={
label
? t("common.seeAllOf", {
action: seeAllLabel ?? t("common.seeAll"),
title: label,
})
: undefined
}
className="ml-auto gap-0.5 text-xs"
>
{seeAllLabel ?? t("common.seeAll")}
<ChevronRight className="h-3.5 w-3.5" />
</Button>
-1
View File
@@ -231,7 +231,6 @@ export default function Modal({
size="sm"
variant="ghost"
onClick={onClose}
data-dismiss=""
>
<X className="w-4 h-4" />
</IconButton>
@@ -0,0 +1,69 @@
// @vitest-environment jsdom
//
// A tag's row in the manager says how many channels it is on, beside a hover list of them, over the
// Channels page whose chip names the same tag with a number: all three count the same channels —
// the followed ones the Subscriptions tab shows, hidden ones left out. Every link (the number a
// delete removes) is only the delete's confirm.
import { describe, it, expect, vi, beforeAll, afterEach } from "vitest";
import { render, screen, cleanup, fireEvent } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
vi.mock("react-i18next", async (importOriginal) => ({
...(await importOriginal<typeof import("react-i18next")>()),
useTranslation: () => ({
t: (key: string, o?: { count?: number }) => (o?.count != null ? `${key}:${o.count}` : key),
i18n: { language: "en" },
}),
}));
import TagManager from "./TagManager";
import { api, type ManagedChannel, type Tag } from "../../lib/api";
beforeAll(() => {
class Stub {
observe() {}
unobserve() {}
disconnect() {}
takeRecords() {
return [];
}
}
globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
});
afterEach(() => {
cleanup();
vi.restoreAllMocks();
});
const tag = {
id: 5,
name: "Music",
color: null,
category: "other",
system: false,
channel_count: 4,
subscribed_count: 2,
} as Tag;
const ch = (id: string, hidden: boolean) =>
({ id, title: id, hidden, tag_ids: [5] }) as unknown as ManagedChannel;
describe("TagManager · a tag's count", () => {
it("counts the followed channels the Subscriptions tab shows, not every link", async () => {
vi.spyOn(api, "tags").mockResolvedValue([tag]);
vi.spyOn(api, "channels").mockResolvedValue([
ch("UCa", false),
ch("UCb", false),
ch("UChid", true),
]);
render(
<QueryClientProvider client={new QueryClient()}>
<TagManager onClose={() => {}} onFocusChannel={() => {}} />
</QueryClientProvider>,
);
expect(await screen.findByText("tagManager.channels:2")).toBeTruthy();
expect(screen.queryByText("tagManager.channels:4")).toBeNull();
fireEvent.mouseEnter(screen.getByText("tagManager.channels:2"));
expect(await screen.findByText("UCa")).toBeTruthy();
expect(screen.queryByText("UChid")).toBeNull();
});
});
+2 -2
View File
@@ -71,7 +71,7 @@ function TagRow({
: ""
}`}
>
{t("tagManager.channels", { count: tag.channel_count })}
{t("tagManager.channels", { count: tag.subscribed_count })}
</span>
<IconButton variant="danger" onClick={onDelete} label={t("tagManager.delete")}>
<Trash2 />
@@ -119,7 +119,7 @@ export default function TagManager({
const userTags = (tagsQuery.data ?? []).filter((tg) => !tg.system);
const channelsForTag = (tagId: number) =>
(channelsQuery.data ?? [])
.filter((c) => c.tag_ids.includes(tagId))
.filter((c) => !c.hidden && c.tag_ids.includes(tagId))
.map((c) => ({ id: c.id, title: c.title ?? c.id }));
const invalidate = () => {
+1
View File
@@ -28,6 +28,7 @@
"loadError": "Couldn't load this — check your connection and try again",
"deletedTag": "Deleted tag",
"seeAll": "See all",
"seeAllOf": "{{action}}: {{title}}",
"scrollLeft": "Scroll left",
"scrollRight": "Scroll right",
"search": "Search",
+1
View File
@@ -28,6 +28,7 @@
"loadError": "Nem sikerült betölteni — ellenőrizd a kapcsolatot, és próbáld újra",
"deletedTag": "Törölt címke",
"seeAll": "Összes",
"seeAllOf": "{{action}}: {{title}}",
"scrollLeft": "Görgetés balra",
"scrollRight": "Görgetés jobbra",
"search": "Keresés",
+2 -1
View File
@@ -10,7 +10,8 @@ export interface Tag {
/** Every channel link it has, a channel the user no longer follows included (an unsubscribe keeps
* the link) — what deleting the tag removes. */
channel_count: number;
/** The user's own links on channels they follow: the rows the Channels page's filter shows. */
/** The user's own links on channels they follow and have not hidden: the rows the Channels page's
* filter shows (a hidden channel has its own tab). */
subscribed_count: number;
}
@@ -18,6 +18,7 @@ import { api, HttpError } from "./api";
import { msg, notify } from "./notifications";
import { useChannelTagActions } from "./useChannelTagActions";
import { useChannelPriority } from "./useChannelPriority";
import { qk } from "./queryKeys";
const notifyMock = vi.mocked(notify);
beforeEach(() => notifyMock.mockClear());
@@ -44,6 +45,23 @@ describe("useChannelTagActions", () => {
});
});
// A tag counts the followed channels it is on: a toggle that lands changes that number.
it("refreshes the tags once a toggle lands", async () => {
vi.spyOn(api, "attachChannelTag").mockResolvedValue({ ok: true } as never);
const qc = new QueryClient();
const spy = vi.spyOn(qc, "invalidateQueries");
const { result } = renderHook(
() => useChannelTagActions({ optimisticToggle: () => {}, onError: () => {} }),
{
wrapper: ({ children }) => (
<QueryClientProvider client={qc}>{children}</QueryClientProvider>
),
},
);
act(() => result.current.toggleTag("UC1", 7, false));
await waitFor(() => expect(spy.mock.calls.map(([f]) => f?.queryKey)).toContainEqual(qk.tags()));
});
it("names a refused create-and-attach", async () => {
vi.spyOn(api, "createTag").mockRejectedValue(new HttpError(429, "slow"));
const { result } = renderHook(
@@ -21,9 +21,23 @@ describe("a subscription change", () => {
qk.feedSummary(),
qk.discoveredChannels(),
qk.watchedChannels(),
qk.tags(),
]);
});
// A tag counts the followed channels it is on, not hidden: a hide changes the number as an
// unsubscribe does, and both places that hide a channel must refresh it.
it("a hide refreshes the tags wherever it is made", () => {
const root = new URL("../", import.meta.url);
const sites = readdirSync(root, { recursive: true, encoding: "utf8" })
.map((f) => f.split("\\").join("/"))
.filter((f) => f.endsWith(".tsx") && !f.includes(".test.") && !f.startsWith("design-lab/"))
.map((f) => [f, readFileSync(new URL(f, root), "utf8")] as const)
.filter(([, src]) => /api\.updateChannel\([^)]*hidden/.test(src));
expect(sites.length).toBeGreaterThanOrEqual(2);
for (const [f, src] of sites) expect(src, f).toContain("queryKey: qk.tags()");
});
it("is refreshed through that one function wherever it is made", () => {
const root = new URL("../", import.meta.url);
const sites = readdirSync(root, { recursive: true, encoding: "utf8" })
+3 -2
View File
@@ -3,8 +3,8 @@ import type { QueryClient } from "@tanstack/react-query";
import { qk } from "./queryKeys";
/** Everything a subscribe or an unsubscribe changes: the channel itself, the manager's list and its
* counts, the feed and its facets (the channel's videos come or go), and the two discovery lists (a
* subscribed channel leaves them). ONE set for every site that makes the change: each used to keep
* counts, the feed and its facets (the channel's videos come or go), the two discovery lists (a
* subscribed channel leaves them), and the tags (a tag counts the followed channels it is on). ONE set for every site that makes the change: each used to keep
* its own list, and each list missed something. */
export function invalidateSubscription(qc: QueryClient, channelId: string): void {
for (const queryKey of [
@@ -15,6 +15,7 @@ export function invalidateSubscription(qc: QueryClient, channelId: string): void
qk.feedSummary(),
qk.discoveredChannels(),
qk.watchedChannels(),
qk.tags(),
])
void qc.invalidateQueries({ queryKey });
}
+5 -1
View File
@@ -28,7 +28,11 @@ export function useChannelTagActions(opts: {
? api.detachChannelTag(channelId, tagId)
: api.attachChannelTag(channelId, tagId);
call
.then(() => opts.onSuccess?.(channelId))
.then(() => {
// A tag counts the followed channels it is on: the toggle changed that number.
void qc.invalidateQueries({ queryKey: qk.tags() });
opts.onSuccess?.(channelId);
})
.catch((err: unknown) => {
notifyActionError(err, msg("channels.tags.updateFailed"));
opts.onError(channelId);
@@ -38,7 +38,7 @@ const chips = [chip("alpha"), chip("beta"), chip("gamma")];
// The surface mounts with `visibility: hidden` until floating-ui has positioned it, which never
// happens in jsdom (every box measures 0) — so it is out of the accessibility tree and only a
// `hidden: true` query can see it. Presence is the whole question here, not visibility.
const menu = () => screen.queryByRole("menu", { hidden: true });
const menu = () => screen.queryByRole("dialog", { hidden: true });
const moreBtn = () => screen.queryByRole("button", { name: /channels.tags.moreCount/ });
afterEach(() => {
@@ -112,8 +112,10 @@ describe("TagFilterChips counts", () => {
);
const name = (label: string) =>
screen.getAllByRole("button").find((b) => b.textContent?.startsWith(label));
expect(name("alpha")?.textContent).toBe("alpha42");
expect(name("beta")?.textContent).toBe("beta0");
expect(name("alpha")?.textContent).toBe("alpha · 42");
expect(name("beta")?.textContent).toBe("beta · 0");
// In the chip's own colour: a dimmed count falls under the text contrast floor.
expect(name("alpha")?.querySelector('[class*="opacity"]')).toBeNull();
});
it("draws nothing after the name of a chip that has no count", () => {
@@ -125,8 +127,8 @@ describe("TagFilterChips counts", () => {
});
});
// The channel manager puts the picked chips first, so a toggle moves its chip: along the row, out
// of the "+N" fold, or into it. The keyboard's focus goes with the chip, not to <body>.
// The channel manager puts the picked chips first, so a toggle moves its chip: out of the "+N" fold,
// into it, or out of the list. The keyboard's focus goes with the chip, not to <body>.
describe("TagFilterChips focus on a toggle that moves the chip", () => {
function Row({ start }: { start: string[] }) {
const [picked, setPicked] = useState(start);
@@ -146,17 +148,9 @@ describe("TagFilterChips focus on a toggle that moves the chip", () => {
const named = (label: string) =>
screen.getAllByRole("button", { hidden: true }).find((b) => b.textContent === label)!;
it("along the row: the chip keeps the focus where it now stands", () => {
fit.visible = 3;
render(<Row start={[]} />);
press(named("gamma"));
expect(screen.getAllByRole("button").map((b) => b.textContent)).toEqual([
"gamma",
"alpha",
"beta",
]);
expect(document.activeElement).toBe(named("gamma"));
});
// Along the row needs no test of its own: a keyed move within one parent keeps its node, and
// React gives the focus back to it after the commit. Only a remount (row ↔ fold) or a chip that
// leaves the list loses it.
it("out of the fold: picked in the popover, it is focused on the row", () => {
fit.visible = 1;
@@ -167,13 +161,41 @@ describe("TagFilterChips focus on a toggle that moves the chip", () => {
const onRow = named("beta");
expect(onRow).not.toBe(inFold);
expect(document.activeElement).toBe(onRow);
// …and the fold is shut: the focus is not left outside a popover still open.
expect(menu()).toBeNull();
expect(moreBtn()).toHaveProperty("ariaExpanded", "false");
});
// A stored pick of a tag that no longer exists is an active chip that an un-pick removes for good.
function WithGhost() {
const [ghost, setGhost] = useState(true);
const rest = ["alpha", "beta"].map(chip);
return (
<TagFilterChips
chips={
ghost
? [{ ...chip("ghost"), active: true, onToggle: () => setGhost(false) }, ...rest]
: rest
}
/>
);
}
it.each([
["with room for all", 3],
["with a fold", 1],
])("a chip that leaves the list hands the focus to the one now in its place (%s)", (_, n) => {
fit.visible = n;
render(<WithGhost />);
press(named("ghost"));
expect(document.activeElement).toBe(named("alpha"));
});
it("into a closed fold: the focus goes to the +N that now holds it", () => {
fit.visible = 1;
render(<Row start={["gamma"]} />);
press(named("gamma"));
expect(named("alpha").closest('[role="menu"]')).toBeNull(); // alpha leads the row now
expect(named("alpha").closest('[role="dialog"]')).toBeNull(); // alpha leads the row now
expect(document.activeElement).toBe(moreBtn());
});
@@ -34,19 +34,32 @@ function ChipText({ chip }: { chip: FilterChip }) {
return (
<>
{chip.label}
{chip.count != null && <span className="ml-1 opacity-70 tabular-nums">{chip.count}</span>}
{/* The count in the chip's own colour, set off by a dot rather than dimmed: a dimmed count
falls under the text contrast floor, on a resting chip and on a pressed one. */}
{chip.count != null && (
<>
{" "}
<span aria-hidden="true">·</span> <span className="tabular-nums">{chip.count}</span>
</>
)}
</>
);
}
function TagChip({ chip, onToggled }: { chip: FilterChip; onToggled: (key: string) => void }) {
function TagChip({
chip,
onToggled,
}: {
chip: FilterChip;
onToggled: (key: string, from: HTMLElement) => void;
}) {
return (
<Chip
data-chip-key={chip.key}
onClick={(e) => {
// Before the toggle: a pressed chip may move (the caller's order can put picks first), and
// the focus must go where it went.
if (e.currentTarget === document.activeElement) onToggled(chip.key);
if (e.currentTarget === document.activeElement) onToggled(chip.key, e.currentTarget);
chip.onToggle();
}}
pressed={chip.active}
@@ -107,31 +120,43 @@ export default function TagFilterChips({
const pop = useAnchoredPopover({
open,
onOpenChange: setOpen,
mode: "menu",
// A fold of toggle buttons, not a menu of items: each chip is a pressed/unpressed button in the
// Tab order, which a role=menu with no menu items would hide from a screen reader.
mode: "dialog",
role: "dialog",
placement: "bottom-end",
});
const { setReference } = pop.refs;
const moreLabel = t("channels.tags.moreCount", { count: hidden });
// A toggle may move its chip — between the row and the "+N" fold (two parents: the button is
// remounted), or along the row (a keyed move that drops the focus to <body>). The focus follows
// it: onto the chip where it now stands, or onto "+N" when it went into a closed fold. Held for
// the frame, since the fit re-decides in layout effects after the toggle's render.
const follow = useRef<string | null>(null);
// A toggle may move its chip between the row and the "+N" fold (two parents: the button is
// remounted and the focus falls to <body>), or take it out of the list altogether (a stored pick of
// a tag that no longer exists). A move along the row keeps its node, and React gives it the focus
// back. The focus follows the chip: onto it where it now stands, onto "+N" when it went into a
// closed fold, or onto the chip now in its old place when it is gone. A toggle in the open fold
// closes it, so the focus never sits outside a popover left open. Held for the frame, since the fit
// re-decides in layout effects after the toggle's render.
const follow = useRef<{ key: string; at: number } | null>(null);
const foldRef = useRef<HTMLDivElement | null>(null);
const moreRef = useRef<HTMLButtonElement | null>(null);
const onToggled = (chipKey: string) => {
follow.current = chipKey;
const onToggled = (chipKey: string, from: HTMLElement) => {
follow.current = { key: chipKey, at: chips.findIndex((c) => c.key === chipKey) };
requestAnimationFrame(() => (follow.current = null));
if (foldRef.current?.contains(from)) setOpen(false);
};
useLayoutEffect(() => {
const chipKey = follow.current;
if (!chipKey) return;
const find = (box: HTMLElement | null) =>
[...(box?.querySelectorAll<HTMLElement>("[data-chip-key]") ?? [])].find(
(b) => b.dataset.chipKey === chipKey,
);
const el = find(rowRef.current) ?? find(foldRef.current) ?? moreRef.current;
const target = follow.current;
if (!target) return;
const inBox = (box: HTMLElement | null) => [
...(box?.querySelectorAll<HTMLElement>("[data-chip-key]") ?? []),
];
const row = inBox(rowRef.current);
const find = (box: HTMLElement[]) => box.find((b) => b.dataset.chipKey === target.key);
const gone = !chips.some((c) => c.key === target.key);
const el =
find(row) ??
find(inBox(foldRef.current)) ??
(gone ? (row[Math.min(target.at, row.length - 1)] ?? moreRef.current) : moreRef.current);
if (el && document.activeElement !== el) el.focus();
});
const setMore = (el: HTMLButtonElement | null) => {
@@ -50,8 +50,10 @@ export function useChannelActions({
const nameOf = (c: ManagedChannel) =>
c.title == null ? msg("channels.row.thisChannel") : raw(c.title);
// A hide also changes the tags' counts: a tag counts only the channels the Subscriptions tab shows.
const refresh = () => {
qc.invalidateQueries({ queryKey: qk.channels() });
qc.invalidateQueries({ queryKey: qk.tags() });
qc.invalidateQueries({ queryKey: qk.myStatus() });
qc.invalidateQueries({ queryKey: qk.feed() });
qc.invalidateQueries({ queryKey: qk.feedSummary() });
@@ -194,6 +194,8 @@ export default function ChannelPage({
qc.invalidateQueries({ queryKey: qk.channel(channelId) });
qc.invalidateQueries({ queryKey: qk.feed() });
qc.invalidateQueries({ queryKey: qk.channels() });
// A tag counts only the channels the Subscriptions tab shows, which a hide leaves out.
qc.invalidateQueries({ queryKey: qk.tags() });
},
});
@@ -152,6 +152,59 @@ describe("ShareDialog · opened to share", () => {
expect(h.copyText).toHaveBeenCalledTimes(1);
});
// An expired link opens nothing: it is no link to hand out, and reads as dead at a glance.
it("offers no Copy on an expired link, and marks it with a warning", async () => {
h.links = [row(1, "/file/old", lastWeek), row(2, "/file/b", nextWeek)];
open();
await waitFor(() => expect(h.copyText).toHaveBeenCalledWith(`${origin}/file/b`));
expect(screen.getAllByRole("button", { name: "downloads.share.copy" })).toHaveLength(1);
expect(screen.getByText("downloads.share.expiredOn", { exact: false }).className).toContain(
"text-warn",
);
expect(screen.getAllByRole("link", { name: "downloads.share.openNewTab" })).toHaveLength(2);
});
// Closed before the link it made came back, the dialog must not open next time on the list it
// had and make another.
it("makes one link, closed before it came back and opened again", async () => {
h.links = [];
let made: (l: unknown) => void = () => {};
h.createDownloadLink.mockReturnValue(new Promise((res) => (made = res)));
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
const dialog = () => (
<QueryClientProvider client={qc}>
<ShareDialog job={{ id: 9 } as DownloadJob} onClose={() => {}} />
</QueryClientProvider>
);
const first = render(dialog());
await waitFor(() => expect(h.createDownloadLink).toHaveBeenCalledTimes(1));
first.unmount();
made(row(4, "/file/new"));
await new Promise((r) => setTimeout(r, 20));
h.links = [row(4, "/file/new")];
render(dialog());
await screen.findByDisplayValue(`${origin}/file/new`);
await new Promise((r) => setTimeout(r, 20));
expect(h.createDownloadLink).toHaveBeenCalledTimes(1);
});
it("leaves a New link form the user opened while the first link was on its way", async () => {
h.links = [];
let made: (l: unknown) => void = () => {};
h.createDownloadLink.mockReturnValue(new Promise((res) => (made = res)));
open();
fireEvent.click(await screen.findByText("downloads.share.newLink"));
fireEvent.change(screen.getByPlaceholderText("downloads.share.password"), {
target: { value: "s3cret" },
});
h.links = [row(4, "/file/new")];
made(row(4, "/file/new"));
await screen.findByDisplayValue(`${origin}/file/new`);
expect(screen.getByPlaceholderText<HTMLInputElement>("downloads.share.password").value).toBe(
"s3cret",
);
});
// The clipboard holds what was copied last: the link copied on opening stops saying so.
it("stops claiming the opening copy once another link is copied", async () => {
h.links = [row(2, "/file/b"), row(3, "/file/c")];
+47 -19
View File
@@ -25,6 +25,7 @@ import { copyText } from "../../lib/clipboard";
import { Input, InnerInput } from "../../components/ui/Input";
import { CheckRow } from "../../components/ui/NativeControls";
import { SURFACE } from "../../components/ui/surfaces";
import { Badge } from "../../components/ui/Badge";
import UserPicker from "./UserPicker";
import { useCopyFeedback } from "../../components/ui/useCopyFeedback";
import { LoadingState, StateMessage } from "../../components/ui/QueryState";
@@ -126,14 +127,15 @@ function LinkRow({
},
});
// An expired link opens nothing, so it is no link to hand out: no Copy, and it reads as dead at a
// glance (a warn badge, its address struck through). Open stays, to check it; Revoke, to clear it.
const expired = linkExpired(link);
const badges: string[] = [];
if (link.has_password) badges.push(t("downloads.share.hasPassword"));
if (link.allow_download) badges.push(t("downloads.share.downloadable"));
if (link.expires_at)
if (link.expires_at && !expired)
badges.push(
t(linkExpired(link) ? "downloads.share.expiredOn" : "downloads.share.expiresOn", {
date: formatDate(link.expires_at, i18n.language),
}),
t("downloads.share.expiresOn", { date: formatDate(link.expires_at, i18n.language) }),
);
return (
@@ -142,14 +144,25 @@ function LinkRow({
<Link2 className="w-4 h-4 text-muted shrink-0" />
{fullUrl ? (
<>
<InnerInput className="text-xs text-muted truncate" readOnly value={fullUrl} />
<IconButton label={t("downloads.share.copy")} size="sm" variant="ghost" onClick={copy}>
{copied || copiedOnOpen ? (
<Check className="w-4 h-4 text-success" />
) : (
<Copy className="w-4 h-4" />
)}
</IconButton>
<InnerInput
className={`text-xs text-muted truncate ${expired ? "line-through" : ""}`}
readOnly
value={fullUrl}
/>
{!expired && (
<IconButton
label={t("downloads.share.copy")}
size="sm"
variant="ghost"
onClick={copy}
>
{copied || copiedOnOpen ? (
<Check className="w-4 h-4 text-success" />
) : (
<Copy className="w-4 h-4" />
)}
</IconButton>
)}
<IconLink
label={t("downloads.share.openNewTab")}
href={fullUrl}
@@ -214,6 +227,11 @@ function LinkRow({
<span className="inline-flex items-center gap-1">
<Eye className="w-3 h-3" /> {t("downloads.share.views", { count: link.view_count })}
</span>
{expired && link.expires_at && (
<Badge tone="warn">
{t("downloads.share.expiredOn", { date: formatDate(link.expires_at, i18n.language) })}
</Badge>
)}
{badges.map((b) => (
<span
key={b}
@@ -309,16 +327,26 @@ function LinkShare({ target }: { target: LinkTarget }) {
expires_days: expiryDays,
password: password.trim() || undefined,
}),
onSuccess: () => {
// In the list at once, as the one-click Share puts it: closed before the refetch, the dialog
// would open next time on the cached list without it and make another.
onSuccess: (made) => {
qc.setQueryData<ShareLink[]>(linksKey(target), (old) => [made, ...(old ?? [])]);
invalidate();
setCreating(false);
// Back to the form's own default for the next link: downloadable.
setAllowDownload(true);
setExpiryDays(null);
setPassword("");
},
meta: { failTitle: "downloads.share.linkFailed" },
});
// The form's own success, not the mutation's: the link made on open goes through the same
// mutation, and must not close and clear a form the user opened while it was on its way.
const createFromForm = () =>
create.mutate(undefined, {
onSuccess: () => {
setCreating(false);
// Back to the form's own default for the next link: downloadable.
setAllowDownload(true);
setExpiryDays(null);
setPassword("");
},
});
// Open with a link already generated: sharing is almost always the goal, so mint a default
// (no password, no expiry, downloadable) link the moment the dialog finds none that opens — as if the
@@ -419,7 +447,7 @@ function LinkShare({ target }: { target: LinkTarget }) {
<Button
variant="primary"
size="sm"
onClick={() => create.mutate()}
onClick={createFromForm}
disabled={create.isPending}
>
{t("downloads.share.createLink")}
@@ -72,7 +72,7 @@ const FOLDERS = [
];
const wrap = (
onClose = vi.fn(),
onClose: () => void = vi.fn(),
folderId: number | null = null,
initialFile: File | null = null,
) => {
@@ -186,34 +186,58 @@ describe("UploadDialog · where the file goes", () => {
expect(await sentTo()).toBe(7);
});
// The name field saves on blur, and a press on Cancel or ✕ blurs it before the dialog closes.
it.each([
["Cancel", () => screen.getByText("downloads.actions.cancel")],
["the close button", () => screen.getByRole("button", { name: "common.close" })],
])("dismissing with %s while naming makes no folder", async (_n, dismiss) => {
const onClose = vi.fn();
wrap(onClose);
// The name field saves on blur, and a press blurs it before the press acts: one that closes the
// dialog (Cancel, the ✕, the backdrop) makes no folder; one that leaves it open keeps the name.
const naming = async (onClose: () => void) => {
const r = wrap(onClose);
fireEvent.click(await target());
fireEvent.click(await screen.findByTestId("upload-target-new"));
const field = await screen.findByLabelText("files.folder.newName");
fireEvent.change(field, { target: { value: "Trips" } });
const control = dismiss();
fireEvent.pointerDown(control);
return { r, field };
};
const pressOn = (el: Element, field: HTMLElement, click = true) => {
fireEvent.pointerDown(el);
fireEvent.mouseDown(el);
fireEvent.blur(field);
fireEvent.click(control);
expect(onClose).toHaveBeenCalled();
await new Promise((r) => setTimeout(r, 20)); // a mutation calls its function a tick later
fireEvent.pointerUp(el);
fireEvent.mouseUp(el);
if (click) fireEvent.click(el);
};
const settled = () => new Promise((r) => setTimeout(r, 20)); // a mutation calls its function a tick later
it.each([
["Cancel", () => screen.getByText("downloads.actions.cancel")],
["the close button", () => screen.getByRole("button", { name: "common.close" })],
["the backdrop", () => screen.getByRole("dialog")],
])("dismissing with %s while naming makes no folder", async (_n, dismiss) => {
let unmount = () => {};
const { r, field } = await naming(() => unmount());
unmount = r.unmount;
pressOn(dismiss(), field);
await settled();
expect(screen.queryByRole("dialog")).toBeNull();
expect(h.createFolder).not.toHaveBeenCalled();
});
// A right-button press on the backdrop blurs the field but closes nothing: the name stands.
it("a press that closes nothing makes the folder it left", async () => {
h.createFolder.mockResolvedValue({ ...FOLDERS[1], id: 7, name: "Trips" });
const { field } = await naming(vi.fn());
pressOn(screen.getByRole("dialog"), field, false);
await waitFor(() => expect(h.createFolder).toHaveBeenCalledWith("Trips"));
});
it("leaving the name for anything else in the dialog makes the folder", async () => {
h.createFolder.mockResolvedValue({ ...FOLDERS[1], id: 7, name: "Trips" });
wrap();
fireEvent.click(await target());
fireEvent.click(await screen.findByTestId("upload-target-new"));
const field = await screen.findByLabelText("files.folder.newName");
fireEvent.change(field, { target: { value: "Trips" } });
fireEvent.pointerDown(screen.getByText("downloads.upload.targetLabel"));
const { field } = await naming(vi.fn());
pressOn(screen.getByText("downloads.upload.targetLabel"), field);
await waitFor(() => expect(h.createFolder).toHaveBeenCalledWith("Trips"));
});
it("leaving the name by Tab makes the folder at once", async () => {
h.createFolder.mockResolvedValue({ ...FOLDERS[1], id: 7, name: "Trips" });
const { field } = await naming(vi.fn());
fireEvent.blur(field);
await waitFor(() => expect(h.createFolder).toHaveBeenCalledWith("Trips"));
});
@@ -227,7 +251,10 @@ describe("UploadDialog · where the file goes", () => {
const field = await screen.findByLabelText("files.folder.newName");
fireEvent.change(field, { target: { value: "Trips" } });
fireEvent.keyDown(field, { key: "Enter" });
expect(await screen.findByText("downloads.upload.makingFolder")).toBeTruthy();
const making = await screen.findByText("downloads.upload.makingFolder");
// Said in a live region, and the keyboard is held on its place, not dropped to <body>.
expect(making.closest('[role="status"]')).toBeTruthy();
await waitFor(() => expect(document.activeElement).toBe(making.parentElement));
made({ ...FOLDERS[1], id: 7, name: "Trips" });
await waitFor(async () => expect(document.activeElement).toBe(await target()));
});
+43 -26
View File
@@ -82,19 +82,34 @@ export default function UploadDialog({
const [naming, setNaming] = useState(false);
const targetBox = useRef<HTMLDivElement>(null);
const refocusTarget = useRef(false);
// A press on Cancel, the ✕ or the backdrop blurs the name field before the dialog closes, and the
// field saves on blur: that press is "never mind", not "make it". The press is seen before the
// blur it causes, in every browser (which control takes the focus, if any, differs: Safari
// focuses no button).
const dismissing = useRef(false);
// The name field saves on blur, and a press blurs it before the press does anything: a press on
// Cancel, the ✕ or the backdrop that then closes the dialog is "never mind", not "make it". So a
// name left by a press is made once the press is over, and only if the dialog is still here — its
// close unmounts this and drops the wait. A name left without a press (Tab) is made at once.
const pressing = useRef(false);
const afterPress = useRef<(() => void) | null>(null);
useEffect(() => {
const onPress = (e: PointerEvent) => {
const el = e.target instanceof Element ? e.target : null;
dismissing.current =
!!el && (!!el.closest("[data-dismiss]") || el.matches('[role="dialog"]'));
let wait = 0;
const down = () => {
pressing.current = true;
};
const up = () => {
pressing.current = false;
const run = afterPress.current;
afterPress.current = null;
// After the click the press ends in, where a dismissal closes the dialog.
if (run) wait = window.setTimeout(run);
};
document.addEventListener("pointerdown", down, true);
document.addEventListener("pointerup", up, true);
document.addEventListener("pointercancel", up, true);
return () => {
document.removeEventListener("pointerdown", down, true);
document.removeEventListener("pointerup", up, true);
document.removeEventListener("pointercancel", up, true);
window.clearTimeout(wait);
afterPress.current = null;
};
document.addEventListener("pointerdown", onPress, true);
return () => document.removeEventListener("pointerdown", onPress, true);
}, []);
const makeFolder = useMutation({
mutationFn: (folder: string) => api.createFolder(folder),
@@ -106,12 +121,13 @@ export default function UploadDialog({
},
meta: { failTitle: "errors.writeFailed.folder.create" },
});
// A new folder's name ended by a key leaves the keyboard's focus nowhere: back on the list — once
// it is back, as the folder is made (or fails): until then its place says it is being made.
// A new folder's name ended by a key leaves the keyboard's focus nowhere: back on the list once it
// is back, as the folder is made (or fails). Until then on its place, which says it is being made.
useEffect(() => {
if (naming || !refocusTarget.current) return;
const list = targetBox.current?.querySelector("button");
if (!list) return;
const box = targetBox.current;
const list = box?.querySelector("button");
if (!list) return box?.focus();
refocusTarget.current = false;
list.focus();
}, [naming, makeFolder.isPending]);
@@ -131,7 +147,6 @@ export default function UploadDialog({
// After the menu has closed: it hands the focus back as it goes, which took it off a field already
// there, and a name field that loses the focus is done, with no name.
if (value === NEW_FOLDER) {
dismissing.current = false; // a press before the naming began (Cancel aborting an upload) is not this one
window.setTimeout(() => setNaming(true));
} else if (value === UPLOADS) setTarget(null);
else setTarget(Number(value));
@@ -268,19 +283,22 @@ export default function UploadDialog({
onDone={(folder, byKey) => {
refocusTarget.current = byKey;
setNaming(false);
if (!byKey && dismissing.current) return;
if (folder?.trim()) makeFolder.mutate(folder.trim());
const name = folder?.trim();
if (!name) return;
if (!byKey && pressing.current) afterPress.current = () => makeFolder.mutate(name);
else makeFolder.mutate(name);
}}
/>
) : (
// The select's own height held while the list loads, so it does not push the form down.
<div ref={targetBox} className="min-h-8 flex items-center">
{makeFolder.isPending && (
// Upload waits for it (`targetSettling`): this says why.
<span className="text-sm text-muted">
{t("downloads.upload.makingFolder", { name: makeFolder.variables })}
</span>
)}
// It holds the keyboard's focus while a folder is made (tabIndex -1: never a Tab stop), and
// says it in a live region, as the list it replaces is gone until then.
<div ref={targetBox} tabIndex={-1} className="min-h-8 flex items-center outline-none">
<span role="status" className="text-sm text-muted">
{/* Upload waits for it (`targetSettling`): this says why. */}
{makeFolder.isPending &&
t("downloads.upload.makingFolder", { name: makeFolder.variables })}
</span>
{foldersQ.data && !busy && !makeFolder.isPending && (
<FilterSelect
hideLabel
@@ -327,7 +345,6 @@ export default function UploadDialog({
variant="ghost"
size="sm"
onClick={() => (busy ? abortRef.current?.abort() : onClose())}
data-dismiss=""
>
{t("downloads.actions.cancel")}
</Button>
@@ -204,7 +204,11 @@ describe("PlexDiscover", () => {
onSeeAllContinue={onSeeAllContinue}
/>,
);
fireEvent.click(await screen.findByRole("button", { name: "common.seeAll" }));
fireEvent.click(
await screen.findByRole("button", {
name: "common.seeAllOf:plex.discover.continueWatching",
}),
);
expect(onSeeAllContinue).toHaveBeenCalledOnce();
expect(onSeeAllRecent).not.toHaveBeenCalled();
});
@@ -224,9 +228,10 @@ describe("PlexDiscover", () => {
onSeeAllContinue={noop}
/>,
);
// The last of them: Continue watching, above it, has its own.
const seeAll = await screen.findAllByRole("button", { name: "common.seeAll" });
fireEvent.click(seeAll[seeAll.length - 1]!);
// Each See all is named with its rail: Continue watching, above it, has its own.
fireEvent.click(
await screen.findByRole("button", { name: "common.seeAllOf:plex.discover.recentlyAdded" }),
);
expect(onSeeAllRecent).toHaveBeenCalledOnce();
});
+16
View File
@@ -294,6 +294,22 @@ describe("WatchPage honest failures", () => {
expect(page.getByRole("button", { name: button })).toBeTruthy();
});
it("a repeated Enter sends one try while it is on its way", async () => {
window.history.replaceState({}, "", "/watch/tok-enter");
const fetchMock = vi.fn().mockResolvedValueOnce(res(200, { needs_password: true }));
fetchMock.mockImplementation(() => new Promise(() => {}));
vi.stubGlobal("fetch", fetchMock);
const page = render(<WatchPage />);
const field = await page.findByPlaceholderText("Enter password");
fireEvent.change(field, { target: { value: "guess" } });
fireEvent.keyDown(field, { key: "Enter" });
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2));
fireEvent.keyDown(field, { key: "Enter" });
fireEvent.keyDown(field, { key: "Enter" });
await new Promise((r) => setTimeout(r, 20));
expect(fetchMock).toHaveBeenCalledTimes(2);
});
it("too many password tries says so, and keeps the form", async () => {
window.history.replaceState({}, "", "/watch/tok-429");
const page = await tryPassword(async () => res(429));
+3 -1
View File
@@ -259,7 +259,9 @@ export default function WatchPage() {
value={password}
autoFocus
onChange={(e) => setPassword(e.target.value)}
onKeyDown={(e) => e.key === "Enter" && password && unlock()}
// Not again while one is on its way, as the button: a held or repeated Enter would
// spend the link's tries (and its view count) once per press.
onKeyDown={(e) => e.key === "Enter" && password && !unlocking && unlock()}
placeholder={T.passwordPlaceholder}
aria-describedby={error ? errorId : undefined}
/>