diff --git a/src/components/CallFooter.tsx b/src/components/CallFooter.tsx index 86d67db73..632b26148 100644 --- a/src/components/CallFooter.tsx +++ b/src/components/CallFooter.tsx @@ -5,15 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { - Fragment, - type FC, - type JSX, - type Ref, - useCallback, - useMemo, - useRef, -} from "react"; +import { type FC, type JSX, type Ref, useCallback, useMemo } from "react"; import classNames from "classnames"; import { useTranslation } from "react-i18next"; import { logger } from "matrix-js-sdk/lib/logger"; @@ -180,29 +172,27 @@ export const CallFooter: FC = ({ const selectBackgroundEffect = useBehavior(vm.selectBackgroundEffect$); const { added, addBackground } = useAddedBackgrounds(); - const chooseFile = useRef(null); - const onAddBackgroundImage = useCallback((): void => { - chooseFile.current?.click(); - }, []); - const onFileChosen = useCallback( - (event: React.ChangeEvent): void => { - const file = event.target.files?.[0]; - // Cleared so choosing the same file twice in a row still counts. - event.target.value = ""; - if (!file) return; - addBackground(file).catch((e) => { - // 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 - // call, so for now this is only logged. - logger.warn( - e instanceof UnusableImage - ? `Cannot use that file as a background: ${e.reason}` - : "Could not keep that background", - e, - ); - }); + const onAddBackgroundImage = useCallback( + (file: File): void => { + // Chosen for the user straight away: they picked this picture to use it, + // and leaving it unselected would ask them to pick it twice. + addBackground(file) + .then((id) => + selectBackgroundEffect?.(serializeEffect({ kind: "added", id })), + ) + .catch((e) => { + // 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 + // call, so for now this is only logged. + logger.warn( + e instanceof UnusableImage + ? `Cannot use that file as a background: ${e.reason}` + : "Could not keep that background", + e, + ); + }); }, - [addBackground], + [addBackground, selectBackgroundEffect], ); // The catalogue is named here rather than in the view model: the names are @@ -282,36 +272,26 @@ export const CallFooter: FC = ({ if ((videoOptions?.length ?? 0) > 0) { buttons.push( - - {/* The picker the add tile opens. Hidden, and driven from the tile, - because a file input cannot be styled into one. */} - - - , + , ); } else { buttons.push( diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 610d440bc..35ef1b2ea 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -11,6 +11,7 @@ import { type CSSProperties, type FC, useEffect, + useRef, type ReactElement, } from "react"; import { @@ -104,8 +105,12 @@ export interface MediaMuteAndSwitchButtonProps { * wherever background processing is unavailable. */ onSelectBackgroundEffect?: (id: string) => void; - /** Called when the add tile is chosen. Omit to leave that tile out. */ - onAddBackgroundImage?: () => void; + /** + * Called with the file the user chose from the add tile. Omit to leave that + * tile out. The picker lives here rather than with the caller because + * opening it takes the focus, which would otherwise dismiss the menu. + */ + onAddBackgroundImage?: (file: File) => void; /** * 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` @@ -162,6 +167,21 @@ export const MediaMuteAndSwitchButton: FC = ({ // Which device we have asked for but not yet been given. Carries the kind as // well as the id, because an input and an output can share an id: "default" // names both on Chrome. + // Held open across the file picker: a native dialog takes the focus, and the + // menu would take that as a click elsewhere and close behind it. + const [choosingFile, setChoosingFile] = useState(false); + const chooseFile = useRef(null); + useEffect(() => { + const input = chooseFile.current; + if (!input) return; + // Dismissing the picker without choosing fires `cancel`, which React does + // not type, so it is listened for directly. Without it the menu would + // stay pinned open after a cancelled pick. + const done = (): void => setChoosingFile(false); + input.addEventListener("cancel", done); + return (): void => input.removeEventListener("cancel", done); + }, [onAddBackgroundImage]); + const [plannedSelection, setPlannedSelection] = useState<{ kind: "input" | "output"; id: string; @@ -528,7 +548,8 @@ export const MediaMuteAndSwitchButton: FC = ({ } onSelect={(e) => { e.preventDefault(); - onAddBackgroundImage(); + setChoosingFile(true); + chooseFile.current?.click(); }} key="add-background-image" />, @@ -545,6 +566,21 @@ export const MediaMuteAndSwitchButton: FC = ({ > {/* The mute button lives inside */} {button} + {onAddBackgroundImage !== undefined && ( + { + const file = e.target.files?.[0]; + // Cleared so the same file can be chosen twice in a row. + e.target.value = ""; + setChoosingFile(false); + if (file) onAddBackgroundImage(file); + }} + /> + )} = ({ // sit on top of the first one. Kept for the accessible name only. showTitle={false} open={menuOpen} - onOpenChange={setMenuOpen} + onOpenChange={(open) => { + // Ignore the close the file picker provokes by taking the focus. + if (!open && choosingFile) return; + setMenuOpen(open); + }} side="top" trigger={