Remove a background of one's own

- A cross on the tile's top-right corner removes an added background,
  critical-coloured at rest and named by its tooltip, as design draws it.
  The one in force offers none, since taking it away would leave nothing
  chosen; the view model refuses it too.
- The cross sits beside the item rather than in it, which would take the
  press first and choose the tile, and isn't a control of its own, so the
  menu keeps one keyboard model: Delete or Backspace on the tile does the
  same, and the tile says so with aria-keyshortcuts.
- Removed from IndexedDB and its picture released. End to end on
  Chromium and Firefox, a background removed by Delete is still gone
  after a reload.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
fkwp
2026-10-07 16:04:42 +02:00
co-authored by Claude Opus 5.5
parent 5292908098
commit 66ecfa7527
11 changed files with 352 additions and 21 deletions
+27
View File
@@ -326,6 +326,33 @@ test.describe("a background of one's own", () => {
await addImage(page, red);
await expect(mine).toBeVisible();
});
test("a removed image stays removed", async ({ page }) => {
test.setTimeout(120_000);
await page.goto("/");
await SpaHelpers.createCall(
page,
"Removing user",
"Own background removed",
);
await addImage(page, await redImage(page));
const mine = page
.getByRole("group", { name: "Background effects" })
.getByRole("menuitemradio", { name: "Background 3" });
await expect(mine).toHaveAttribute("aria-keyshortcuts", "Delete");
await mine.focus();
await page.keyboard.press("Delete");
await expect(mine).toHaveCount(0);
await page.reload();
await expect(page.getByTestId("lobby_joinCall")).toBeVisible();
await page.getByRole("button", { name: "Camera", exact: true }).click();
await expect(
page.getByRole("group", { name: "Background effects" }),
).toBeVisible();
await expect(mine).toHaveCount(0);
expect(await keptImages(page)).toEqual([]);
});
});
test.describe("what can be added", () => {
@@ -90,3 +90,42 @@ Please see LICENSE in the repository root for full details.
text-align: center;
overflow-wrap: anywhere;
}
.removable {
position: relative;
display: flex;
min-inline-size: 0;
}
.removable > .tile {
flex: 1;
}
/* Clear of the target for choosing the tile. */
.remove {
position: absolute;
inset-block-start: 0;
inset-inline-end: 0;
translate: 20% -20%;
display: none;
align-items: center;
justify-content: center;
inline-size: 28px;
block-size: 28px;
border-radius: 50%;
background: var(--cpd-color-bg-canvas-default);
color: var(--cpd-color-icon-critical-primary);
box-shadow: 0 1.2px 2.4px 0 rgb(0 0 0 / 15%);
cursor: pointer;
}
.remove:hover {
background: var(--cpd-color-bg-critical-subtle-hovered);
box-shadow: 0 0 0 var(--cpd-border-width-1)
var(--cpd-color-border-critical-subtle);
}
.removable:hover .remove,
.removable:focus-within .remove {
display: flex;
}
+64 -12
View File
@@ -5,13 +5,19 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE in the repository root for full details.
*/
import { type FC, type ReactNode, type ReactElement } from "react";
import {
type FC,
type KeyboardEvent,
type ReactNode,
type ReactElement,
} from "react";
import { VisuallyHidden } from "@radix-ui/react-visually-hidden";
import { InlineSpinner, MenuItem } from "@vector-im/compound-web";
import { InlineSpinner, MenuItem, Tooltip } from "@vector-im/compound-web";
import {
BlockIcon,
BlurIcon,
CheckCircleSolidIcon,
CloseIcon,
PlusIcon,
} from "@vector-im/compound-design-tokens/assets/web/icons";
@@ -25,6 +31,8 @@ export interface BackgroundEffectOption {
kind: "none" | "blur" | "image";
/** For kind "image". */
imageUrl?: string;
/** Whether it is the user's to remove. */
removable?: boolean;
}
interface Props {
@@ -42,6 +50,9 @@ interface Props {
/** Offers to add a background of one's own. Omit to leave that tile out. */
onAdd?: () => void;
addLabel?: string;
/** Removes a removable one, never the one in force. Omit to offer none. */
onRemove?: (id: string) => void;
removeLabel?: string;
}
/**
@@ -58,6 +69,8 @@ export const BackgroundEffectGrid: FC<Props> = ({
describedBy,
onAdd,
addLabel,
onRemove,
removeLabel,
}) => {
const choose = (id: string): void => {
if (id !== selected) onSelect?.(id);
@@ -65,7 +78,29 @@ export const BackgroundEffectGrid: FC<Props> = ({
const tiles = effects.map((effect) => {
const checked = effect.id === selected;
const disabled = onSelect === undefined && effect.kind !== "none";
return (
const remove =
effect.removable && !checked && onRemove !== undefined
? (): void => onRemove(effect.id)
: undefined;
const removeProps =
remove === undefined
? {}
: {
"aria-keyshortcuts": "Delete",
onKeyDown: (e: KeyboardEvent): void => {
if (e.key !== "Delete" && e.key !== "Backspace") return;
e.preventDefault();
remove();
},
};
const content = (
<TileContent
effect={effect}
checked={checked}
settling={checked && settling}
/>
);
const tile = (
<MenuItem
key={effect.id}
role="menuitemradio"
@@ -87,17 +122,37 @@ export const BackgroundEffectGrid: FC<Props> = ({
choose(effect.id);
}
}
{...removeProps}
>
<TileContent
effect={effect}
checked={checked}
settling={checked && settling}
/>
{content}
</MenuItem>
);
if (remove === undefined) return tile;
// Beside the item, not in it: inside, the item takes the press first and
// the tile is chosen instead.
return (
<div key={effect.id} className={styles.removable}>
{tile}
<Tooltip label={removeLabel ?? ""}>
{/* Not a control of its own, so the menu keeps one keyboard model:
the keyboard reaches the same action through Delete. */}
<span aria-hidden className={styles.remove} onClick={remove}>
<CloseIcon width={20} height={20} />
</span>
</Tooltip>
</div>
);
});
// A command rather than a choice, so an item beside the radios.
const addContent = (
<>
<span aria-hidden className={styles.swatch}>
<PlusIcon width={24} height={24} />
</span>
<VisuallyHidden>{addLabel}</VisuallyHidden>
</>
);
const addTile =
onAdd === undefined ? null : (
<MenuItem
@@ -109,10 +164,7 @@ export const BackgroundEffectGrid: FC<Props> = ({
onAdd();
}}
>
<span aria-hidden className={styles.swatch}>
<PlusIcon width={24} height={24} />
</span>
<VisuallyHidden>{addLabel}</VisuallyHidden>
{addContent}
</MenuItem>
);
+4 -2
View File
@@ -127,18 +127,20 @@ export const Default: Story = {
selectBackgroundEffect: fn(),
backgroundEffect: "blur",
backgroundEffects: [
{ id: "none", kind: "none" },
{ id: "blur", kind: "blur" },
{ id: "none", kind: "none", removable: false },
{ id: "blur", kind: "blur", removable: false },
...shippedBackgrounds.map((background) => ({
id: `image:${background.id}`,
kind: "image" as const,
imageUrl: background.imagePath,
removable: false,
})),
],
backgroundEffectNotice: undefined,
backgroundEffectSettling: false,
backgroundImageRefusal: undefined,
addBackgroundImage: fn(),
removeBackgroundEffect: fn(),
hangup: fn(),
buttonSize: "lg",
showFooter: true,
+6
View File
@@ -45,6 +45,8 @@ export interface BackgroundEffectChoice {
id: string;
kind: "none" | "blur" | "image";
imageUrl?: string;
/** One the user added, which they can take away again. */
removable: boolean;
}
export interface AudioOutputSwitcher {
@@ -75,6 +77,8 @@ export interface FooterActions {
selectBackgroundEffect: ((id: string) => void) | undefined;
/** Undefined where no background can be added. */
addBackgroundImage: ((file: File) => void) | undefined;
/** Removes an added background by its option id; never the one in force. */
removeBackgroundEffect: ((id: string) => void) | undefined;
toggleScreenSharing: (() => void) | undefined;
/** Also controls if the settings button is visible */
openSettings: (() => void) | undefined;
@@ -186,6 +190,7 @@ export const CallFooter: FC<FooterProps> = ({
const backgroundEffectNotice = useBehavior(vm.backgroundEffectNotice$);
const backgroundEffectSettling = useBehavior(vm.backgroundEffectSettling$);
const addBackgroundImage = useBehavior(vm.addBackgroundImage$);
const removeBackgroundEffect = useBehavior(vm.removeBackgroundEffect$);
const refusal = useBehavior(vm.backgroundImageRefusal$);
const refusalMessage = useMemo(() => {
switch (refusal?.reason) {
@@ -270,6 +275,7 @@ export const CallFooter: FC<FooterProps> = ({
onSelectBackgroundEffect={selectBackgroundEffect}
backgroundEffectSettling={backgroundEffectSettling}
onAddBackgroundImage={addBackgroundImage}
onRemoveBackgroundEffect={removeBackgroundEffect}
backgroundImageRefusal={refusalMessage}
backgroundEffectNotice={
backgroundEffectNotice === "unavailable"
+23 -4
View File
@@ -51,6 +51,7 @@ vi.mock("@livekit/track-processors", () => ({
const store = vi.hoisted(() => ({
add: vi.fn<(file: Blob) => Promise<string>>(),
remove: vi.fn<(id: string) => Promise<void>>(),
}));
vi.mock("../livekit/backgroundImages", async (original) => {
const { BehaviorSubject } = await import("rxjs");
@@ -59,6 +60,7 @@ vi.mock("../livekit/backgroundImages", async (original) => {
addedBackgrounds: {
added$: new BehaviorSubject<AddedBackground[] | undefined>([]),
add: store.add,
remove: store.remove,
},
};
});
@@ -324,21 +326,27 @@ describe("createCallFooterViewModel", () => {
expect(lobbyOn("desktop").backgroundEffectNotice$.value).toBeUndefined();
});
it("offers every effect in order", () => {
it("offers every effect in order, the added ones removable", () => {
sdkSupportMock.mockReturnValue(true);
const added$ = addedBackgrounds.added$ as BehaviorSubject<
AddedBackground[] | undefined
>;
added$.next([{ id: "mine", url: "blob:mine" }]);
expect(lobbyOn("desktop").backgroundEffects$.value).toEqual([
{ id: "none", kind: "none" },
{ id: "blur", kind: "blur" },
{ id: "none", kind: "none", removable: false },
{ id: "blur", kind: "blur", removable: false },
...shippedBackgrounds.map((background) => ({
id: `image:${background.id}`,
kind: "image",
imageUrl: background.imagePath,
removable: false,
})),
{ id: "added:mine", kind: "image", imageUrl: "blob:mine" },
{
id: "added:mine",
kind: "image",
imageUrl: "blob:mine",
removable: true,
},
]);
added$.next([]);
});
@@ -412,6 +420,17 @@ describe("createCallFooterViewModel", () => {
expect(vm.backgroundImageRefusal$.value).toBeUndefined();
});
it("never removes the background in force", () => {
sdkSupportMock.mockReturnValue(true);
store.remove.mockResolvedValue();
const vm = lobbyOn("desktop");
backgroundEffectSetting.setValue("added:worn");
vm.removeBackgroundEffect$.value!("added:worn");
expect(store.remove).not.toHaveBeenCalled();
vm.removeBackgroundEffect$.value!("added:spare");
expect(store.remove).toHaveBeenCalledWith("spare");
});
it("availability is the same before and during a call", () => {
for (const supported of [true, false]) {
sdkSupportMock.mockReturnValue(supported);
+24 -2
View File
@@ -109,6 +109,7 @@ function buildDeviceBehaviors(
| "backgroundEffectSettling$"
| "addBackgroundImage$"
| "backgroundImageRefusal$"
| "removeBackgroundEffect$"
> {
const options$ = (
available$: Behavior<Map<string, MenuOptions["label"]>>,
@@ -210,6 +211,25 @@ function buildDeviceBehaviors(
),
),
backgroundImageRefusal$: refusal$,
// Never the one in force: taking it away would leave nothing chosen.
removeBackgroundEffect$: scope.behavior(
offered$.pipe(
map((offered) =>
offered
? (optionId: string): void => {
const effect = parseEffect(optionId);
if (effect.kind !== "added") return;
if (backgroundEffectSetting.getValue() === optionId) return;
addedBackgrounds
.remove(effect.id)
.catch((e) =>
logger.warn("Could not remove that background", e),
);
}
: undefined,
),
),
),
};
}
@@ -381,17 +401,19 @@ function backgroundEffectChoices(
added: AddedBackground[],
): BackgroundEffectChoice[] {
return [
{ id: serializeEffect({ kind: "none" }), kind: "none" },
{ id: serializeEffect({ kind: "blur" }), kind: "blur" },
{ id: serializeEffect({ kind: "none" }), kind: "none", removable: false },
{ id: serializeEffect({ kind: "blur" }), kind: "blur", removable: false },
...shippedBackgrounds.map((background) => ({
id: serializeEffect({ kind: "shipped", id: background.id }),
kind: "image" as const,
imageUrl: background.imagePath,
removable: false,
})),
...added.map((background) => ({
id: serializeEffect({ kind: "added", id: background.id }),
kind: "image" as const,
imageUrl: background.url,
removable: true,
})),
];
}
@@ -374,6 +374,126 @@ export const BackgroundImageRefused: Story = {
},
};
/** A flat picture, for backgrounds of one's own. */
function swatch(colour: string): string {
return `data:image/svg+xml,${encodeURIComponent(
`<svg xmlns="http://www.w3.org/2000/svg" width="16" height="9"><rect width="16" height="9" fill="${colour}"/></svg>`,
)}`;
}
/** One's own backgrounds are removable, never the one in force. */
export const AddedBackgroundsCanBeRemoved: Story = {
args: {
...VideoUnmute.args,
backgroundEffects: [
...effects,
{
id: "added:one",
label: "Background 3",
kind: "image",
imageUrl: swatch("#c2410c"),
removable: true,
},
{
id: "added:two",
label: "Background 4",
kind: "image",
imageUrl: swatch("#1d4ed8"),
removable: true,
},
],
selectedBackgroundEffect: "added:one",
onRemoveBackgroundEffect: fn(),
},
play: async ({ args, canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(canvas.getByRole("button", { name: "Camera" }));
const body = within(document.body);
const inForce = await body.findByRole("menuitemradio", {
name: "Background 3",
});
await expect(inForce).not.toHaveAttribute("aria-keyshortcuts");
const other = body.getByRole("menuitemradio", { name: "Background 4" });
await expect(other).toHaveAttribute("aria-keyshortcuts", "Delete");
other.focus();
await userEvent.keyboard("{Delete}");
await expect(args.onRemoveBackgroundEffect).toHaveBeenCalledWith(
"added:two",
);
const wrapper = other.parentElement!;
await userEvent.hover(wrapper);
const cross = wrapper.querySelector<HTMLElement>(
"[aria-hidden]:last-child",
)!;
const probe = document.createElement("span");
probe.style.color = "var(--cpd-color-icon-critical-primary)";
cross.append(probe);
const critical = getComputedStyle(probe).color;
probe.remove();
await expect(getComputedStyle(cross).color).toBe(critical);
const tile = other.getBoundingClientRect();
const box = cross.getBoundingClientRect();
await expect(box.right).toBeGreaterThan(tile.right);
await expect(box.top).toBeLessThan(tile.top);
await userEvent.hover(cross);
await waitFor(async () =>
expect(
[...document.body.querySelectorAll("div")].some(
(d) => d.textContent === "Remove" && d.offsetParent !== null,
),
).toBe(true),
);
},
};
/** Removing by pointer while the selection really changes. */
export const RemovingWithLiveSelection: Story = {
args: AddedBackgroundsCanBeRemoved.args,
render: function WithLiveSelection(args): JSX.Element {
const [selected, setSelected] = useState("none");
const [offered, setOffered] = useState(args.backgroundEffects ?? []);
return (
<MediaMuteAndSwitchButton
{...args}
backgroundEffects={offered}
selectedBackgroundEffect={selected}
onSelectBackgroundEffect={setSelected}
onRemoveBackgroundEffect={(id): void =>
setOffered((current) => current.filter((o) => o.id !== id))
}
/>
);
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(canvas.getByRole("button", { name: "Camera" }));
const body = within(document.body);
const tile = await body.findByRole("menuitemradio", {
name: "Background 4",
});
await userEvent.hover(tile);
const cross = tile.parentElement!.querySelector<HTMLElement>(
"[aria-hidden]:last-child",
)!;
await expect(cross).toBeVisible();
await userEvent.click(cross);
await waitFor(async () =>
expect(
body.queryByRole("menuitemradio", { name: "Background 4" }),
).toBeNull(),
);
await expect(
body.getByRole("menuitemradio", { name: "None" }),
).toHaveAttribute("aria-checked", "true");
},
};
/** In a short call the effects scroll into view with the list. */
export const BackgroundEffectsScrollWhenTheyDoNotFit: Story = {
args: {
@@ -83,6 +83,8 @@ export interface MediaMuteAndSwitchButtonProps {
backgroundEffectSettling?: boolean;
/** Called with the file chosen from the add tile. Omit to leave it out. */
onAddBackgroundImage?: (file: File) => void;
/** Removes a removable effect, never the one in force. */
onRemoveBackgroundEffect?: (id: string) => void;
/** Why the last file offered couldn't be used; a new object each time. */
backgroundImageRefusal?: { text: string };
/**
@@ -120,6 +122,7 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
backgroundEffectNotice,
backgroundEffectSettling,
onAddBackgroundImage,
onRemoveBackgroundEffect,
backgroundImageRefusal,
onSelect,
}) => {
@@ -492,6 +495,8 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
}
}
addLabel={t("action.add_background_image")}
onRemove={onRemoveBackgroundEffect}
removeLabel={t("action.remove")}
/>
)}
{/* In the list, so the menu grows no taller for it. */}
+27 -1
View File
@@ -81,6 +81,23 @@ describe("AddedBackgrounds", () => {
expect(await storage.list()).toHaveLength(maxAddedBackgrounds);
});
it("forgets a removed image, and lets its picture go", async () => {
const revoke = vi
.spyOn(URL, "revokeObjectURL")
.mockImplementation(() => {});
const storage = memoryStorage();
const store = new AddedBackgrounds(storage, asGiven);
await flushPromises();
const first = await store.add(new Blob(["a"]));
const second = await store.add(new Blob(["b"]));
const url = store.added$.value![0].url;
await store.remove(first);
expect(store.added$.value?.map((a) => a.id)).toEqual([second]);
expect((await storage.list()).map((k) => k.id)).toEqual([second]);
expect(revoke).toHaveBeenCalledWith(url);
});
it("offers nothing where the browser keeps nothing", async () => {
const store = new AddedBackgrounds(null);
await flushPromises();
@@ -116,7 +133,7 @@ describe("prepareImage", () => {
});
describe("IndexedDBImageStorage", () => {
it("keeps images across sessions", async () => {
it("keeps images across sessions until they are deleted", async () => {
const indexedDB = new IDBFactory();
const image = new Blob(["x"], { type: "image/webp" });
const first = new IndexedDBImageStorage(indexedDB);
@@ -131,6 +148,8 @@ describe("IndexedDBImageStorage", () => {
["a", 1],
["b", 2],
]);
await second.delete("a");
expect((await first.list()).map(({ id }) => id)).toEqual(["b"]);
});
});
@@ -144,5 +163,12 @@ function memoryStorage(kept: KeptImage[] = []): BackgroundImageStorage {
kept.push(image);
return Promise.resolve();
},
delete: async (id): Promise<void> => {
kept.splice(
kept.findIndex((k) => k.id === id),
1,
);
return Promise.resolve();
},
};
}
+13
View File
@@ -42,6 +42,7 @@ export interface KeptImage {
export interface BackgroundImageStorage {
list(): Promise<KeptImage[]>;
put(image: KeptImage): Promise<void>;
delete(id: string): Promise<void>;
}
/** The backgrounds this device keeps, which never leave it. */
@@ -96,6 +97,14 @@ export class AddedBackgrounds {
]);
return kept.id;
}
public async remove(id: string): Promise<void> {
await this.storage?.delete(id);
const current = this.subject$.value ?? [];
const gone = current.find((a) => a.id === id);
if (gone) URL.revokeObjectURL(gone.url);
this.subject$.next(current.filter((a) => a.id !== id));
}
}
/**
@@ -190,6 +199,10 @@ export class IndexedDBImageStorage implements BackgroundImageStorage {
public async put(image: KeptImage): Promise<void> {
await this.run("readwrite", (s) => s.put(image));
}
public async delete(id: string): Promise<void> {
await this.run("readwrite", (s) => s.delete(id));
}
}
function openStorage(): BackgroundImageStorage | null {