Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Allow maintaining a different right panel width for thread panels #11064

Merged
merged 11 commits into from
Jun 15, 2023
33 changes: 30 additions & 3 deletions src/components/structures/MainSplit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,24 @@ interface IProps {
collapsedRhs?: boolean;
panel?: JSX.Element;
children: ReactNode;
/**
* A unique identifier for this panel split.
*
* This is appended to the key used to store the panel size in localStorage, allowing the widths of different
* panels to be stored.
*/
sizeKey?: string;
/**
* The size to use for the panel component if one isn't persisted in storage. Defaults to 350.
*/
defaultSize: number;
}

export default class MainSplit extends React.Component<IProps> {
public static defaultProps = {
defaultSize: 350,
};

private onResizeStart = (): void => {
this.props.resizeNotifier.startResizing();
};
Expand All @@ -37,21 +52,32 @@ export default class MainSplit extends React.Component<IProps> {
this.props.resizeNotifier.notifyRightHandleResized();
};

private get sizeSettingStorageKey(): string {
let key = "mx_rhs_size";
if (!!this.props.sizeKey) {
key += `_${this.props.sizeKey}`;
}
return key;
}

private onResizeStop = (
event: MouseEvent | TouchEvent,
direction: Direction,
elementRef: HTMLElement,
delta: NumberSize,
): void => {
this.props.resizeNotifier.stopResizing();
window.localStorage.setItem("mx_rhs_size", (this.loadSidePanelSize().width + delta.width).toString());
window.localStorage.setItem(
this.sizeSettingStorageKey,
(this.loadSidePanelSize().width + delta.width).toString(),
);
};

private loadSidePanelSize(): { height: string | number; width: number } {
let rhsSize = parseInt(window.localStorage.getItem("mx_rhs_size")!, 10);
let rhsSize = parseInt(window.localStorage.getItem(this.sizeSettingStorageKey)!, 10);

if (isNaN(rhsSize)) {
rhsSize = 350;
rhsSize = this.props.defaultSize;
}

return {
Expand All @@ -70,6 +96,7 @@ export default class MainSplit extends React.Component<IProps> {
if (hasResizer) {
children = (
<Resizable
key={this.props.sizeKey}
defaultSize={this.loadSidePanelSize()}
minWidth={264}
maxWidth="50%"
Expand Down
29 changes: 26 additions & 3 deletions src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ export interface IRoomState {
showApps: boolean;
isPeeking: boolean;
showRightPanel: boolean;
/**
* Whether the right panel shown is either of ThreadPanel or ThreadView.
* Always false when `showRightPanel` is false.
*/
threadRightPanel: boolean;
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
// error object, as from the matrix client/server API
// If we failed to load information about the room,
// store the error here.
Expand Down Expand Up @@ -223,7 +228,6 @@ export interface IRoomState {
wasContextSwitch?: boolean;
editState?: EditorStateTransfer;
timelineRenderingType: TimelineRenderingType;
threadId?: string;
liveTimeline?: EventTimeline;
narrow: boolean;
msc3946ProcessDynamicPredecessor: boolean;
Expand Down Expand Up @@ -402,6 +406,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
showApps: false,
isPeeking: false,
showRightPanel: false,
threadRightPanel: false,
joining: false,
showTopUnreadMessagesBar: false,
statusBarVisible: false,
Expand Down Expand Up @@ -623,6 +628,11 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
mainSplitContentType: room ? this.getMainSplitContentType(room) : undefined,
initialEventId: undefined, // default to clearing this, will get set later in the method if needed
showRightPanel: roomId ? this.context.rightPanelStore.isOpenForRoom(roomId) : false,
threadRightPanel: roomId
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
? [RightPanelPhases.ThreadView, RightPanelPhases.ThreadPanel].includes(
this.context.rightPanelStore.currentCardForRoom(roomId).phase!,
)
: false,
activeCall: roomId ? CallStore.instance.getActiveCall(roomId) : null,
};

Expand Down Expand Up @@ -1011,8 +1021,14 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
}

private onRightPanelStoreUpdate = (): void => {
const { roomId } = this.state;
this.setState({
showRightPanel: this.state.roomId ? this.context.rightPanelStore.isOpenForRoom(this.state.roomId) : false,
showRightPanel: roomId ? this.context.rightPanelStore.isOpenForRoom(roomId) : false,
threadRightPanel: roomId
? [RightPanelPhases.ThreadView, RightPanelPhases.ThreadPanel].includes(
this.context.rightPanelStore.currentCardForRoom(roomId).phase!,
)
: false,
});
};

Expand Down Expand Up @@ -2439,7 +2455,14 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
viewingCall={viewingCall}
activeCall={this.state.activeCall}
/>
<MainSplit panel={rightPanel} resizeNotifier={this.props.resizeNotifier}>
<MainSplit
panel={rightPanel}
resizeNotifier={this.props.resizeNotifier}
// Override defaults when a thread is being shown to allow persisting a separate
// right panel width for thread panels as they tend to want to be wider.
sizeKey={this.state.threadRightPanel ? "thread" : undefined}
defaultSize={this.state.threadRightPanel ? 500 : undefined}
Copy link
Member

Choose a reason for hiding this comment

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

a comment explaining this logic wouldn't hurt.

I think the idea is: for a thread panel we default to 500px, for other panels we default to... something else?

Copy link
Member

Choose a reason for hiding this comment

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

according to the IProps on MainSplit, defaultSize must be a number, not undefined?

Copy link
Member Author

@t3chguy t3chguy Jun 14, 2023

Choose a reason for hiding this comment

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

Its an optional prop, because of its presence in defaultProps and React's Defaultize behaviour on types, otherwise TSC strict would have been shouting at me

Copy link
Member

Choose a reason for hiding this comment

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

otherwise TSC strict would have been shouting at me

I did wonder!

>
<div
className={mainSplitContentClasses}
ref={this.roomViewBody}
Expand Down
7 changes: 6 additions & 1 deletion src/contexts/RoomContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ export enum TimelineRenderingType {
Pinned = "Pinned",
}

const RoomContext = createContext<IRoomState>({
const RoomContext = createContext<
IRoomState & {
threadId?: string;
}
>({
roomLoading: true,
peekLoading: false,
shouldPeek: true,
Expand All @@ -39,6 +43,7 @@ const RoomContext = createContext<IRoomState>({
showApps: false,
isPeeking: false,
showRightPanel: true,
threadRightPanel: false,
joining: false,
showTopUnreadMessagesBar: false,
statusBarVisible: false,
Expand Down
63 changes: 63 additions & 0 deletions test/components/structures/MainSplit-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from "react";
import { render } from "@testing-library/react";

import MainSplit from "../../../src/components/structures/MainSplit";
import ResizeNotifier from "../../../src/utils/ResizeNotifier";

describe("<MainSplit/>", () => {
const resizeNotifier = new ResizeNotifier();
const children = (
<div>
Child<span>Foo</span>Bar
</div>
);
const panel = <div>Right panel</div>;

it("renders", () => {
const { asFragment, container } = render(
<MainSplit resizeNotifier={resizeNotifier} children={children} panel={panel} />,
);
expect(asFragment()).toMatchSnapshot();
// Assert it matches the default width of 350
expect(container.querySelector<HTMLElement>(".mx_RightPanel_ResizeWrapper")!.style.width).toBe("350px");
});

it("respects defaultSize prop", () => {
const { asFragment, container } = render(
<MainSplit resizeNotifier={resizeNotifier} children={children} panel={panel} defaultSize={500} />,
);
expect(asFragment()).toMatchSnapshot();
// Assert it matches the default width of 350
expect(container.querySelector<HTMLElement>(".mx_RightPanel_ResizeWrapper")!.style.width).toBe("500px");
});

it("prefers size stashed in LocalStorage to the defaultSize prop", () => {
localStorage.setItem("mx_rhs_size_thread", "333");
const { container } = render(
<MainSplit
resizeNotifier={resizeNotifier}
children={children}
panel={panel}
sizeKey="thread"
defaultSize={400}
/>,
);
expect(container.querySelector<HTMLElement>(".mx_RightPanel_ResizeWrapper")!.style.width).toBe("333px");
});
});
1 change: 1 addition & 0 deletions test/components/structures/RoomView-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ describe("RoomView", () => {
});

it("updates url preview visibility on encryption state change", async () => {
room.getMyMembership = jest.fn().mockReturnValue("join");
// we should be starting unencrypted
expect(cli.isCryptoEnabled()).toEqual(false);
expect(cli.isRoomEncrypted(room.roomId)).toEqual(false);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<MainSplit/> renders 1`] = `
<DocumentFragment>
<div
class="mx_MainSplit"
>
<div>
Child
<span>
Foo
</span>
Bar
</div>
<div
class="mx_RightPanel_ResizeWrapper"
style="position: relative; user-select: auto; width: 350px; height: 100%; max-width: 50%; min-width: 264px; box-sizing: border-box; flex-shrink: 0;"
>
<div>
Right panel
</div>
<div>
<div
class="mx_ResizeHandle--horizontal"
style="position: absolute; user-select: none; width: 10px; height: 100%; top: 0px; left: -5px; cursor: col-resize;"
/>
</div>
</div>
</div>
</DocumentFragment>
`;

exports[`<MainSplit/> respects defaultSize prop 1`] = `
<DocumentFragment>
<div
class="mx_MainSplit"
>
<div>
Child
<span>
Foo
</span>
Bar
</div>
<div
class="mx_RightPanel_ResizeWrapper"
style="position: relative; user-select: auto; width: 500px; height: 100%; max-width: 50%; min-width: 264px; box-sizing: border-box; flex-shrink: 0;"
>
<div>
Right panel
</div>
<div>
<div
class="mx_ResizeHandle--horizontal"
style="position: absolute; user-select: none; width: 10px; height: 100%; top: 0px; left: -5px; cursor: col-resize;"
/>
</div>
</div>
</div>
</DocumentFragment>
`;
1 change: 1 addition & 0 deletions test/components/views/rooms/SendMessageComposer-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe("<SendMessageComposer/>", () => {
showApps: false,
isPeeking: false,
showRightPanel: true,
threadRightPanel: false,
joining: false,
atEndOfLiveTimeline: true,
showTopUnreadMessagesBar: false,
Expand Down
1 change: 1 addition & 0 deletions test/test-utils/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export function getRoomContext(room: Room, override: Partial<IRoomState>): IRoom
showApps: false,
isPeeking: false,
showRightPanel: true,
threadRightPanel: false,
joining: false,
atEndOfLiveTimeline: true,
showTopUnreadMessagesBar: false,
Expand Down
1 change: 1 addition & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ export function createTestClient(): MatrixClient {
searchUserDirectory: jest.fn().mockResolvedValue({ limited: false, results: [] }),
setDeviceVerified: jest.fn(),
joinRoom: jest.fn(),
getSyncStateData: jest.fn(),
} as unknown as MatrixClient;

client.reEmitter = new ReEmitter(client);
Expand Down