New Header edgecase fixes: Close lobby button not shown, disable join button in various places, more... (#12235)

* Add missing tooltip Signed-off-by: Timo K <toger5@hotmail.de> * fix incoming call toast (icon + disabled button if there is an ongoing call) Signed-off-by: Timo K <toger5@hotmail.de> * room header - fix join button not getting disabled if there is an ongoing call - fix close lobby button not shown (instead we see the join button) Signed-off-by: Timo K <toger5@hotmail.de> * additional tests Signed-off-by: Timo K <toger5@hotmail.de> * fix tests Signed-off-by: Timo K <toger5@hotmail.de> * update snapshot Signed-off-by: Timo K <toger5@hotmail.de> * fix not open menu if disabled Signed-off-by: Timo K <toger5@hotmail.de> * add tooltip provider Signed-off-by: Timo K <toger5@hotmail.de> * update snap class Signed-off-by: Timo K <toger5@hotmail.de> * room header snap update Signed-off-by: Timo K <toger5@hotmail.de> * fix snapshot Signed-off-by: Timo K <toger5@hotmail.de> --------- Signed-off-by: Timo K <toger5@hotmail.de>
2024-11-15 20:54:59 +08:00 · 2024-02-08 17:48:50 +01:00 · 2024-02-08 17:48:50 +01:00 · 552c65db98
commit 552c65db98
parent 1c789cbb18
5 changed files with 132 additions and 53 deletions
--- a/src/components/views/rooms/RoomHeader.tsx
+++ b/src/components/views/rooms/RoomHeader.tsx
@ -125,30 +125,45 @@ export default function RoomHeader({
            </IconButton>
        </Tooltip>
    );
+
    const joinCallButton = (
-        <Button
-            size="sm"
-            onClick={videoClick}
-            Icon={VideoCallIcon}
-            className="mx_RoomHeader_join_button"
-            color="primary"
-        >
-            {_t("action|join")}
-        </Button>
+        <Tooltip label={videoCallDisabledReason ?? _t("voip|video_call")}>
+            <Button
+                size="sm"
+                onClick={videoClick}
+                Icon={VideoCallIcon}
+                className="mx_RoomHeader_join_button"
+                disabled={!!videoCallDisabledReason}
+                color="primary"
+                aria-label={videoCallDisabledReason ?? _t("action|join")}
+            >
+                {_t("action|join")}
+            </Button>
+        </Tooltip>
    );
-    const [menuOpen, setMenuOpen] = useState(false);
+
    const callIconWithTooltip = (
        <Tooltip label={videoCallDisabledReason ?? _t("voip|video_call")}>
            <VideoCallIcon />
        </Tooltip>
    );
+
+    const [menuOpen, setMenuOpen] = useState(false);
+
+    const onOpenChange = useCallback(
+        (newOpen: boolean) => {
+            if (!videoCallDisabledReason) setMenuOpen(newOpen);
+        },
+        [videoCallDisabledReason],
+    );
+
    const startVideoCallButton = (
        <>
            {/* Can be either a menu or just a button depending on the number of call options.*/}
            {callOptions.length > 1 ? (
                <Menu
                    open={menuOpen}
-                    onOpenChange={setMenuOpen}
+                    onOpenChange={onOpenChange}
                    title={_t("voip|video_call_using")}
                    trigger={
                        <IconButton
@ -165,6 +180,7 @@ export default function RoomHeader({
                        <MenuItem
                            key={option}
                            label={getPlatformCallTypeLabel(option)}
+                            aria-label={getPlatformCallTypeLabel(option)}
                            onClick={(ev) => videoCallClick(ev, option)}
                            Icon={VideoCallIcon}
                            onSelect={() => {} /* Dummy handler since we want the click event.*/}
@ -195,7 +211,7 @@ export default function RoomHeader({
    );
    const closeLobbyButton = (
        <Tooltip label={_t("voip|close_lobby")}>
-            <IconButton onClick={toggleCall}>
+            <IconButton onClick={toggleCall} aria-label={_t("voip|close_lobby")}>
                <CloseCallIcon />
            </IconButton>
        </Tooltip>
@ -296,7 +312,7 @@ export default function RoomHeader({

                    {((isConnectedToCall && isViewingCall) || isVideoRoom(room)) && <VideoRoomChatButton room={room} />}

-                    {hasActiveCallSession && !isConnectedToCall ? (
+                    {hasActiveCallSession && !isConnectedToCall && !isViewingCall ? (
                        joinCallButton
                    ) : (
                        <>
--- a/src/hooks/room/useRoomCall.ts
+++ b/src/hooks/room/useRoomCall.ts
@ -61,6 +61,7 @@ const enum State {
    NoPermission,
    Unpinned,
    Ongoing,
+    NotJoined,
 }

 /**
@ -176,7 +177,7 @@ export const useRoomCall = (
        if (activeCalls.find((call) => call.roomId != room.roomId)) {
            return State.Ongoing;
        }
-        if (hasGroupCall || hasJitsiWidget || hasManagedHybridWidget) {
+        if (hasGroupCall && (hasJitsiWidget || hasManagedHybridWidget)) {
            return promptPinWidget ? State.Unpinned : State.Ongoing;
        }
        if (hasLegacyCall) {
@ -243,6 +244,7 @@ export const useRoomCall = (
            videoCallDisabledReason = _t("voip|disabled_no_one_here");
            break;
        case State.Unpinned:
+        case State.NotJoined:
        case State.NoCall:
            voiceCallDisabledReason = null;
            videoCallDisabledReason = null;
--- a/src/hooks/useCall.ts
+++ b/src/hooks/useCall.ts
@ -74,14 +74,14 @@ export const useParticipatingMembers = (call: Call): RoomMember[] => {
    }, [participants]);
 };

-export const useFull = (call: Call): boolean => {
+export const useFull = (call: Call | null): boolean => {
    return (
        useParticipantCount(call) >=
        (SdkConfig.get("element_call").participant_limit ?? DEFAULTS.element_call.participant_limit!)
    );
 };

-export const useJoinCallButtonDisabledTooltip = (call: Call): string | null => {
+export const useJoinCallButtonDisabledTooltip = (call: Call | null): string | null => {
    const isFull = useFull(call);
    const state = useConnectionState(call);

--- a/src/toasts/IncomingCallToast.tsx
+++ b/src/toasts/IncomingCallToast.tsx
@ -14,13 +14,13 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */

-import React, { useCallback, useEffect, useMemo } from "react";
+import React, { useCallback, useEffect, useMemo, useState } from "react";
 import { MatrixEvent } from "matrix-js-sdk/src/matrix";
 // eslint-disable-next-line no-restricted-imports
 import { MatrixRTCSessionManagerEvents } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSessionManager";
 // eslint-disable-next-line no-restricted-imports
 import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession";
-import { Button } from "@vector-im/compound-web";
+import { Button, Tooltip, TooltipProvider } from "@vector-im/compound-web";
 import { Icon as VideoCallIcon } from "@vector-im/compound-design-tokens/icons/video-call-solid.svg";

 import { _t } from "../languageHandler";
@ -41,30 +41,37 @@ import { useDispatcher } from "../hooks/useDispatcher";
 import { ActionPayload } from "../dispatcher/payloads";
 import { Call } from "../models/Call";
 import { AudioID } from "../LegacyCallHandler";
-import { useTypedEventEmitter } from "../hooks/useEventEmitter";
+import { useEventEmitter, useTypedEventEmitter } from "../hooks/useEventEmitter";
 import AccessibleTooltipButton from "../components/views/elements/AccessibleTooltipButton";
+import { CallStore, CallStoreEvent } from "../stores/CallStore";

 export const getIncomingCallToastKey = (callId: string, roomId: string): string => `call_${callId}_${roomId}`;
 const MAX_RING_TIME_MS = 10 * 1000;

 interface JoinCallButtonWithCallProps {
    onClick: (e: ButtonEvent) => void;
-    call: Call;
+    call: Call | null;
+    disabledTooltip: string | undefined;
 }

-function JoinCallButtonWithCall({ onClick, call }: JoinCallButtonWithCallProps): JSX.Element {
-    const disabledTooltip = useJoinCallButtonDisabledTooltip(call);
+function JoinCallButtonWithCall({ onClick, call, disabledTooltip }: JoinCallButtonWithCallProps): JSX.Element {
+    let disTooltip = disabledTooltip;
+    const disabledBecauseFullTooltip = useJoinCallButtonDisabledTooltip(call);
+    disTooltip = disabledTooltip ?? disabledBecauseFullTooltip ?? undefined;

    return (
-        <Button
-            className="mx_IncomingCallToast_joinButton"
-            onClick={onClick}
-            disabled={disabledTooltip !== null}
-            kind="primary"
-            size="sm"
-        >
-            {_t("action|join")}
-        </Button>
+        <Tooltip label={disTooltip ?? _t("voip|video_call")}>
+            <Button
+                className="mx_IncomingCallToast_joinButton"
+                onClick={onClick}
+                disabled={disTooltip != undefined}
+                kind="primary"
+                Icon={VideoCallIcon}
+                size="sm"
+            >
+                {_t("action|join")}
+            </Button>
+        </Tooltip>
    );
 }

@ -77,7 +84,11 @@ export function IncomingCallToast({ notifyEvent }: Props): JSX.Element {
    const room = MatrixClientPeg.safeGet().getRoom(roomId) ?? undefined;
    const call = useCall(roomId);
    const audio = useMemo(() => document.getElementById(AudioID.Ring) as HTMLMediaElement, []);
-
+    const [activeCalls, setActiveCalls] = useState<Call[]>(Array.from(CallStore.instance.activeCalls));
+    useEventEmitter(CallStore.instance, CallStoreEvent.ActiveCalls, () => {
+        setActiveCalls(Array.from(CallStore.instance.activeCalls));
+    });
+    const otherCallIsOngoing = activeCalls.find((call) => call.roomId !== roomId);
    // Start ringing if not already.
    useEffect(() => {
        const isRingToast = (notifyEvent.getContent() as unknown as { notify_type: string })["notify_type"] == "ring";
@ -157,7 +168,7 @@ export function IncomingCallToast({ notifyEvent }: Props): JSX.Element {
    );

    return (
-        <React.Fragment>
+        <TooltipProvider>
            <div>
                <RoomAvatar room={room ?? undefined} size="24px" />
            </div>
@ -178,25 +189,17 @@ export function IncomingCallToast({ notifyEvent }: Props): JSX.Element {
                        />
                    )}
                </div>
-                {call ? (
-                    <JoinCallButtonWithCall onClick={onJoinClick} call={call} />
-                ) : (
-                    <Button
-                        className="mx_IncomingCallToast_joinButton"
-                        onClick={onJoinClick}
-                        kind="primary"
-                        size="sm"
-                        Icon={VideoCallIcon}
-                    >
-                        {_t("action|join")}
-                    </Button>
-                )}
+                <JoinCallButtonWithCall
+                    onClick={onJoinClick}
+                    call={call}
+                    disabledTooltip={otherCallIsOngoing ? "Ongoing call" : undefined}
+                />
            </div>
            <AccessibleTooltipButton
                className="mx_IncomingCallToast_closeButton"
                onClick={onCloseClick}
                title={_t("action|close")}
            />
-        </React.Fragment>
+        </TooltipProvider>
    );
 }
--- a/test/components/views/rooms/RoomHeader-test.tsx
+++ b/test/components/views/rooms/RoomHeader-test.tsx
@ -55,7 +55,9 @@ import { Call, ElementCall } from "../../../../src/models/Call";
 import * as ShieldUtils from "../../../../src/utils/ShieldUtils";
 import { Container, WidgetLayoutStore } from "../../../../src/stores/widgets/WidgetLayoutStore";
 import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";
-
+import * as UseCall from "../../../../src/hooks/useCall";
+import { SdkContextClass } from "../../../../src/contexts/SDKContext";
+import WidgetStore, { IApp } from "../../../../src/stores/WidgetStore";
 jest.mock("../../../../src/utils/ShieldUtils");

 function getWrapper(): RenderOptions {
@ -322,25 +324,30 @@ describe("RoomHeader", () => {
            // allow element calls
            jest.spyOn(room.currentState, "mayClientSendStateEvent").mockReturnValue(true);
            jest.spyOn(WidgetLayoutStore.instance, "isInContainer").mockReturnValue(true);
-
-            jest.spyOn(CallStore.instance, "getCall").mockReturnValue({ widget: {}, on: () => {} } as unknown as Call);
-
+            const widget = { type: "m.jitsi" } as IApp;
+            jest.spyOn(CallStore.instance, "getCall").mockReturnValue({
+                widget,
+                on: () => {},
+            } as unknown as Call);
+            jest.spyOn(WidgetStore.instance, "getApps").mockReturnValue([widget]);
            const { container } = render(<RoomHeader room={room} />, getWrapper());
            expect(getByLabelText(container, "Ongoing call")).toHaveAttribute("aria-disabled", "true");
        });

        it("clicking on ongoing (unpinned) call re-pins it", () => {
-            jest.spyOn(SdkConfig, "get").mockReturnValue({ use_exclusively: true });
+            mockRoomMembers(room, 3);
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
            // allow calls
            jest.spyOn(room.currentState, "mayClientSendStateEvent").mockReturnValue(true);
            jest.spyOn(WidgetLayoutStore.instance, "isInContainer").mockReturnValue(false);
            const spy = jest.spyOn(WidgetLayoutStore.instance, "moveToContainer");

-            const widget = {};
+            const widget = { type: "m.jitsi" } as IApp;
            jest.spyOn(CallStore.instance, "getCall").mockReturnValue({
                widget,
                on: () => {},
            } as unknown as Call);
+            jest.spyOn(WidgetStore.instance, "getApps").mockReturnValue([widget]);

            const { container } = render(<RoomHeader room={room} />, getWrapper());
            expect(getByLabelText(container, "Video call")).not.toHaveAttribute("aria-disabled", "true");
@ -431,6 +438,57 @@ describe("RoomHeader", () => {
            fireEvent.click(videoButton);
            expect(dispatcherSpy).toHaveBeenCalledWith(expect.objectContaining({ view_call: true }));
        });
+
+        it("buttons are disabled if there is an ongoing call", async () => {
+            mockRoomMembers(room, 3);
+
+            jest.spyOn(CallStore.prototype, "activeCalls", "get").mockReturnValue(
+                new Set([{ roomId: "some_other_room" } as Call]),
+            );
+            const { container } = render(<RoomHeader room={room} />, getWrapper());
+
+            const [videoButton, voiceButton] = getAllByLabelText(container, "Ongoing call");
+
+            expect(voiceButton).toHaveAttribute("aria-disabled", "true");
+            expect(videoButton).toHaveAttribute("aria-disabled", "true");
+        });
+
+        it("join button is shown if there is an ongoing call", async () => {
+            mockRoomMembers(room, 3);
+            jest.spyOn(UseCall, "useParticipantCount").mockReturnValue(3);
+            const { container } = render(<RoomHeader room={room} />, getWrapper());
+            const joinButton = getByLabelText(container, "Join");
+            expect(joinButton).not.toHaveAttribute("aria-disabled", "true");
+        });
+
+        it("join button is disabled if there is an other ongoing call", async () => {
+            mockRoomMembers(room, 3);
+            jest.spyOn(UseCall, "useParticipantCount").mockReturnValue(3);
+            jest.spyOn(CallStore.prototype, "activeCalls", "get").mockReturnValue(
+                new Set([{ roomId: "some_other_room" } as Call]),
+            );
+            const { container } = render(<RoomHeader room={room} />, getWrapper());
+            const joinButton = getByLabelText(container, "Ongoing call");
+
+            expect(joinButton).toHaveAttribute("aria-disabled", "true");
+        });
+
+        it("close lobby button is shown", async () => {
+            mockRoomMembers(room, 3);
+
+            jest.spyOn(SdkContextClass.instance.roomViewStore, "isViewingCall").mockReturnValue(true);
+            const { container } = render(<RoomHeader room={room} />, getWrapper());
+            getByLabelText(container, "Close lobby");
+        });
+
+        it("close lobby button is shown if there is an ongoing call but we are viewing the lobby", async () => {
+            mockRoomMembers(room, 3);
+            jest.spyOn(UseCall, "useParticipantCount").mockReturnValue(3);
+            jest.spyOn(SdkContextClass.instance.roomViewStore, "isViewingCall").mockReturnValue(true);
+
+            const { container } = render(<RoomHeader room={room} />, getWrapper());
+            getByLabelText(container, "Close lobby");
+        });
    });

    describe("public room", () => {