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.
This commit is contained in:
Robin 2023-09-18 20:47:47 -04:00
parent 535712d108
commit 4253963b95
8 changed files with 87 additions and 70 deletions

View File

@ -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"),

View File

@ -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;
};

View File

@ -50,12 +50,11 @@ export const AppSelectionModal: FC<Props> = ({ 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;
});

View File

@ -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<Props> = ({
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({
</div>
</div>
</main>
<Body className={styles.footer}>
<Link color="primary" to="/">
{t("Return to home screen")}
</Link>
</Body>
{!confineToRoom && (
<Body className={styles.footer}>
<Link color="primary" to="/">
{t("Return to home screen")}
</Link>
</Body>
)}
</>
);
} else {
@ -183,15 +189,18 @@ export function CallEndedView({
"\n" +
t("How did it go?")}
</Headline>
{!surveySubmitted && PosthogAnalytics.instance.isEnabled()
{(!surveySubmitted || confineToRoom) &&
PosthogAnalytics.instance.isEnabled()
? qualitySurveyDialog
: createAccountDialog}
</main>
<Body className={styles.footer}>
<Link color="primary" to="/">
{t("Not now, return to home screen")}
</Link>
</Body>
{!confineToRoom && (
<Body className={styles.footer}>
<Link color="primary" to="/">
{t("Not now, return to home screen")}
</Link>
</Body>
)}
</>
);
}
@ -200,12 +209,10 @@ export function CallEndedView({
return (
<>
<Header>
<LeftNav>
<HeaderLogo />
</LeftNav>
<LeftNav>{!confineToRoom && <HeaderLogo />}</LeftNav>
<RightNav />
</Header>
<div className={styles.container}>{renderBody()}</div>
</>
);
}
};

View File

@ -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}

View File

@ -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<Props> = ({
matrixInfo,
muteStates,
onEnter,
isEmbedded,
confineToRoom,
hideHeader,
participatingMembers,
onShareClick,
@ -85,7 +85,7 @@ export const LobbyView: FC<Props> = ({
const onLeaveClick = useCallback(() => history.push("/"), [history]);
const recentsButtonInFooter = useMediaQuery("(max-height: 500px)");
const recentsButton = !isEmbedded && (
const recentsButton = !confineToRoom && (
<Link className={styles.recents} href="#" onClick={onLeaveClick}>
{t("Back to recents")}
</Link>
@ -140,7 +140,7 @@ export const LobbyView: FC<Props> = ({
disabled={muteStates.audio.setEnabled === null}
/>
<SettingsButton onPress={openSettings} />
{!isEmbedded && <HangupButton onPress={onLeaveClick} />}
{!confineToRoom && <HangupButton onPress={onLeaveClick} />}
</div>
</div>
</div>

View File

@ -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") && (
<AppSelectionModal roomId={roomId} />
)}
</>

View File

@ -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);