From 4253963b9566d680368e2550767e46f2e6bfd3ec Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 18 Sep 2023 20:47:47 -0400 Subject: [PATCH] Untangle the semantics of isEmbedded This deletes the isEmbedded flag from UrlParams, replacing it with an alternative set of flags that I think is more sensible and well-defined. --- src/UrlParams.ts | 27 ++++++++++++--- src/e2ee/sharedKeyManagement.ts | 23 +++++-------- src/room/AppSelectionModal.tsx | 9 +++-- src/room/CallEndedView.tsx | 59 ++++++++++++++++++--------------- src/room/GroupCallView.tsx | 13 ++++---- src/room/LobbyView.tsx | 8 ++--- src/room/RoomPage.tsx | 12 +++---- src/settings/SettingsModal.tsx | 6 ++-- 8 files changed, 87 insertions(+), 70 deletions(-) diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 91a292a3..a7d48111 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -27,6 +27,11 @@ interface RoomIdentifier { viaServers: string[]; } +// If you need to add a new flag to this interface, prefer a name that describes +// a specific behavior (such as 'confineToRoom'), rather than one that describes +// the situations that call for this behavior ('isEmbedded'). This makes it +// clearer what each flag means, and helps us avoid coupling Element Call's +// behavior to the needs of specific consumers. interface UrlParams { /** * Anything about what room we're pointed to should be from useRoomIdentifier which @@ -37,10 +42,14 @@ interface UrlParams { */ roomId: string | null; /** - * Whether the app is running in embedded mode, and should keep the user - * confined to the current room. + * Whether the app should keep the user confined to the current call/room. */ - isEmbedded: boolean; + confineToRoom: boolean; + /** + * Whether upon entering a room, the user should be prompted to launch the + * native mobile app. (Affects only Android and iOS.) + */ + appPrompt: boolean; /** * Whether the app should pause before joining the call until it sees an * io.element.join widget action, allowing it to be preloaded. @@ -101,6 +110,10 @@ interface UrlParams { password: string | null; } +// This is here as a stopgap, but what would be far nicer is a function that +// takes a UrlParams and returns a query string. That would enable us to +// consolidate all the data about URL parameters and their meanings to this one +// file. export function editFragmentQuery( hash: string, edit: (params: URLSearchParams) => URLSearchParams @@ -169,11 +182,15 @@ export const getUrlParams = ( // the room ID is, then that's what it is. roomId: parser.getParam("roomId"), password: parser.getParam("password"), - isEmbedded: parser.hasParam("embed"), + // This flag has 'embed' as an alias for historical reasons + confineToRoom: parser.hasParam("confineToRoom") || parser.hasParam("embed"), + // Defaults to true + appPrompt: parser.getParam("appPrompt") !== "false", preload: parser.hasParam("preload"), hideHeader: parser.hasParam("hideHeader"), hideScreensharing: parser.hasParam("hideScreensharing"), - e2eEnabled: parser.getParam("enableE2e") !== "false", // Defaults to true + // Defaults to true + e2eEnabled: parser.getParam("enableE2e") !== "false", userId: parser.getParam("userId"), displayName: parser.getParam("displayName"), deviceId: parser.getParam("deviceId"), diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 83eca2ea..2af0b463 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -20,6 +20,7 @@ import { useEnableE2EE } from "../settings/useSetting"; import { useLocalStorage } from "../useLocalStorage"; import { useClient } from "../ClientContext"; import { PASSWORD_STRING, useUrlParams } from "../UrlParams"; +import { widget } from "../widget"; export const getRoomSharedKeyLocalStorageKey = (roomId: string): string => `room-shared-key-${roomId}`; @@ -67,19 +68,13 @@ export const useManageRoomSharedKey = (roomId: string): string | null => { }; export const useIsRoomE2EE = (roomId: string): boolean | null => { - const { isEmbedded } = useUrlParams(); - const client = useClient(); - const room = useMemo( - () => client.client?.getRoom(roomId) ?? null, - [roomId, client.client] + const { client } = useClient(); + const room = useMemo(() => client?.getRoom(roomId) ?? null, [roomId, client]); + // For now, rooms in widget mode are never considered encrypted. + // In the future, when widget mode gains encryption support, then perhaps we + // should inspect the e2eEnabled URL parameter here? + return useMemo( + () => widget === null && (room === null || !room.getCanonicalAlias()), + [room] ); - const isE2EE = useMemo(() => { - if (isEmbedded) { - return false; - } else { - return room ? !room?.getCanonicalAlias() : null; - } - }, [isEmbedded, room]); - - return isE2EE; }; diff --git a/src/room/AppSelectionModal.tsx b/src/room/AppSelectionModal.tsx index 3480e9d8..34ab7c3b 100644 --- a/src/room/AppSelectionModal.tsx +++ b/src/room/AppSelectionModal.tsx @@ -50,12 +50,11 @@ export const AppSelectionModal: FC = ({ roomId }) => { ? window.location.href : getRoomUrl(roomId, roomSharedKey ?? undefined) ); - // Edit the URL so that it opens in embedded mode. We do this for two - // reasons: It causes the mobile app to limit the user to only visiting the - // room in question, and it prevents this app selection prompt from being - // shown a second time. + // Edit the URL to prevent the app selection prompt from appearing a second + // time within the app, and to keep the user confined to the current room url.hash = editFragmentQuery(url.hash, (params) => { - params.set("embed", ""); + params.set("appPrompt", "false"); + params.set("confineToRoom", ""); return params; }); diff --git a/src/room/CallEndedView.tsx b/src/room/CallEndedView.tsx index 69d2de79..6050b9e9 100644 --- a/src/room/CallEndedView.tsx +++ b/src/room/CallEndedView.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { FormEventHandler, useCallback, useState } from "react"; +import { FC, FormEventHandler, useCallback, useState } from "react"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { Trans, useTranslation } from "react-i18next"; import { useHistory } from "react-router-dom"; @@ -30,19 +30,23 @@ import { FieldRow, InputField } from "../input/Input"; import { StarRatingInput } from "../input/StarRatingInput"; import { RageshakeButton } from "../settings/RageshakeButton"; -export function CallEndedView({ - client, - isPasswordlessUser, - endedCallId, - leaveError, - reconnect, -}: { +interface Props { client: MatrixClient; isPasswordlessUser: boolean; + confineToRoom: boolean; endedCallId: string; leaveError?: Error; reconnect: () => void; -}) { +} + +export const CallEndedView: FC = ({ + client, + isPasswordlessUser, + confineToRoom, + endedCallId, + leaveError, + reconnect, +}) => { const { t } = useTranslation(); const history = useHistory(); @@ -72,14 +76,14 @@ export function CallEndedView({ if (isPasswordlessUser) { // setting this renders the callEndedView with the invitation to create an account setSurverySubmitted(true); - } else { + } else if (!confineToRoom) { // if the user already has an account immediately go back to the home screen history.push("/"); } }, 1000); }, 1000); }, - [endedCallId, history, isPasswordlessUser, starRating] + [endedCallId, history, isPasswordlessUser, confineToRoom, starRating] ); const createAccountDialog = isPasswordlessUser && ( @@ -161,11 +165,13 @@ export function CallEndedView({ - - - {t("Return to home screen")} - - + {!confineToRoom && ( + + + {t("Return to home screen")} + + + )} ); } else { @@ -183,15 +189,18 @@ export function CallEndedView({ "\n" + t("How did it go?")} - {!surveySubmitted && PosthogAnalytics.instance.isEnabled() + {(!surveySubmitted || confineToRoom) && + PosthogAnalytics.instance.isEnabled() ? qualitySurveyDialog : createAccountDialog} - - - {t("Not now, return to home screen")} - - + {!confineToRoom && ( + + + {t("Not now, return to home screen")} + + + )} ); } @@ -200,12 +209,10 @@ export function CallEndedView({ return ( <>
- - - + {!confineToRoom && }
{renderBody()}
); -} +}; diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 6a8c9333..21a29b92 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -56,7 +56,7 @@ declare global { interface Props { client: MatrixClient; isPasswordlessUser: boolean; - isEmbedded: boolean; + confineToRoom: boolean; preload: boolean; hideHeader: boolean; rtcSession: MatrixRTCSession; @@ -65,7 +65,7 @@ interface Props { export function GroupCallView({ client, isPasswordlessUser, - isEmbedded, + confineToRoom, preload, hideHeader, rtcSession, @@ -233,13 +233,13 @@ export function GroupCallView({ if ( !isPasswordlessUser && - !isEmbedded && + !confineToRoom && !PosthogAnalytics.instance.isEnabled() ) { history.push("/"); } }, - [rtcSession, isPasswordlessUser, isEmbedded, history] + [rtcSession, isPasswordlessUser, confineToRoom, history] ); useEffect(() => { @@ -334,7 +334,7 @@ export function GroupCallView({ // submitting anything. if ( isPasswordlessUser || - (PosthogAnalytics.instance.isEnabled() && !isEmbedded) || + (PosthogAnalytics.instance.isEnabled() && widget === null) || leaveError ) { return ( @@ -342,6 +342,7 @@ export function GroupCallView({ endedCallId={rtcSession.room.roomId} client={client} isPasswordlessUser={isPasswordlessUser} + confineToRoom={confineToRoom} leaveError={leaveError} reconnect={onReconnect} /> @@ -363,7 +364,7 @@ export function GroupCallView({ matrixInfo={matrixInfo} muteStates={muteStates} onEnter={() => enterRTCSession(rtcSession)} - isEmbedded={isEmbedded} + confineToRoom={confineToRoom} hideHeader={hideHeader} participatingMembers={participatingMembers} onShareClick={onShareClick} diff --git a/src/room/LobbyView.tsx b/src/room/LobbyView.tsx index 6b2ddf70..f9601f08 100644 --- a/src/room/LobbyView.tsx +++ b/src/room/LobbyView.tsx @@ -42,7 +42,7 @@ interface Props { matrixInfo: MatrixInfo; muteStates: MuteStates; onEnter: () => void; - isEmbedded: boolean; + confineToRoom: boolean; hideHeader: boolean; participatingMembers: RoomMember[]; onShareClick: (() => void) | null; @@ -53,7 +53,7 @@ export const LobbyView: FC = ({ matrixInfo, muteStates, onEnter, - isEmbedded, + confineToRoom, hideHeader, participatingMembers, onShareClick, @@ -85,7 +85,7 @@ export const LobbyView: FC = ({ const onLeaveClick = useCallback(() => history.push("/"), [history]); const recentsButtonInFooter = useMediaQuery("(max-height: 500px)"); - const recentsButton = !isEmbedded && ( + const recentsButton = !confineToRoom && ( {t("Back to recents")} @@ -140,7 +140,7 @@ export const LobbyView: FC = ({ disabled={muteStates.audio.setEnabled === null} /> - {!isEmbedded && } + {!confineToRoom && } diff --git a/src/room/RoomPage.tsx b/src/room/RoomPage.tsx index 76302ca9..1a53dc49 100644 --- a/src/room/RoomPage.tsx +++ b/src/room/RoomPage.tsx @@ -30,7 +30,8 @@ import { platform } from "../Platform"; import { AppSelectionModal } from "./AppSelectionModal"; export const RoomPage: FC = () => { - const { isEmbedded, preload, hideHeader, displayName } = useUrlParams(); + const { confineToRoom, appPrompt, preload, hideHeader, displayName } = + useUrlParams(); const { roomAlias, roomId, viaServers } = useRoomIdentifier(); @@ -74,12 +75,12 @@ export const RoomPage: FC = () => { client={client!} rtcSession={rtcSession} isPasswordlessUser={passwordlessUser} - isEmbedded={isEmbedded} + confineToRoom={confineToRoom} preload={preload} hideHeader={hideHeader} /> ), - [client, passwordlessUser, isEmbedded, preload, hideHeader] + [client, passwordlessUser, confineToRoom, preload, hideHeader] ); let content: ReactNode; @@ -107,9 +108,8 @@ export const RoomPage: FC = () => { return ( <> {content} - {/* On mobile, show a prompt to launch the mobile app. If in embedded mode, - that means we *are* in the mobile app and should show no further prompt. */} - {(platform === "android" || platform === "ios") && !isEmbedded && ( + {/* On Android and iOS, show a prompt to launch the mobile app. */} + {appPrompt && (platform === "android" || platform === "ios") && ( )} diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 39c3789a..a0e14ee9 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -43,12 +43,12 @@ import { Body, Caption } from "../typography/Typography"; import { AnalyticsNotice } from "../analytics/AnalyticsNotice"; import { ProfileSettingsTab } from "./ProfileSettingsTab"; import { FeedbackSettingsTab } from "./FeedbackSettingsTab"; -import { useUrlParams } from "../UrlParams"; import { useMediaDevices, MediaDevice, useMediaDeviceNames, } from "../livekit/MediaDevicesContext"; +import { widget } from "../widget"; interface Props { open: boolean; @@ -61,8 +61,6 @@ interface Props { export const SettingsModal = (props: Props) => { const { t } = useTranslation(); - const { isEmbedded } = useUrlParams(); - const [showInspector, setShowInspector] = useShowInspector(); const [optInAnalytics, setOptInAnalytics] = useOptInAnalytics(); const [developerSettingsTab, setDeveloperSettingsTab] = @@ -282,7 +280,7 @@ export const SettingsModal = (props: Props) => { ); const tabs = [audioTab, videoTab]; - if (!isEmbedded) tabs.push(profileTab); + if (widget === null) tabs.push(profileTab); tabs.push(feedbackTab, moreTab); if (developerSettingsTab) tabs.push(developerTab);