Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove space-specific right panel store handling #28538

Merged
merged 6 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 5 additions & 20 deletions src/components/structures/RightPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@ export default class RightPanel extends React.Component<Props, IState> {
}

// redraw the badge on the membership list
if (this.state.phase === RightPanelPhases.RoomMemberList) {
if (this.state.phase === RightPanelPhases.MemberList) {
this.delayedUpdate();
} else if (
this.state.phase === RightPanelPhases.RoomMemberInfo &&
this.state.phase === RightPanelPhases.MemberInfo &&
member.userId === this.state.cardState?.member?.userId
) {
// refresh the member info (e.g. new power level)
Comment on lines 111 to 118
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part isn't really testable, relies on forceUpdate and is all sorts of hellish

Expand Down Expand Up @@ -157,7 +157,7 @@ export default class RightPanel extends React.Component<Props, IState> {
const phase = this.props.overwriteCard?.phase ?? this.state.phase;
const cardState = this.props.overwriteCard?.state ?? this.state.cardState;
switch (phase) {
case RightPanelPhases.RoomMemberList:
case RightPanelPhases.MemberList:
if (!!roomId) {
card = (
<MemberList
Expand All @@ -170,22 +170,8 @@ export default class RightPanel extends React.Component<Props, IState> {
);
}
break;
case RightPanelPhases.SpaceMemberList:
if (!!cardState?.spaceId || !!roomId) {
card = (
<MemberList
roomId={cardState?.spaceId ?? roomId!}
key={cardState?.spaceId ?? roomId!}
onClose={this.onClose}
searchQuery={this.state.searchQuery}
onSearchQueryChanged={this.onSearchQueryChanged}
/>
);
}
break;

case RightPanelPhases.RoomMemberInfo:
case RightPanelPhases.SpaceMemberInfo:
case RightPanelPhases.MemberInfo:
case RightPanelPhases.EncryptionPanel: {
if (!!cardState?.member) {
const roomMember = cardState.member instanceof RoomMember ? cardState.member : undefined;
Expand All @@ -203,8 +189,7 @@ export default class RightPanel extends React.Component<Props, IState> {
}
break;
}
case RightPanelPhases.Room3pidMemberInfo:
case RightPanelPhases.Space3pidMemberInfo:
case RightPanelPhases.ThreePidMemberInfo:
if (!!cardState?.memberInfoEvent) {
card = (
<ThirdPartyMemberInfo event={cardState.memberInfoEvent} key={roomId} onClose={this.onClose} />
Expand Down
8 changes: 4 additions & 4 deletions src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1239,18 +1239,18 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
if (payload.member) {
if (payload.push) {
RightPanelStore.instance.pushCard({
phase: RightPanelPhases.RoomMemberInfo,
phase: RightPanelPhases.MemberInfo,
state: { member: payload.member },
});
} else {
RightPanelStore.instance.setCards([
{ phase: RightPanelPhases.RoomSummary },
{ phase: RightPanelPhases.RoomMemberList },
{ phase: RightPanelPhases.RoomMemberInfo, state: { member: payload.member } },
{ phase: RightPanelPhases.MemberList },
{ phase: RightPanelPhases.MemberInfo, state: { member: payload.member } },
]);
}
} else {
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.RoomMemberList);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.MemberList);
}
break;
case Action.View3pidInvite:
Expand Down
4 changes: 2 additions & 2 deletions src/components/structures/SpaceRoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ const SpaceLanding: React.FC<{ space: Room }> = ({ space }) => {
const storeIsShowingSpaceMembers = useCallback(
() =>
RightPanelStore.instance.isOpenForRoom(space.roomId) &&
RightPanelStore.instance.currentCardForRoom(space.roomId)?.phase === RightPanelPhases.SpaceMemberList,
RightPanelStore.instance.currentCardForRoom(space.roomId)?.phase === RightPanelPhases.MemberList,
[space.roomId],
);
const isShowingMembers = useEventEmitterState(RightPanelStore.instance, UPDATE_EVENT, storeIsShowingSpaceMembers);
Expand Down Expand Up @@ -251,7 +251,7 @@ const SpaceLanding: React.FC<{ space: Room }> = ({ space }) => {
}

const onMembersClick = (): void => {
RightPanelStore.instance.setCard({ phase: RightPanelPhases.SpaceMemberList });
RightPanelStore.instance.setCard({ phase: RightPanelPhases.MemberList });
};

return (
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/UserView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export default class UserView extends React.Component<IProps, IState> {
} else if (this.state.member) {
const panel = (
<RightPanel
overwriteCard={{ phase: RightPanelPhases.RoomMemberInfo, state: { member: this.state.member } }}
overwriteCard={{ phase: RightPanelPhases.MemberInfo, state: { member: this.state.member } }}
resizeNotifier={this.props.resizeNotifier}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/right_panel/RoomSummaryCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ interface IProps {
}

const onRoomMembersClick = (): void => {
RightPanelStore.instance.pushCard({ phase: RightPanelPhases.RoomMemberList }, true);
RightPanelStore.instance.pushCard({ phase: RightPanelPhases.MemberList }, true);
};

const onRoomThreadsClick = (): void => {
Expand Down
11 changes: 4 additions & 7 deletions src/components/views/right_panel/UserInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1739,13 +1739,13 @@ export const UserInfoHeader: React.FC<{
interface IProps {
user: Member;
room?: Room;
phase: RightPanelPhases.RoomMemberInfo | RightPanelPhases.SpaceMemberInfo | RightPanelPhases.EncryptionPanel;
phase: RightPanelPhases.MemberInfo | RightPanelPhases.EncryptionPanel;
onClose(): void;
verificationRequest?: VerificationRequest;
verificationRequestPromise?: Promise<VerificationRequest>;
}

const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPhases.RoomMemberInfo, ...props }) => {
const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPhases.MemberInfo, ...props }) => {
const cli = useContext(MatrixClientContext);

// fetch latest room member if we have a room, so we don't show historical information, falling back to user
Expand All @@ -1767,8 +1767,6 @@ const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPha
// We have no previousPhase for when viewing a UserInfo without a Room at this time
if (room && phase === RightPanelPhases.EncryptionPanel) {
cardState = { member };
} else if (room?.isSpaceRoom()) {
cardState = { spaceId: room.roomId };
}

const onEncryptionPanelClose = (): void => {
Expand All @@ -1777,8 +1775,7 @@ const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPha

let content: JSX.Element | undefined;
switch (phase) {
case RightPanelPhases.RoomMemberInfo:
case RightPanelPhases.SpaceMemberInfo:
case RightPanelPhases.MemberInfo:
content = (
<BasicUserInfo
room={room as Room}
Expand Down Expand Up @@ -1823,7 +1820,7 @@ const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPha
closeLabel={closeLabel}
cardState={cardState}
onBack={(ev: ButtonEvent) => {
if (RightPanelStore.instance.previousCard.phase === RightPanelPhases.RoomMemberList) {
if (RightPanelStore.instance.previousCard.phase === RightPanelPhases.MemberList) {
PosthogTrackers.trackInteraction("WebRightPanelRoomUserInfoBackButton", ev);
}
Comment on lines +1823 to 1825
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't sanely testable

}}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/rooms/RoomHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ export default function RoomHeader({
viewUserOnClick={false}
tooltipLabel={_t("room|header_face_pile_tooltip")}
onClick={(e: ButtonEvent) => {
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.RoomMemberList);
RightPanelStore.instance.showOrHidePhase(RightPanelPhases.MemberList);
e.stopPropagation();
}}
aria-label={_t("common|n_members", { count: memberCount })}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/rooms/RoomInfoLine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const RoomInfoLine: FC<IProps> = ({ room }) => {
// summary is not still loading
const viewMembers = (): void =>
RightPanelStore.instance.setCard({
phase: room.isSpaceRoom() ? RightPanelPhases.SpaceMemberList : RightPanelPhases.RoomMemberList,
phase: RightPanelPhases.MemberList,
});

members = (
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/toasts/VerificationRequestToast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export default class VerificationRequestToast extends React.PureComponent<IProps
RightPanelStore.instance.setCards(
[
{ phase: RightPanelPhases.RoomSummary },
{ phase: RightPanelPhases.RoomMemberInfo, state: { member } },
{ phase: RightPanelPhases.MemberInfo, state: { member } },
{ phase: RightPanelPhases.EncryptionPanel, state: { verificationRequest: request, member } },
],
undefined,
Expand Down
11 changes: 4 additions & 7 deletions src/stores/right-panel/RightPanelStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,15 +304,13 @@ export default class RightPanelStore extends ReadyWatchingStore {
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.MemberInfo:
case RightPanelPhases.EncryptionPanel:
if (!card.state?.member) {
logger.warn("removed card from right panel because of missing member in card state");
}
return !!card.state?.member;
case RightPanelPhases.Room3pidMemberInfo:
case RightPanelPhases.Space3pidMemberInfo:
case RightPanelPhases.ThreePidMemberInfo:
if (!card.state?.memberInfoEvent) {
logger.warn("removed card from right panel because of missing memberInfoEvent in card state");
}
Expand All @@ -327,7 +325,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
}

private getVerificationRedirect(card: IRightPanelCard): IRightPanelCard | null {
if (card.phase === RightPanelPhases.RoomMemberInfo && card.state) {
if (card.phase === RightPanelPhases.MemberInfo && card.state) {
// RightPanelPhases.RoomMemberInfo -> needs to be changed to RightPanelPhases.EncryptionPanel if there is a pending verification request
const { member } = card.state;
const pendingRequest = member
Expand Down Expand Up @@ -385,8 +383,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
if (panel?.history) {
panel.history = panel.history.filter(
(card: IRightPanelCard) =>
card.phase != RightPanelPhases.RoomMemberInfo &&
card.phase != RightPanelPhases.Room3pidMemberInfo,
card.phase != RightPanelPhases.MemberInfo && card.phase != RightPanelPhases.ThreePidMemberInfo,
);
}
}
Expand Down
4 changes: 0 additions & 4 deletions src/stores/right-panel/RightPanelStoreIPanelState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export interface IRightPanelCardState {
verificationRequest?: VerificationRequest;
verificationRequestPromise?: Promise<VerificationRequest>;
widgetId?: string;
spaceId?: string;
// Room3pidMemberInfo, Space3pidMemberInfo,
memberInfoEvent?: MatrixEvent;
// threads
Expand All @@ -32,7 +31,6 @@ export interface IRightPanelCardStateStored {
memberId?: string;
// we do not store the things associated with verification
widgetId?: string;
spaceId?: string;
// 3pidMemberInfo
memberInfoEventId?: string;
// threads
Expand Down Expand Up @@ -80,7 +78,6 @@ export function convertCardToStore(panelState: IRightPanelCard): IRightPanelCard
const state = panelState.state ?? {};
const stateStored: IRightPanelCardStateStored = {
widgetId: state.widgetId,
spaceId: state.spaceId,
isInitialEventHighlighted: state.isInitialEventHighlighted,
initialEventScrollIntoView: state.initialEventScrollIntoView,
threadHeadEventId: !!state?.threadHeadEvent?.getId() ? state.threadHeadEvent.getId() : undefined,
Expand All @@ -97,7 +94,6 @@ function convertStoreToCard(panelStateStore: IRightPanelCardStored, room: Room):
const stateStored = panelStateStore.state ?? {};
const state: IRightPanelCardState = {
widgetId: stateStored.widgetId,
spaceId: stateStored.spaceId,
isInitialEventHighlighted: stateStored.isInitialEventHighlighted,
initialEventScrollIntoView: stateStored.initialEventScrollIntoView,
threadHeadEvent: !!stateStored?.threadHeadEventId
Expand Down
16 changes: 6 additions & 10 deletions src/stores/right-panel/RightPanelStorePhases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,21 @@ import { _t } from "../../languageHandler";

// These are in their own file because of circular imports being a problem.
export enum RightPanelPhases {
// Room & Space stuff
MemberList = "MemberList",
MemberInfo = "MemberInfo",
ThreePidMemberInfo = "ThreePidMemberInfo",

// Room stuff
RoomMemberList = "RoomMemberList",
FilePanel = "FilePanel",
NotificationPanel = "NotificationPanel",
RoomMemberInfo = "RoomMemberInfo",
EncryptionPanel = "EncryptionPanel",
RoomSummary = "RoomSummary",
Widget = "Widget",
PinnedMessages = "PinnedMessages",
Timeline = "Timeline",
Extensions = "Extensions",

Room3pidMemberInfo = "Room3pidMemberInfo",

// Space stuff
SpaceMemberList = "SpaceMemberList",
SpaceMemberInfo = "SpaceMemberInfo",
Space3pidMemberInfo = "Space3pidMemberInfo",

// Thread stuff
ThreadView = "ThreadView",
ThreadPanel = "ThreadPanel",
Expand All @@ -42,7 +38,7 @@ export function backLabelForPhase(phase: RightPanelPhases | null): string | null
return _t("chat_card_back_action_label");
case RightPanelPhases.RoomSummary:
return _t("room_summary_card_back_action_label");
case RightPanelPhases.RoomMemberList:
case RightPanelPhases.MemberList:
return _t("member_list_back_action_label");
case RightPanelPhases.ThreadView:
return _t("thread_view_back_action_label");
Expand Down
4 changes: 2 additions & 2 deletions src/stores/right-panel/action-handlers/View3pidInvite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import { RightPanelPhases } from "../RightPanelStorePhases";
export const onView3pidInvite = (payload: ActionPayload, rightPanelStore: RightPanelStore): void => {
if (payload.event) {
rightPanelStore.pushCard({
phase: RightPanelPhases.Room3pidMemberInfo,
phase: RightPanelPhases.ThreePidMemberInfo,
state: { memberInfoEvent: payload.event },
});
} else {
rightPanelStore.showOrHidePhase(RightPanelPhases.RoomMemberList);
rightPanelStore.showOrHidePhase(RightPanelPhases.MemberList);
}
};
2 changes: 1 addition & 1 deletion src/verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function setRightPanel(state: IRightPanelCardState): void {
} else {
RightPanelStore.instance.setCards([
{ phase: RightPanelPhases.RoomSummary },
{ phase: RightPanelPhases.RoomMemberInfo, state: { member: state.member } },
{ phase: RightPanelPhases.MemberInfo, state: { member: state.member } },
{ phase: RightPanelPhases.EncryptionPanel, state },
]);
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit-tests/components/structures/RightPanel-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe("RightPanel", () => {
if (name !== "RightPanel.phases") return realGetValue(name, roomId);
if (roomId === "r1") {
return {
history: [{ phase: RightPanelPhases.RoomMemberList }],
history: [{ phase: RightPanelPhases.MemberList }],
isOpen: true,
};
}
Expand Down Expand Up @@ -123,7 +123,7 @@ describe("RightPanel", () => {
await rpsUpdated;
await waitFor(() => expect(screen.queryByTestId("spinner")).not.toBeInTheDocument());

// room one will be in the RoomMemberList phase - confirm this is rendered
// room one will be in the MemberList phase - confirm this is rendered
expect(container.getElementsByClassName("mx_MemberList")).toHaveLength(1);

// wait for RPS room 2 updates to fire, then rerender
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,7 @@ describe("<RoomSummaryCard />", () => {

fireEvent.click(getByText("People"));

expect(RightPanelStore.instance.pushCard).toHaveBeenCalledWith(
{ phase: RightPanelPhases.RoomMemberList },
true,
);
expect(RightPanelStore.instance.pushCard).toHaveBeenCalledWith({ phase: RightPanelPhases.MemberList }, true);
});

it("opens room threads list on button click", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ describe("<UserInfo />", () => {
const defaultProps = {
user: defaultUser,
// idk what is wrong with this type
phase: RightPanelPhases.RoomMemberInfo as RightPanelPhases.RoomMemberInfo,
phase: RightPanelPhases.MemberInfo as RightPanelPhases.MemberInfo,
onClose: jest.fn(),
};

Expand Down Expand Up @@ -455,7 +455,7 @@ describe("<UserInfo />", () => {
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false));

const { container } = renderComponent({
phase: RightPanelPhases.SpaceMemberInfo,
phase: RightPanelPhases.MemberInfo,
verificationRequest,
room: mockRoom,
});
Expand Down Expand Up @@ -649,7 +649,7 @@ describe("<UserInfo />", () => {
mockClient.getDomain.mockReturnValue("example.com");

const { container } = renderComponent({
phase: RightPanelPhases.RoomMemberInfo,
phase: RightPanelPhases.MemberInfo,
room: mockRoom,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe("<PinnedMessageBanner />", () => {
// The Right panel is opened on another card
jest.spyOn(RightPanelStore.instance, "isOpenForRoom").mockReturnValue(true);
jest.spyOn(RightPanelStore.instance, "currentCard", "get").mockReturnValue({
phase: RightPanelPhases.RoomMemberList,
phase: RightPanelPhases.MemberList,
});

renderBanner();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe("RoomHeader", () => {

fireEvent.click(facePile);

expect(setCardSpy).toHaveBeenCalledWith({ phase: RightPanelPhases.RoomMemberList });
expect(setCardSpy).toHaveBeenCalledWith({ phase: RightPanelPhases.MemberList });
});

it("has room info icon that opens the room info panel", async () => {
Expand Down
Loading
Loading