Use the separator as border between roomlist and main panel (#33598)

* Remove border from roomlist container

The separator will act as the border so we no longer need the roomlist
border.

* Use pointer events to detect click event

Otherwise the onClick handler would run when you resize the panel.

* Support showing the border in separator

* Update tests

* Disable double click behaviour on separator

* Fix screenshot tests failing

apps/web/res/css/views/rooms/RoomListPanel/_RoomListPanel.pcss

@@ -8,5 +8,4 @@
 .mx_RoomListPanel {
     background-color: var(--cpd-color-bg-canvas-default);
     height: 100%;
-    border-right: 1px solid var(--cpd-color-bg-subtle-primary);
 }

apps/web/src/viewmodels/structures/ResizerViewModel.ts

@@ -47,8 +47,15 @@ export class ResizerViewModel
      */
     private panelHandle?: PanelImperativeHandle;
 
+    /**
+     * Needed to distinguish between a drag and a click on the separator.
+     */
+    private readonly mouseClickHandler: MouseClickHandler;
+
     public constructor() {
         super(undefined, getInitialState());
+        // Run onSeparatorClick when the separator is clicked.
+        this.mouseClickHandler = new MouseClickHandler(this.onSeparatorClick);
     }
 
     public onLeftPanelResize = debounce((panelSize: PanelSize): void => {
@@ -79,13 +86,25 @@ export class ResizerViewModel
         this.panelHandle = handle;
     };
 
-    public onSeparatorClick = (): void => {
+    private onSeparatorClick = (): void => {
         if (this.panelHandle?.isCollapsed()) {
             const lastSize = SettingsStore.getValue("RoomList.panelSize");
             this.panelHandle.resize(`${lastSize ?? 100}%`);
         }
     };
 
+    public onPointerUp = (): void => {
+        this.mouseClickHandler.onPointerUp();
+    };
+
+    public onPointerMove = (): void => {
+        this.mouseClickHandler.onPointerMove();
+    };
+
+    public onPointerDown = (): void => {
+        this.mouseClickHandler.onPointerDown();
+    };
+
     public onFocus = (): void => {
         /**
          * The intention here is to make the separator visible when it is focused by keyboard
@@ -108,3 +127,26 @@ export class ResizerViewModel
         this.snapshot.merge({ isFocusedViaKeyboard: false });
     };
 }
+
+/**
+ * Dragging the separator will emit a click event.
+ * This class uses pointer event handlers to distinguish between a drag and a click
+ * on the separator.
+ */
+class MouseClickHandler {
+    public constructor(private readonly onClick: () => void) {}
+
+    private isResize = false;
+
+    public onPointerUp = (): void => {
+        if (!this.isResize) this.onClick();
+    };
+
+    public onPointerDown = (): void => {
+        this.isResize = false;
+    };
+
+    public onPointerMove = (): void => {
+        this.isResize = true;
+    };
+}

apps/web/test/viewmodels/structures/ResizerViewModel-test.ts

@@ -63,12 +63,33 @@ describe("LeftPanelResizerViewModel", () => {
         });
     });
 
-    it("should noop on onSeparatorClick() when handle is not yet set", () => {
+    it("should noop on click when handle is not yet set", () => {
         const vm = new ResizerViewModel();
-        expect(() => vm.onSeparatorClick()).not.toThrow();
+        expect(() => {
+            // Click
+            vm.onPointerDown();
+            vm.onPointerUp();
+        }).not.toThrow();
     });
 
-    describe("should expand panel on onSeparatorClick()", () => {
+    it("should noop on mouse drag", () => {
+        const vm = new ResizerViewModel();
+        SettingsStore.setValue("RoomList.panelSize", null, SettingLevel.DEVICE, 34);
+        const mockHandle = {
+            resize: jest.fn(),
+            isCollapsed: jest.fn().mockReturnValue(true),
+        } as unknown as PanelImperativeHandle;
+        vm.setPanelHandle(mockHandle);
+
+        // Simulate drag
+        vm.onPointerDown();
+        vm.onPointerMove();
+        vm.onPointerUp();
+
+        expect(mockHandle.resize).not.toHaveBeenCalledWith("34%");
+    });
+
+    describe("should expand panel on click()", () => {
         it("to last non-zero width that the user set", () => {
             const vm = new ResizerViewModel();
             SettingsStore.setValue("RoomList.panelSize", null, SettingLevel.DEVICE, 34);
@@ -78,7 +99,9 @@ describe("LeftPanelResizerViewModel", () => {
             } as unknown as PanelImperativeHandle;
             vm.setPanelHandle(mockHandle);
 
-            vm.onSeparatorClick();
+            // Simulate click
+            vm.onPointerDown();
+            vm.onPointerUp();
 
             expect(mockHandle.resize).toHaveBeenCalledWith("34%");
         });
@@ -91,7 +114,9 @@ describe("LeftPanelResizerViewModel", () => {
             } as unknown as PanelImperativeHandle;
             vm.setPanelHandle(mockHandle);
 
-            vm.onSeparatorClick();
+            // Simulate click
+            vm.onPointerDown();
+            vm.onPointerUp();
 
             expect(mockHandle.resize).toHaveBeenCalledWith("100%");
         });

packages/playwright-common/src/fixtures/panel.ts

@@ -12,7 +12,7 @@ import { test as base } from "./services.js";
  * Rounding this number to a whole number would mean updating a whole
  * bunch of screenshots.
  */
-const LEFT_PANEL_WIDTH = "369.6875px";
+const LEFT_PANEL_WIDTH = "368.6875px";
 
 export const test = base.extend<{
     /**

packages/shared-components/src/resize/separator/SeparatorView.module.css

@@ -6,21 +6,25 @@
  */
 
 .separator {
-    /* Hide the separator by default */
-    display: none;
     /* Necessary to avoid weird focus outlines (doubled on one side, absent on one side etc...) */
     outline-offset: -2px;
 }
 
-.separator[data-separator="hover"],
+/* When type=border, we just render a 1px border */
+.separator[data-separator-type="border"] {
+    width: 0px;
+    border-right: 1px solid var(--cpd-color-bg-subtle-primary);
+}
+
+/* When type=bar, we render the 12px separator */
+.separator[data-separator-type="bar"] {
+    width: 12px;
+    display: flex;
+    align-items: center;
+    border-right: 1px solid var(--cpd-color-bg-subtle-primary);
+}
+
+.separator[data-separator-type="bar"][data-separator="hover"],
 .separator:focus-visible {
     background: var(--cpd-color-bg-action-tertiary-hovered);
 }
-
-.visible {
-    /* Show the separator when the left panel is collapsed */
-    display: flex;
-    align-items: center;
-    width: 12px;
-    border-right: 1px solid var(--cpd-color-bg-subtle-primary);
-}

packages/shared-components/src/resize/separator/SeparatorView.stories.tsx

@@ -16,8 +16,15 @@ import { Flex } from "../../core/utils/Flex";
 
 type SeparatorViewProps = ResizerViewSnapshot & SeparatorViewActions;
 
-const Wrapper = ({ onFocus, onBlur, onSeparatorClick, ...snapshot }: SeparatorViewProps): JSX.Element => {
-    const vm = useMockedViewModel(snapshot, { onFocus, onBlur, onSeparatorClick });
+const Wrapper = ({
+    onFocus,
+    onBlur,
+    onPointerDown,
+    onPointerMove,
+    onPointerUp,
+    ...snapshot
+}: SeparatorViewProps): JSX.Element => {
+    const vm = useMockedViewModel(snapshot, { onFocus, onBlur, onPointerDown, onPointerMove, onPointerUp });
     return <SeparatorView className="Separator" vm={vm} />;
 };
 
@@ -30,7 +37,9 @@ const meta = {
     args: {
         onFocus: fn(),
         onBlur: fn(),
-        onSeparatorClick: fn(),
+        onPointerUp: fn(),
+        onPointerMove: fn(),
+        onPointerDown: fn(),
         isCollapsed: true,
         isFocusedViaKeyboard: false,
     },

packages/shared-components/src/resize/separator/SeparatorView.test.tsx

@@ -23,7 +23,9 @@ class MockViewModel extends BaseViewModel<ResizerViewSnapshot, unknown> implemen
     }
     public onBlur: () => void = vi.fn();
     public onFocus: () => void = vi.fn();
-    public onSeparatorClick: () => void = vi.fn();
+    public onPointerUp: () => void = vi.fn();
+    public onPointerMove: () => void = vi.fn();
+    public onPointerDown: () => void = vi.fn();
 }
 
 function renderPanel(initialSnapshot?: Partial<ResizerViewSnapshot>): MockViewModel {
@@ -55,11 +57,12 @@ describe("<SeparatorView />", () => {
         expect(container).toMatchSnapshot();
     });
 
-    it("should call onSeparatorClick() when clicked", async () => {
+    it("should call onPointerDown and onPointerUp on pointer events", async () => {
         const vm = renderPanel();
         const separator = screen.getByRole("separator");
         await userEvent.click(separator);
-        expect(vm.onSeparatorClick).toHaveBeenCalledOnce();
+        expect(vm.onPointerDown).toHaveBeenCalledOnce();
+        expect(vm.onPointerUp).toHaveBeenCalledOnce();
     });
 
     it("should call onFocus and onBlur when receiving/loosing focus", async () => {

packages/shared-components/src/resize/separator/SeparatorView.tsx

@@ -18,9 +18,19 @@ import { useI18n } from "../../core/i18n/i18nContext";
 
 export interface SeparatorViewActions {
     /**
-     * onClick handler for the separator.
+     * onPointerUp handler for separator.
      */
-    onSeparatorClick: () => void;
+    onPointerUp: () => void;
+
+    /**
+     * onPointerMove handler for separator.
+     */
+    onPointerMove: () => void;
+
+    /**
+     * onPointerDown handler for separator.
+     */
+    onPointerDown: () => void;
 
     /**
      * onFocus handler for the separator.
@@ -45,28 +55,39 @@ export function SeparatorView({ vm, className }: Props): React.ReactNode {
     const { translate: _t } = useI18n();
     const { isCollapsed, isFocusedViaKeyboard } = useViewModel(vm);
 
-    const classes = classNames(styles.separator, className, {
-        [styles.visible]: isCollapsed || isFocusedViaKeyboard,
-    });
+    /**
+     * There are two types of separator:
+     * - bar: This shows a thick bar separator with a resize icon in the middle; shown when the panel is collapsed.
+     * - border: This is just a 1px wide separator; shown when the panel is expanded.
+     */
+    const type = isCollapsed || isFocusedViaKeyboard ? "bar" : "border";
+
+    const barContent = (
+        <Tooltip description={_t("left_panel|separator_label")} placement="right">
+            <DragIcon
+                width="20px"
+                height="12px"
+                // Without a custom view-box, this svg would scale incorrectly and would appear tiny within the separator.
+                // See https://github.com/element-hq/compound/issues/242
+                viewBox="3.999704360961914 8.999704360961914 16.000295639038086 6.000591278076172"
+                transform="rotate(90)"
+            />
+        </Tooltip>
+    );
 
     return (
         <Separator
-            className={classes}
-            onClick={vm.onSeparatorClick}
+            className={classNames(styles.separator, className)}
+            onPointerUp={vm.onPointerUp}
+            onPointerMove={vm.onPointerMove}
+            onPointerDown={vm.onPointerDown}
             onFocus={vm.onFocus}
             onBlur={vm.onBlur}
             aria-label={_t("left_panel|separator_label")}
+            data-separator-type={type}
+            disableDoubleClick
         >
-            <Tooltip description={_t("left_panel|separator_label")} placement="right">
-                <DragIcon
-                    width="20px"
-                    height="12px"
-                    // Without a custom view-box, this svg would scale incorrectly and would appear tiny within the separator.
-                    // See https://github.com/element-hq/compound/issues/242
-                    viewBox="3.999704360961914 8.999704360961914 16.000295639038086 6.000591278076172"
-                    transform="rotate(90)"
-                />
-            </Tooltip>
+            {type === "bar" ? barContent : null}
         </Separator>
     );
 }

packages/shared-components/src/resize/separator/__snapshots__/SeparatorView.test.tsx.snap

@@ -36,8 +36,9 @@ exports[`<SeparatorView /> > renders Default story 1`] = `
         aria-valuemax="99.502"
         aria-valuemin="0"
         aria-valuenow="0"
-        class="SeparatorView-module_separator Separator SeparatorView-module_visible"
+        class="SeparatorView-module_separator Separator"
         data-separator="inactive"
+        data-separator-type="bar"
         data-testid="react-use-id-3"
         id="react-use-id-3"
         role="separator"
@@ -115,8 +116,9 @@ exports[`<SeparatorView /> > renders KeyboardFocused story 1`] = `
         aria-valuemax="99.502"
         aria-valuemin="0"
         aria-valuenow="49.751"
-        class="SeparatorView-module_separator Separator SeparatorView-module_visible"
+        class="SeparatorView-module_separator Separator"
         data-separator="inactive"
+        data-separator-type="bar"
         data-testid="react-use-id-3"
         id="react-use-id-3"
         role="separator"
@@ -188,29 +190,21 @@ exports[`<SeparatorView /> > renders LeftPanelExpanded story 1`] = `
         </div>
       </div>
       <div
+        aria-controls="react-use-id-2"
         aria-label="Click or drag to expand"
         aria-orientation="vertical"
+        aria-valuemax="96.618"
+        aria-valuemin="0"
+        aria-valuenow="48.309"
         class="SeparatorView-module_separator Separator"
         data-separator="inactive"
+        data-separator-type="border"
         data-testid="react-use-id-3"
         id="react-use-id-3"
         role="separator"
         style="flex: 0 0 auto; touch-action: none;"
         tabindex="0"
-      >
-        <svg
-          fill="currentColor"
-          height="12px"
-          transform="rotate(90)"
-          viewBox="3.999704360961914 8.999704360961914 16.000295639038086 6.000591278076172"
-          width="20px"
-          xmlns="http://www.w3.org/2000/svg"
-        >
-          <path
-            d="M5 15a.97.97 0 0 1-.713-.287A.97.97 0 0 1 4 14q0-.424.287-.713A.97.97 0 0 1 5 13h14q.424 0 .712.287.288.288.288.713 0 .424-.288.713A.97.97 0 0 1 19 15zm0-4a.97.97 0 0 1-.713-.287A.97.97 0 0 1 4 10q0-.424.287-.713A.97.97 0 0 1 5 9h14q.424 0 .712.287Q20 9.576 20 10t-.288.713A.97.97 0 0 1 19 11z"
-          />
-        </svg>
-      </div>
+      />
       <div
         data-panel="true"
         data-testid="react-use-id-4"