From f54f03b8b8703a186ca483fe0b08d90232e084c8 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 20 Mar 2022 12:58:24 +0000 Subject: [PATCH 1/8] don't restore MemberInfo from RightPanel history when viewing a room As bringing up a specific MemberInfo when you view a room is freaky, even if it happened to be the last thing you were doing in that room. Fixes https://github.com/vector-im/element-web/issues/21487 --- src/components/structures/RightPanel.tsx | 22 +++++++++++++++++++ src/stores/right-panel/RightPanelStore.ts | 6 ++++- .../right-panel/RightPanelStoreIPanelState.ts | 6 +++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RightPanel.tsx b/src/components/structures/RightPanel.tsx index 65cddbd2253..d129d2ff53a 100644 --- a/src/components/structures/RightPanel.tsx +++ b/src/components/structures/RightPanel.tsx @@ -84,6 +84,14 @@ export default class RightPanel extends React.Component { public componentDidMount(): void { this.context.on(RoomStateEvent.Members, this.onRoomStateMember); RightPanelStore.instance.on(UPDATE_EVENT, this.onRightPanelStoreUpdate); + + if (this.props.room?.roomId) { + // skip cards which are not to be restored from history + // (i.e. memberinfo cards which have already been viewed) + while (RightPanelStore.instance.currentCardForRoom(this.props.room.roomId)?.state.skipFromHistory) { + RightPanelStore.instance.popCard(); + } + } } public componentWillUnmount(): void { @@ -91,6 +99,15 @@ export default class RightPanel extends React.Component { RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate); } + private static skipHistory(card: IRightPanelCard): boolean { + // determines phases whose history should not be persisted per-room + // so we don't find ourselves being teleported into random old MemberInfo + // views when viewing a given room. + // See https://github.com/vector-im/element-web/issues/21487 + return (card.phase === RightPanelPhases.RoomMemberInfo || + card.phase === RightPanelPhases.Room3pidMemberInfo); + } + public static getDerivedStateFromProps(props: IProps): Partial { let currentCard: IRightPanelCard; if (props.room) { @@ -100,6 +117,11 @@ export default class RightPanel extends React.Component { currentCard = RightPanelStore.instance.currentGroup; } + if (RightPanel.skipHistory(currentCard)) { + // having displayed this card, don't restore this card from history in future + RightPanelStore.instance.skipHistoryForCard(currentCard); + } + 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 diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 3ea50ec0bb3..4adc15b2553 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -159,7 +159,7 @@ export default class RightPanelStore extends ReadyWatchingStore { if (!this.isPhaseValid(targetPhase)) return; if ((targetPhase === this.currentCardForRoom(rId)?.phase && !!cardState)) { - // Update state: set right panel with a new state but keep the phase (dont know it this is ever needed...) + // Update state: set right panel with a new state but keep the phase (don't know it this is ever needed...) const hist = this.byRoom[rId]?.history ?? []; hist[hist.length - 1].state = cardState; this.emitAndUpdateSettings(); @@ -214,6 +214,10 @@ export default class RightPanelStore extends ReadyWatchingStore { this.emitAndUpdateSettings(); } + public skipHistoryForCard(card: IRightPanelCard) { + card.state.skipFromHistory = true; + } + public popCard(roomId: string = null) { const rId = roomId ?? this.viewedRoomId; if (!this.byRoom[rId]) return; diff --git a/src/stores/right-panel/RightPanelStoreIPanelState.ts b/src/stores/right-panel/RightPanelStoreIPanelState.ts index 34fecf018d3..877cf1e3f44 100644 --- a/src/stores/right-panel/RightPanelStoreIPanelState.ts +++ b/src/stores/right-panel/RightPanelStoreIPanelState.ts @@ -38,6 +38,8 @@ export interface IRightPanelCardState { threadHeadEvent?: MatrixEvent; initialEvent?: MatrixEvent; isInitialEventHighlighted?: boolean; + // should this card be skipped when switching to a room? + skipFromHistory?: boolean; } export interface IRightPanelCardStateStored { @@ -54,6 +56,8 @@ export interface IRightPanelCardStateStored { threadHeadEventId?: string; initialEventId?: string; isInitialEventHighlighted?: boolean; + // should this card be skipped when switching to a room? + skipFromHistory?: boolean; } export interface IRightPanelCard { @@ -104,6 +108,7 @@ export function convertCardToStore(panelState: IRightPanelCard): IRightPanelCard panelState.state.initialEvent.getId() : undefined, memberId: !!state?.member?.userId ? panelState.state.member.userId : undefined, + skipFromHistory: state.skipFromHistory, }; return { state: stateStored, phase: panelState.phase }; @@ -125,6 +130,7 @@ function convertStoreToCard(panelStateStore: IRightPanelCardStored, room: Room): room.findEventById(stateStored.initialEventId) : undefined, member: !!stateStored?.memberId ? room.getMember(stateStored.memberId) : undefined, + skipFromHistory: stateStored.skipFromHistory, }; return { state: state, phase: panelStateStore.phase }; From dd911ed066d56872271e2e4825f0bd821910558d Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 20 Mar 2022 19:17:38 +0000 Subject: [PATCH 2/8] switch to using isCardStateValid() to prune out the cards to be skipped this fixes the problem where clicking an avatar when RightPanel was hidden would not take you all the way to their MemberInfo. However, it also means that after SAS verif, you return to MemberList rather than MemberInfo, so will need yet another different approach... --- src/components/structures/RightPanel.tsx | 25 +++-------------------- src/stores/right-panel/RightPanelStore.ts | 11 ++++++---- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/components/structures/RightPanel.tsx b/src/components/structures/RightPanel.tsx index d129d2ff53a..21a6b5536ac 100644 --- a/src/components/structures/RightPanel.tsx +++ b/src/components/structures/RightPanel.tsx @@ -84,14 +84,6 @@ export default class RightPanel extends React.Component { public componentDidMount(): void { this.context.on(RoomStateEvent.Members, this.onRoomStateMember); RightPanelStore.instance.on(UPDATE_EVENT, this.onRightPanelStoreUpdate); - - if (this.props.room?.roomId) { - // skip cards which are not to be restored from history - // (i.e. memberinfo cards which have already been viewed) - while (RightPanelStore.instance.currentCardForRoom(this.props.room.roomId)?.state.skipFromHistory) { - RightPanelStore.instance.popCard(); - } - } } public componentWillUnmount(): void { @@ -99,15 +91,6 @@ export default class RightPanel extends React.Component { RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate); } - private static skipHistory(card: IRightPanelCard): boolean { - // determines phases whose history should not be persisted per-room - // so we don't find ourselves being teleported into random old MemberInfo - // views when viewing a given room. - // See https://github.com/vector-im/element-web/issues/21487 - return (card.phase === RightPanelPhases.RoomMemberInfo || - card.phase === RightPanelPhases.Room3pidMemberInfo); - } - public static getDerivedStateFromProps(props: IProps): Partial { let currentCard: IRightPanelCard; if (props.room) { @@ -117,11 +100,6 @@ export default class RightPanel extends React.Component { currentCard = RightPanelStore.instance.currentGroup; } - if (RightPanel.skipHistory(currentCard)) { - // having displayed this card, don't restore this card from history in future - RightPanelStore.instance.skipHistoryForCard(currentCard); - } - 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 @@ -233,6 +211,9 @@ export default class RightPanel extends React.Component { verificationRequest={cardState.verificationRequest} verificationRequestPromise={cardState.verificationRequestPromise} />; + if (this.state.cardState && phase != RightPanelPhases.EncryptionPanel) { + this.state.cardState.skipFromHistory = true; + } break; } case RightPanelPhases.Room3pidMemberInfo: diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 4adc15b2553..101cd91d9e5 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -214,10 +214,6 @@ export default class RightPanelStore extends ReadyWatchingStore { this.emitAndUpdateSettings(); } - public skipHistoryForCard(card: IRightPanelCard) { - card.state.skipFromHistory = true; - } - public popCard(roomId: string = null) { const rId = roomId ?? this.viewedRoomId; if (!this.byRoom[rId]) return; @@ -291,6 +287,12 @@ export default class RightPanelStore extends ReadyWatchingStore { // we store id's of users and matrix events. If are not yet fetched on reload the right panel cannot display them. // or potentially other errors. // (A nicer fix could be to indicate, that the right panel is loading if there is missing state data and re-emit if the data is available) + + if (card.state?.skipFromHistory) { + console.warn("removed card from right panel because we've already shown it and should skip"); + return false; + } + switch (card.phase) { case RightPanelPhases.ThreadView: if (!card.state.threadHeadEvent) { @@ -371,6 +373,7 @@ export default class RightPanelStore extends ReadyWatchingStore { } private onVerificationRequestUpdate = () => { + if (!this.currentCard?.state) return; const { member } = this.currentCard.state; if (!member) return; const pendingRequest = pendingVerificationRequestForUser(member); From 03045bde46b4cb9b58ab44a68fe5f2460f8b2716 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 20 Mar 2022 20:04:27 +0000 Subject: [PATCH 3/8] switch to clearing out member cards on view_room --- src/components/structures/RightPanel.tsx | 3 --- src/stores/right-panel/RightPanelStore.ts | 22 +++++++++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/components/structures/RightPanel.tsx b/src/components/structures/RightPanel.tsx index 21a6b5536ac..65cddbd2253 100644 --- a/src/components/structures/RightPanel.tsx +++ b/src/components/structures/RightPanel.tsx @@ -211,9 +211,6 @@ export default class RightPanel extends React.Component { verificationRequest={cardState.verificationRequest} verificationRequestPromise={cardState.verificationRequestPromise} />; - if (this.state.cardState && phase != RightPanelPhases.EncryptionPanel) { - this.state.cardState.skipFromHistory = true; - } break; } case RightPanelPhases.Room3pidMemberInfo: diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 101cd91d9e5..e5d4b37d166 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -22,7 +22,7 @@ import defaultDispatcher from '../../dispatcher/dispatcher'; import { pendingVerificationRequestForUser } from '../../verification'; import SettingsStore from "../../settings/SettingsStore"; import { RightPanelPhases } from "./RightPanelStorePhases"; -import { ActionPayload } from "../../dispatcher/payloads"; +import { Action } from "../../dispatcher/actions"; import { SettingLevel } from "../../settings/SettingLevel"; import { UPDATE_EVENT } from '../AsyncStore'; import { ReadyWatchingStore } from '../ReadyWatchingStore'; @@ -287,12 +287,6 @@ export default class RightPanelStore extends ReadyWatchingStore { // we store id's of users and matrix events. If are not yet fetched on reload the right panel cannot display them. // or potentially other errors. // (A nicer fix could be to indicate, that the right panel is loading if there is missing state data and re-emit if the data is available) - - if (card.state?.skipFromHistory) { - console.warn("removed card from right panel because we've already shown it and should skip"); - return false; - } - switch (card.phase) { case RightPanelPhases.ThreadView: if (!card.state.threadHeadEvent) { @@ -440,6 +434,20 @@ export default class RightPanelStore extends ReadyWatchingStore { } break; } + case Action.ViewRoom: { + // if we're switching to a room, clear out any stale MemberInfo cards + // in order to fix https://github.com/vector-im/element-web/issues/21487 + if (payload.room_id && this.currentCard?.phase !== RightPanelPhases.EncryptionPanel) { + const panel = this.byRoom[payload.room_id]; + if (panel && panel.history) { + panel.history = panel.history.filter( + (card) => card.phase != RightPanelPhases.RoomMemberInfo && + card.phase != RightPanelPhases.Room3pidMemberInfo + ); + } + } + break; + } } }; From 4d144ba14ac2dcab3ce6187d3d38b9ac0cf7537c Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 20 Mar 2022 20:05:05 +0000 Subject: [PATCH 4/8] switch to clearing out member cards on view_room --- src/stores/right-panel/RightPanelStoreIPanelState.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/stores/right-panel/RightPanelStoreIPanelState.ts b/src/stores/right-panel/RightPanelStoreIPanelState.ts index 877cf1e3f44..34fecf018d3 100644 --- a/src/stores/right-panel/RightPanelStoreIPanelState.ts +++ b/src/stores/right-panel/RightPanelStoreIPanelState.ts @@ -38,8 +38,6 @@ export interface IRightPanelCardState { threadHeadEvent?: MatrixEvent; initialEvent?: MatrixEvent; isInitialEventHighlighted?: boolean; - // should this card be skipped when switching to a room? - skipFromHistory?: boolean; } export interface IRightPanelCardStateStored { @@ -56,8 +54,6 @@ export interface IRightPanelCardStateStored { threadHeadEventId?: string; initialEventId?: string; isInitialEventHighlighted?: boolean; - // should this card be skipped when switching to a room? - skipFromHistory?: boolean; } export interface IRightPanelCard { @@ -108,7 +104,6 @@ export function convertCardToStore(panelState: IRightPanelCard): IRightPanelCard panelState.state.initialEvent.getId() : undefined, memberId: !!state?.member?.userId ? panelState.state.member.userId : undefined, - skipFromHistory: state.skipFromHistory, }; return { state: stateStored, phase: panelState.phase }; @@ -130,7 +125,6 @@ function convertStoreToCard(panelStateStore: IRightPanelCardStored, room: Room): room.findEventById(stateStored.initialEventId) : undefined, member: !!stateStored?.memberId ? room.getMember(stateStored.memberId) : undefined, - skipFromHistory: stateStored.skipFromHistory, }; return { state: state, phase: panelStateStore.phase }; From bd41ee12856b379f8426d57d74433a1d1a540134 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 20 Mar 2022 20:17:04 +0000 Subject: [PATCH 5/8] lint --- src/stores/right-panel/RightPanelStore.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index e5d4b37d166..cf04aaea14f 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -22,6 +22,7 @@ import defaultDispatcher from '../../dispatcher/dispatcher'; import { pendingVerificationRequestForUser } from '../../verification'; import SettingsStore from "../../settings/SettingsStore"; import { RightPanelPhases } from "./RightPanelStorePhases"; +import { ActionPayload } from "../../dispatcher/payloads"; import { Action } from "../../dispatcher/actions"; import { SettingLevel } from "../../settings/SettingLevel"; import { UPDATE_EVENT } from '../AsyncStore'; @@ -442,7 +443,7 @@ export default class RightPanelStore extends ReadyWatchingStore { if (panel && panel.history) { panel.history = panel.history.filter( (card) => card.phase != RightPanelPhases.RoomMemberInfo && - card.phase != RightPanelPhases.Room3pidMemberInfo + card.phase != RightPanelPhases.Room3pidMemberInfo, ); } } From 6347b883078ecb66e344c1c090012b9ea7b68855 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 21 Mar 2022 11:26:30 +0000 Subject: [PATCH 6/8] use optional chaining Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/stores/right-panel/RightPanelStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index cf04aaea14f..850f05efef1 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -440,7 +440,7 @@ export default class RightPanelStore extends ReadyWatchingStore { // in order to fix https://github.com/vector-im/element-web/issues/21487 if (payload.room_id && this.currentCard?.phase !== RightPanelPhases.EncryptionPanel) { const panel = this.byRoom[payload.room_id]; - if (panel && panel.history) { + if (panel?.history) { panel.history = panel.history.filter( (card) => card.phase != RightPanelPhases.RoomMemberInfo && card.phase != RightPanelPhases.Room3pidMemberInfo, From 6234d719f51a23369feff76054ebcbd35ec2fa18 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 21 Mar 2022 11:46:20 +0000 Subject: [PATCH 7/8] watch RVS rtaher than dispatcher for room changes --- src/stores/right-panel/RightPanelStore.ts | 31 ++++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 850f05efef1..fd3de944175 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -22,7 +22,6 @@ import defaultDispatcher from '../../dispatcher/dispatcher'; import { pendingVerificationRequestForUser } from '../../verification'; import SettingsStore from "../../settings/SettingsStore"; import { RightPanelPhases } from "./RightPanelStorePhases"; -import { ActionPayload } from "../../dispatcher/payloads"; import { Action } from "../../dispatcher/actions"; import { SettingLevel } from "../../settings/SettingLevel"; import { UPDATE_EVENT } from '../AsyncStore'; @@ -380,9 +379,25 @@ export default class RightPanelStore extends ReadyWatchingStore { private onRoomViewStoreUpdate = () => { // TODO: only use this function instead of the onDispatch (the whole onDispatch can get removed!) as soon groups are removed + const oldRoomId = this.viewedRoomId; this.viewedRoomId = RoomViewStore.getRoomId(); // load values from byRoomCache with the viewedRoomId. this.loadCacheFromSettings(); + + // if we're switching to a room, clear out any stale MemberInfo cards + // in order to fix https://github.com/vector-im/element-web/issues/21487 + if (oldRoomId !== this.viewedRoomId) { + if (this.currentCard?.phase !== RightPanelPhases.EncryptionPanel) { + const panel = this.byRoom[this.viewedRoomId]; + if (panel?.history) { + panel.history = panel.history.filter( + (card) => card.phase != RightPanelPhases.RoomMemberInfo && + card.phase != RightPanelPhases.Room3pidMemberInfo, + ); + } + } + } + // If the right panel stays open mode is used, and the panel was either // closed or never shown for that room, then force it open and display // the room member list. @@ -435,20 +450,6 @@ export default class RightPanelStore extends ReadyWatchingStore { } break; } - case Action.ViewRoom: { - // if we're switching to a room, clear out any stale MemberInfo cards - // in order to fix https://github.com/vector-im/element-web/issues/21487 - if (payload.room_id && this.currentCard?.phase !== RightPanelPhases.EncryptionPanel) { - const panel = this.byRoom[payload.room_id]; - if (panel?.history) { - panel.history = panel.history.filter( - (card) => card.phase != RightPanelPhases.RoomMemberInfo && - card.phase != RightPanelPhases.Room3pidMemberInfo, - ); - } - } - break; - } } }; From d23b994e0859a23461ccaa79b7fe747f48081381 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 21 Mar 2022 11:52:56 +0000 Subject: [PATCH 8/8] fix import --- src/stores/right-panel/RightPanelStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index fd3de944175..ffac86139aa 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -22,7 +22,7 @@ import defaultDispatcher from '../../dispatcher/dispatcher'; import { pendingVerificationRequestForUser } from '../../verification'; import SettingsStore from "../../settings/SettingsStore"; import { RightPanelPhases } from "./RightPanelStorePhases"; -import { Action } from "../../dispatcher/actions"; +import { ActionPayload } from "../../dispatcher/payloads"; import { SettingLevel } from "../../settings/SettingLevel"; import { UPDATE_EVENT } from '../AsyncStore'; import { ReadyWatchingStore } from '../ReadyWatchingStore';