mirror of
https://github.com/vector-im/element-call.git
synced 2026-09-22 22:29:30 +00:00
Say why a file cannot be used as a background
- The reason appears with the grid, where the user chose the file, using the same Alert the reaction menu already reports errors with - A reason for each: not an image, animated, could not be opened, and a fallback for anything the store itself refuses - Named one by one rather than built from the reason, so the extractor finds every string it has to translate - The background in force is untouched, which is the other half of FR-021: being refused a file changes nothing - Covered by a story, and checked end to end by offering a text file: the message appears, the menu stays open, nothing is kept Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -96,6 +96,10 @@
|
|||||||
},
|
},
|
||||||
"disconnected_banner": "Connectivity to the server has been lost.",
|
"disconnected_banner": "Connectivity to the server has been lost.",
|
||||||
"error": {
|
"error": {
|
||||||
|
"background_animated": "Animated images cannot be used as a background",
|
||||||
|
"background_not_an_image": "That file is not an image",
|
||||||
|
"background_not_kept": "That background could not be saved",
|
||||||
|
"background_undecodable": "That image could not be opened",
|
||||||
"call_is_not_supported": "Call is not supported",
|
"call_is_not_supported": "Call is not supported",
|
||||||
"call_not_found": "Call not found",
|
"call_not_found": "Call not found",
|
||||||
"call_not_found_description": "<0>That link doesn't appear to belong to any existing call. Check that you have the right link, or <2>create a new one</2>.</0>",
|
"call_not_found_description": "<0>That link doesn't appear to belong to any existing call. Check that you have the right link, or <2>create a new one</2>.</0>",
|
||||||
|
|||||||
@@ -5,7 +5,14 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
|
|||||||
Please see LICENSE in the repository root for full details.
|
Please see LICENSE in the repository root for full details.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { type FC, type JSX, type Ref, useCallback, useMemo } from "react";
|
import {
|
||||||
|
type FC,
|
||||||
|
type JSX,
|
||||||
|
type Ref,
|
||||||
|
useCallback,
|
||||||
|
useMemo,
|
||||||
|
useState,
|
||||||
|
} from "react";
|
||||||
import classNames from "classnames";
|
import classNames from "classnames";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { logger } from "matrix-js-sdk/lib/logger";
|
import { logger } from "matrix-js-sdk/lib/logger";
|
||||||
@@ -172,6 +179,27 @@ export const CallFooter: FC<FooterProps> = ({
|
|||||||
const backgroundEffect = useBehavior(vm.backgroundEffect$);
|
const backgroundEffect = useBehavior(vm.backgroundEffect$);
|
||||||
const selectBackgroundEffect = useBehavior(vm.selectBackgroundEffect$);
|
const selectBackgroundEffect = useBehavior(vm.selectBackgroundEffect$);
|
||||||
const { added, addBackground, removeBackground } = useAddedBackgrounds();
|
const { added, addBackground, removeBackground } = useAddedBackgrounds();
|
||||||
|
const [backgroundEffectError, setBackgroundEffectError] = useState<
|
||||||
|
string | undefined
|
||||||
|
>(undefined);
|
||||||
|
|
||||||
|
// Spelled out rather than built from the reason, so the extractor can find
|
||||||
|
// every string it has to translate.
|
||||||
|
const whyRefused = useCallback(
|
||||||
|
(e: unknown): string => {
|
||||||
|
if (e instanceof UnusableImage)
|
||||||
|
switch (e.reason) {
|
||||||
|
case "not-an-image":
|
||||||
|
return t("error.background_not_an_image");
|
||||||
|
case "animated":
|
||||||
|
return t("error.background_animated");
|
||||||
|
case "undecodable":
|
||||||
|
return t("error.background_undecodable");
|
||||||
|
}
|
||||||
|
return t("error.background_not_kept");
|
||||||
|
},
|
||||||
|
[t],
|
||||||
|
);
|
||||||
|
|
||||||
const onRemoveBackgroundEffect = useCallback(
|
const onRemoveBackgroundEffect = useCallback(
|
||||||
(id: string): void => {
|
(id: string): void => {
|
||||||
@@ -188,6 +216,7 @@ export const CallFooter: FC<FooterProps> = ({
|
|||||||
(file: File): void => {
|
(file: File): void => {
|
||||||
// Chosen for the user straight away: they picked this picture to use it,
|
// Chosen for the user straight away: they picked this picture to use it,
|
||||||
// and leaving it unselected would ask them to pick it twice.
|
// and leaving it unselected would ask them to pick it twice.
|
||||||
|
setBackgroundEffectError(undefined);
|
||||||
addBackground(file)
|
addBackground(file)
|
||||||
.then((id) =>
|
.then((id) =>
|
||||||
selectBackgroundEffect?.(serializeEffect({ kind: "added", id })),
|
selectBackgroundEffect?.(serializeEffect({ kind: "added", id })),
|
||||||
@@ -196,6 +225,7 @@ export const CallFooter: FC<FooterProps> = ({
|
|||||||
// TODO: FR-021 wants the user told what went wrong. There is no
|
// TODO: FR-021 wants the user told what went wrong. There is no
|
||||||
// surface for that in the menu yet, and inventing one is design's
|
// surface for that in the menu yet, and inventing one is design's
|
||||||
// call, so for now this is only logged.
|
// call, so for now this is only logged.
|
||||||
|
setBackgroundEffectError(whyRefused(e));
|
||||||
logger.warn(
|
logger.warn(
|
||||||
e instanceof UnusableImage
|
e instanceof UnusableImage
|
||||||
? `Cannot use that file as a background: ${e.reason}`
|
? `Cannot use that file as a background: ${e.reason}`
|
||||||
@@ -204,7 +234,7 @@ export const CallFooter: FC<FooterProps> = ({
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
[addBackground, selectBackgroundEffect],
|
[addBackground, selectBackgroundEffect, whyRefused],
|
||||||
);
|
);
|
||||||
|
|
||||||
// The catalogue is named here rather than in the view model: the names are
|
// The catalogue is named here rather than in the view model: the names are
|
||||||
@@ -305,6 +335,7 @@ export const CallFooter: FC<FooterProps> = ({
|
|||||||
: undefined
|
: undefined
|
||||||
}
|
}
|
||||||
onRemoveBackgroundEffect={onRemoveBackgroundEffect}
|
onRemoveBackgroundEffect={onRemoveBackgroundEffect}
|
||||||
|
backgroundEffectError={backgroundEffectError}
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -369,3 +369,8 @@ Please see LICENSE in the repository root for full details.
|
|||||||
flex: 1;
|
flex: 1;
|
||||||
min-inline-size: 0;
|
min-inline-size: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Sits under the tiles, inside the same margin they keep from the frame. */
|
||||||
|
.effectError {
|
||||||
|
padding: 0 var(--cpd-space-4x) var(--cpd-space-4x);
|
||||||
|
}
|
||||||
|
|||||||
@@ -1014,3 +1014,30 @@ export const RemovingWithLiveSelection: Story = {
|
|||||||
await expect(none).toHaveAttribute("aria-checked", "true");
|
await expect(none).toHaveAttribute("aria-checked", "true");
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/** A file that could not be used, said where the user chose it. */
|
||||||
|
export const BackgroundImageRefused: Story = {
|
||||||
|
args: {
|
||||||
|
...BackgroundEffects.args,
|
||||||
|
backgroundEffectError: "Animated images cannot be used as a background",
|
||||||
|
},
|
||||||
|
play: async ({ canvasElement }) => {
|
||||||
|
const canvas = within(canvasElement);
|
||||||
|
await userEvent.click(canvas.getByRole("button", { name: "Camera" }));
|
||||||
|
const body = within(document.body);
|
||||||
|
|
||||||
|
// Scoped to the open menu: a closed one leaves its own markup behind, and
|
||||||
|
// the first match in the document belongs to that rather than to this.
|
||||||
|
const menu = await body.findByRole("menu");
|
||||||
|
await waitFor(async () =>
|
||||||
|
expect(
|
||||||
|
menu.querySelector<HTMLElement>(`.${styles.effectError}`),
|
||||||
|
).toHaveTextContent("Animated images cannot be used as a background"),
|
||||||
|
);
|
||||||
|
// The grid is still there to choose from: being refused a file changes
|
||||||
|
// nothing about the background in force.
|
||||||
|
await expect(
|
||||||
|
await body.findByRole("menuitemradio", { name: "None" }),
|
||||||
|
).toHaveAttribute("aria-checked", "true");
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import {
|
|||||||
type ReactElement,
|
type ReactElement,
|
||||||
} from "react";
|
} from "react";
|
||||||
import {
|
import {
|
||||||
|
Alert,
|
||||||
Button,
|
Button,
|
||||||
Menu,
|
Menu,
|
||||||
MenuItem,
|
MenuItem,
|
||||||
@@ -120,6 +121,11 @@ export interface MediaMuteAndSwitchButtonProps {
|
|||||||
* wearing, and taking it away would leave them with nothing chosen.
|
* wearing, and taking it away would leave them with nothing chosen.
|
||||||
*/
|
*/
|
||||||
onRemoveBackgroundEffect?: (id: string) => void;
|
onRemoveBackgroundEffect?: (id: string) => void;
|
||||||
|
/**
|
||||||
|
* Why the last file the user offered could not be used, if it could not.
|
||||||
|
* Shown with the grid, where they chose it.
|
||||||
|
*/
|
||||||
|
backgroundEffectError?: string;
|
||||||
/**
|
/**
|
||||||
* For any toggle and option this method will be called.
|
* For any toggle and option this method will be called.
|
||||||
* So toggles need to be implemented by listening here and setting the right toggle item to `enabled`
|
* So toggles need to be implemented by listening here and setting the right toggle item to `enabled`
|
||||||
@@ -172,6 +178,7 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
|
|||||||
onSelectBackgroundEffect,
|
onSelectBackgroundEffect,
|
||||||
onAddBackgroundImage,
|
onAddBackgroundImage,
|
||||||
onRemoveBackgroundEffect,
|
onRemoveBackgroundEffect,
|
||||||
|
backgroundEffectError,
|
||||||
onSelect,
|
onSelect,
|
||||||
}) => {
|
}) => {
|
||||||
// Which device we have asked for but not yet been given. Carries the kind as
|
// Which device we have asked for but not yet been given. Carries the kind as
|
||||||
@@ -760,6 +767,13 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
|
|||||||
<div role="none" className={styles.effectGrid}>
|
<div role="none" className={styles.effectGrid}>
|
||||||
{effectTiles()}
|
{effectTiles()}
|
||||||
</div>
|
</div>
|
||||||
|
{backgroundEffectError !== undefined && (
|
||||||
|
// Beside the grid rather than over the call: the user is
|
||||||
|
// looking here, having just chosen the file this is about.
|
||||||
|
<div role="none" className={styles.effectError}>
|
||||||
|
<Alert type="critical" title={backgroundEffectError} />
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user