Show chat panel when opening a video room with unread messages

src/components/structures/RightPanel.tsx

@@ -91,12 +91,6 @@ export default class RightPanel extends React.Component<IProps, IState> {
             currentCard = RightPanelStore.instance.currentCardForRoom(props.room.roomId);
         }
 
-        if (currentCard?.phase && !RightPanelStore.instance.isPhaseValid(currentCard.phase, !!props.room)) {
-            // XXX: We can probably get rid of this workaround once GroupView is dead, it's unmounting happens weirdly
-            // late causing the app to soft-crash due to lack of a room object being passed to a RightPanel
-            return null; // skip this update, we're about to be unmounted and don't have the appropriate props
-        }
-
         return {
             cardState: currentCard?.state,
             phase: currentCard?.phase,

src/components/structures/RoomView.tsx

@@ -1018,6 +1018,14 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
         this.updatePermissions(room);
         this.checkWidgets(room);
 
+        if (
+            this.getMainSplitContentType(room) !== MainSplitContentType.Timeline
+            && RoomNotificationStateStore.instance.getRoomState(room).isUnread
+        ) {
+            // Automatically open the chat panel to make unread messages easier to discover
+            RightPanelStore.instance.setCard({ phase: RightPanelPhases.Timeline }, true, room.roomId);
+        }
+
         this.setState({
             tombstone: this.getRoomTombstone(room),
             liveTimeline: room.getLiveTimeline(),

src/stores/right-panel/RightPanelStore.ts

@@ -141,12 +141,13 @@ export default class RightPanelStore extends ReadyWatchingStore {
             const hist = this.byRoom[rId]?.history ?? [];
             hist[hist.length - 1].state = cardState;
             this.emitAndUpdateSettings();
-        } else if (targetPhase !== this.currentCard?.phase) {
-            // Set right panel and erase history.
-            this.show();
-            this.setRightPanelCache({ phase: targetPhase, state: cardState ?? {} }, rId);
+        } else if (targetPhase !== this.currentCard?.phase || !this.byRoom[rId]) {
+            // Set right panel and initialize/erase history
+            const history = [{ phase: targetPhase, state: cardState ?? {} }];
+            this.byRoom[rId] = { history, isOpen: true };
+            this.emitAndUpdateSettings();
         } else {
-            this.show();
+            this.show(rId);
             this.emitAndUpdateSettings();
         }
     }
@@ -156,7 +157,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
         const rId = roomId ?? this.viewedRoomId;
         const history = cards.map(c => ({ phase: c.phase, state: c.state ?? {} }));
         this.byRoom[rId] = { history, isOpen: true };
-        this.show();
+        this.show(rId);
         this.emitAndUpdateSettings();
     }
 
@@ -187,7 +188,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
                 isOpen: !allowClose,
             };
         }
-        this.show();
+        this.show(rId);
         this.emitAndUpdateSettings();
     }
 
@@ -200,23 +201,23 @@ export default class RightPanelStore extends ReadyWatchingStore {
         return removedCard;
     }
 
-    public togglePanel(roomId: string = null) {
+    public togglePanel(roomId?: string) {
         const rId = roomId ?? this.viewedRoomId;
         if (!this.byRoom[rId]) return;
 
         this.byRoom[rId].isOpen = !this.byRoom[rId].isOpen;
         this.emitAndUpdateSettings();
     }
 
-    public show() {
-        if (!this.isOpen) {
-            this.togglePanel();
+    public show(roomId?: string) {
+        if (!this.isOpenForRoom(roomId ?? this.viewedRoomId)) {
+            this.togglePanel(roomId);
         }
     }
 
-    public hide() {
-        if (this.isOpen) {
-            this.togglePanel();
+    public hide(roomId?: string) {
+        if (this.isOpenForRoom(roomId ?? this.viewedRoomId)) {
+            this.togglePanel(roomId);
         }
     }
 
@@ -228,7 +229,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
             this.byRoom[this.viewedRoomId] = this.byRoom[this.viewedRoomId] ??
                 convertToStatePanel(SettingsStore.getValue("RightPanel.phases", this.viewedRoomId), room);
         } else {
-            console.warn("Could not restore the right panel after load because there was no associated room object.");
+            logger.warn("Could not restore the right panel after load because there was no associated room object.");
         }
     }
 
@@ -273,37 +274,31 @@ export default class RightPanelStore extends ReadyWatchingStore {
             case RightPanelPhases.ThreadView:
                 if (!SettingsStore.getValue("feature_thread")) return false;
                 if (!card.state.threadHeadEvent) {
-                    console.warn("removed card from right panel because of missing threadHeadEvent in card state");
+                    logger.warn("removed card from right panel because of missing threadHeadEvent in card state");
                 }
                 return !!card.state.threadHeadEvent;
             case RightPanelPhases.RoomMemberInfo:
             case RightPanelPhases.SpaceMemberInfo:
             case RightPanelPhases.EncryptionPanel:
                 if (!card.state.member) {
-                    console.warn("removed card from right panel because of missing member in card state");
+                    logger.warn("removed card from right panel because of missing member in card state");
                 }
                 return !!card.state.member;
             case RightPanelPhases.Room3pidMemberInfo:
             case RightPanelPhases.Space3pidMemberInfo:
                 if (!card.state.memberInfoEvent) {
-                    console.warn("removed card from right panel because of missing memberInfoEvent in card state");
+                    logger.warn("removed card from right panel because of missing memberInfoEvent in card state");
                 }
                 return !!card.state.memberInfoEvent;
             case RightPanelPhases.Widget:
                 if (!card.state.widgetId) {
-                    console.warn("removed card from right panel because of missing widgetId in card state");
+                    logger.warn("removed card from right panel because of missing widgetId in card state");
                 }
                 return !!card.state.widgetId;
         }
         return true;
     }
 
-    private setRightPanelCache(card: IRightPanelCard, roomId?: string) {
-        const history = [{ phase: card.phase, state: card.state ?? {} }];
-        this.byRoom[roomId ?? this.viewedRoomId] = { history, isOpen: true };
-        this.emitAndUpdateSettings();
-    }
-
     private getVerificationRedirect(card: IRightPanelCard): IRightPanelCard {
         if (card.phase === RightPanelPhases.RoomMemberInfo && card.state) {
             // RightPanelPhases.RoomMemberInfo -> needs to be changed to RightPanelPhases.EncryptionPanel if there is a pending verification request
@@ -322,18 +317,11 @@ export default class RightPanelStore extends ReadyWatchingStore {
         return null;
     }
 
-    public isPhaseValid(targetPhase: RightPanelPhases, isViewingRoom = this.isViewingRoom): boolean {
+    public isPhaseValid(targetPhase: RightPanelPhases): boolean {
         if (!RightPanelPhases[targetPhase]) {
             logger.warn(`Tried to switch right panel to unknown phase: ${targetPhase}`);
             return false;
         }
-        if (!isViewingRoom) {
-            logger.warn(
-                `Tried to switch right panel to a room phase: ${targetPhase}, ` +
-                `but we are currently not viewing a room`,
-            );
-            return false;
-        }
         return true;
     }
 
@@ -386,10 +374,6 @@ export default class RightPanelStore extends ReadyWatchingStore {
         this.emitAndUpdateSettings();
     }
 
-    private get isViewingRoom(): boolean {
-        return !!this.viewedRoomId;
-    }
-
     public static get instance(): RightPanelStore {
         if (!RightPanelStore.internalInstance) {
             RightPanelStore.internalInstance = new RightPanelStore();

test/components/structures/RoomView-test.tsx

@@ -16,43 +16,62 @@ limitations under the License.
 
 import React from "react";
 import { mount, ReactWrapper } from "enzyme";
+import { act } from "react-dom/test-utils";
 import { mocked, MockedObject } from "jest-mock";
 import { MatrixClient } from "matrix-js-sdk/src/client";
 import { Room, RoomEvent } from "matrix-js-sdk/src/models/room";
 import { MatrixEvent } from "matrix-js-sdk/src/models/event";
 
-import { stubClient, wrapInMatrixClientContext } from "../../test-utils";
+import { stubClient, mockPlatformPeg, unmockPlatformPeg, wrapInMatrixClientContext } from "../../test-utils";
 import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
 import { Action } from "../../../src/dispatcher/actions";
 import dis from "../../../src/dispatcher/dispatcher";
 import { ViewRoomPayload } from "../../../src/dispatcher/payloads/ViewRoomPayload";
 import { RoomView as _RoomView } from "../../../src/components/structures/RoomView";
 import ResizeNotifier from "../../../src/utils/ResizeNotifier";
 import { RoomViewStore } from "../../../src/stores/RoomViewStore";
+import SettingsStore from "../../../src/settings/SettingsStore";
+import { SettingLevel } from "../../../src/settings/SettingLevel";
 import DMRoomMap from "../../../src/utils/DMRoomMap";
+import { NotificationState } from "../../../src/stores/notifications/NotificationState";
+import RightPanelStore from "../../../src/stores/right-panel/RightPanelStore";
+import { RightPanelPhases } from "../../../src/stores/right-panel/RightPanelStorePhases";
 
 const RoomView = wrapInMatrixClientContext(_RoomView);
 
 describe("RoomView", () => {
     let cli: MockedObject<MatrixClient>;
     let room: Room;
-    beforeEach(() => {
+    let roomCount = 0;
+    beforeEach(async () => {
+        mockPlatformPeg({ reload: () => {} });
         stubClient();
         cli = mocked(MatrixClientPeg.get());
 
-        room = new Room("r1", cli, "@alice:example.com");
+        room = new Room(`!${roomCount++}:example.org`, cli, "@alice:example.org");
         room.getPendingEvents = () => [];
         cli.getRoom.mockReturnValue(room);
         // Re-emit certain events on the mocked client
         room.on(RoomEvent.Timeline, (...args) => cli.emit(RoomEvent.Timeline, ...args));
         room.on(RoomEvent.TimelineReset, (...args) => cli.emit(RoomEvent.TimelineReset, ...args));
 
         DMRoomMap.makeShared();
+
+        RightPanelStore.instance.useUnitTestClient(cli);
+        // @ts-ignore
+        await RightPanelStore.instance.onReady();
+    });
+
+    afterEach(async () => {
+        // @ts-ignore
+        await RightPanelStore.instance.onNotReady();
+        unmockPlatformPeg();
+        jest.restoreAllMocks();
     });
 
     const mountRoomView = async (): Promise<ReactWrapper> => {
         if (RoomViewStore.instance.getRoomId() !== room.roomId) {
-            const switchRoomPromise = new Promise<void>(resolve => {
+            const switchedRoom = new Promise<void>(resolve => {
                 const subscription = RoomViewStore.instance.addListener(() => {
                     if (RoomViewStore.instance.getRoomId()) {
                         subscription.remove();
@@ -67,10 +86,10 @@ describe("RoomView", () => {
                 metricsTrigger: null,
             });
 
-            await switchRoomPromise;
+            await switchedRoom;
         }
 
-        return mount(
+        const roomView = mount(
             <RoomView
                 mxClient={cli}
                 threepidInvite={null}
@@ -81,6 +100,8 @@ describe("RoomView", () => {
                 onRegistered={null}
             />,
         );
+        await act(() => Promise.resolve()); // Allow state to settle
+        return roomView;
     };
     const getRoomViewInstance = async (): Promise<_RoomView> =>
         (await mountRoomView()).find(_RoomView).instance() as _RoomView;
@@ -126,4 +147,25 @@ describe("RoomView", () => {
         room.getUnfilteredTimelineSet().resetLiveTimeline();
         expect(roomViewInstance.state.liveTimeline).not.toEqual(oldTimeline);
     });
+
+    describe("video rooms", () => {
+        beforeEach(async () => {
+            // Make it a video room
+            room.isElementVideoRoom = () => true;
+            await SettingsStore.setValue("feature_video_rooms", null, SettingLevel.DEVICE, true);
+        });
+
+        it("normally doesn't open the chat panel", async () => {
+            jest.spyOn(NotificationState.prototype, "isUnread", "get").mockReturnValue(false);
+            await mountRoomView();
+            expect(RightPanelStore.instance.isOpen).toEqual(false);
+        });
+
+        it("opens the chat panel if there are unread messages", async () => {
+            jest.spyOn(NotificationState.prototype, "isUnread", "get").mockReturnValue(true);
+            await mountRoomView();
+            expect(RightPanelStore.instance.isOpen).toEqual(true);
+            expect(RightPanelStore.instance.currentCard.phase).toEqual(RightPanelPhases.Timeline);
+        });
+    });
 });