diff --git a/apps/web/src/viewmodels/structures/ResizerViewModel.ts b/apps/web/src/viewmodels/structures/ResizerViewModel.ts index 322db045f1..2d9da268db 100644 --- a/apps/web/src/viewmodels/structures/ResizerViewModel.ts +++ b/apps/web/src/viewmodels/structures/ResizerViewModel.ts @@ -15,7 +15,6 @@ import { type ResizerViewSnapshot, } from "@element-hq/web-shared-components"; import { debounce } from "lodash"; -import whatInput from "what-input"; import SettingsStore from "../../settings/SettingsStore"; import { SettingLevel } from "../../settings/SettingLevel"; @@ -25,13 +24,11 @@ function getInitialState(): ResizerViewSnapshot { return { isCollapsed: true, initialSize: 0, - isFocusedViaKeyboard: false, }; } return { isCollapsed: false, initialSize: SettingsStore.getValue("RoomList.panelSize") ?? undefined, - isFocusedViaKeyboard: false, }; } @@ -87,12 +84,18 @@ export class ResizerViewModel }; private onSeparatorClick = (): void => { + // When panel is collapsed, single click should expand the panel. if (this.panelHandle?.isCollapsed()) { const lastSize = SettingsStore.getValue("RoomList.panelSize"); this.panelHandle.resize(`${lastSize ?? 100}%`); } }; + public onDoubleClick = (): void => { + // When the panel is expanded, double click should collapse. + if (!this.panelHandle?.isCollapsed()) this.panelHandle?.collapse(); + }; + public onPointerUp = (): void => { this.mouseClickHandler.onPointerUp(); }; @@ -104,28 +107,6 @@ export class ResizerViewModel public onPointerDown = (): void => { this.mouseClickHandler.onPointerDown(); }; - - public onFocus = (): void => { - /** - * The intention here is to make the separator visible when it is focused by keyboard - * navigation i.e tabbing through the app. - * - * There's a good reason to take this approach instead of just relying on the focus-visible - * selector: - * When exactly an element gets focus-visible is determined by browser heuristics and usually - * interacting with the mouse will not give an element focus-visible. - * However with this separator on chrome, mouse interaction occasionally gives it focus-visible. - * The leads to flakey separator behaviour. - */ - const currentNavigation = whatInput.ask(); - if (currentNavigation === "keyboard") { - this.snapshot.merge({ isFocusedViaKeyboard: true }); - } - }; - - public onBlur = (): void => { - this.snapshot.merge({ isFocusedViaKeyboard: false }); - }; } /** diff --git a/apps/web/test/viewmodels/structures/ResizerViewModel-test.ts b/apps/web/test/viewmodels/structures/ResizerViewModel-test.ts index c8f3d5251e..f25949528a 100644 --- a/apps/web/test/viewmodels/structures/ResizerViewModel-test.ts +++ b/apps/web/test/viewmodels/structures/ResizerViewModel-test.ts @@ -7,7 +7,6 @@ import { waitFor } from "jest-matrix-react"; import { type PanelImperativeHandle } from "@element-hq/web-shared-components"; -import whatInput from "what-input"; import { ResizerViewModel } from "../../../src/viewmodels/structures/ResizerViewModel"; import SettingsStore from "../../../src/settings/SettingsStore"; @@ -27,7 +26,6 @@ describe("LeftPanelResizerViewModel", () => { expect(vm.getSnapshot()).toStrictEqual({ isCollapsed: true, initialSize: 0, - isFocusedViaKeyboard: false, }); }); @@ -37,7 +35,6 @@ describe("LeftPanelResizerViewModel", () => { expect(vm.getSnapshot()).toStrictEqual({ isCollapsed: false, initialSize: 34, - isFocusedViaKeyboard: false, }); }); @@ -46,7 +43,6 @@ describe("LeftPanelResizerViewModel", () => { expect(vm.getSnapshot()).toStrictEqual({ isCollapsed: false, initialSize: undefined, - isFocusedViaKeyboard: false, }); }); }); @@ -89,7 +85,7 @@ describe("LeftPanelResizerViewModel", () => { expect(mockHandle.resize).not.toHaveBeenCalledWith("34%"); }); - describe("should expand panel on click()", () => { + describe("should expand panel on double click when panel is collapsed", () => { it("to last non-zero width that the user set", () => { const vm = new ResizerViewModel(); SettingsStore.setValue("RoomList.panelSize", null, SettingLevel.DEVICE, 34); @@ -98,11 +94,9 @@ describe("LeftPanelResizerViewModel", () => { isCollapsed: jest.fn().mockReturnValue(true), } as unknown as PanelImperativeHandle; vm.setPanelHandle(mockHandle); - // Simulate click vm.onPointerDown(); vm.onPointerUp(); - expect(mockHandle.resize).toHaveBeenCalledWith("34%"); }); @@ -113,22 +107,23 @@ describe("LeftPanelResizerViewModel", () => { isCollapsed: jest.fn().mockReturnValue(true), } as unknown as PanelImperativeHandle; vm.setPanelHandle(mockHandle); - // Simulate click vm.onPointerDown(); vm.onPointerUp(); - expect(mockHandle.resize).toHaveBeenCalledWith("100%"); }); }); - it("should set isFocusedViaKeyboard state correctly", () => { - whatInput.ask = jest.fn().mockReturnValue("keyboard"); + it("should collapse panel on click when panel is expanded", () => { const vm = new ResizerViewModel(); - vm.onFocus(); - expect(vm.getSnapshot().isFocusedViaKeyboard).toStrictEqual(true); - vm.onBlur(); - expect(vm.getSnapshot().isFocusedViaKeyboard).toStrictEqual(false); + const mockHandle = { + collapse: jest.fn(), + isCollapsed: jest.fn().mockReturnValue(false), + } as unknown as PanelImperativeHandle; + vm.setPanelHandle(mockHandle); + + vm.onDoubleClick(); + expect(mockHandle.collapse).toHaveBeenCalled(); }); it("should resize to nearest whole number", () => { diff --git a/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-1.png b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-1.png deleted file mode 100644 index 34149eba6c..0000000000 Binary files a/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-1.png and /dev/null differ diff --git a/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-when-collapsed-1.png b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-when-collapsed-1.png new file mode 100644 index 0000000000..c2f165daa3 Binary files /dev/null and b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-when-collapsed-1.png differ diff --git a/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-when-expanded-1.png b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-when-expanded-1.png new file mode 100644 index 0000000000..f9188daa98 Binary files /dev/null and b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/hover-when-expanded-1.png differ diff --git a/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/keyboard-focused-1.png b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/keyboard-focused-1.png index 769bf9de51..d79630a5a0 100644 Binary files a/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/keyboard-focused-1.png and b/packages/shared-components/__vis__/linux/__baselines__/resize/separator/SeparatorView.stories.tsx/keyboard-focused-1.png differ diff --git a/packages/shared-components/__vis__/linux/__baselines__/room/timeline/DateSeparatorView/DateSeparatorView.stories.tsx/with-jump-to-date-picker-auto.png b/packages/shared-components/__vis__/linux/__baselines__/room/timeline/DateSeparatorView/DateSeparatorView.stories.tsx/with-jump-to-date-picker-auto.png index c1092c385f..57890f534b 100644 Binary files a/packages/shared-components/__vis__/linux/__baselines__/room/timeline/DateSeparatorView/DateSeparatorView.stories.tsx/with-jump-to-date-picker-auto.png and b/packages/shared-components/__vis__/linux/__baselines__/room/timeline/DateSeparatorView/DateSeparatorView.stories.tsx/with-jump-to-date-picker-auto.png differ diff --git a/packages/shared-components/src/resize/index.ts b/packages/shared-components/src/resize/index.ts index 7030fbe0fe..00a2b96250 100644 --- a/packages/shared-components/src/resize/index.ts +++ b/packages/shared-components/src/resize/index.ts @@ -27,11 +27,6 @@ export interface ResizerViewSnapshot { * This is the initial size of the panel if available; should be interpreted as percentage. */ initialSize?: number; - /** - * Whether the separator is currently focused by navigating - * to it using keyboard input. - */ - isFocusedViaKeyboard: boolean; } /** diff --git a/packages/shared-components/src/resize/panel/LeftResizablePanelView.stories.tsx b/packages/shared-components/src/resize/panel/LeftResizablePanelView.stories.tsx index b3badd97be..96d2048e86 100644 --- a/packages/shared-components/src/resize/panel/LeftResizablePanelView.stories.tsx +++ b/packages/shared-components/src/resize/panel/LeftResizablePanelView.stories.tsx @@ -41,14 +41,9 @@ const meta = { component: LeftResizablePanelViewWrapper, // This is a structural component, so nothing to visually test. tags: ["autodocs", "!snapshot"], - argTypes: { - // This snapshot state is not relevant for this View. - isFocusedViaKeyboard: { table: { disable: true } }, - }, args: { initialSize: 20, isCollapsed: false, - isFocusedViaKeyboard: false, onLeftPanelResize: fn(), setPanelHandle: fn(), }, diff --git a/packages/shared-components/src/resize/separator/SeparatorView.module.css b/packages/shared-components/src/resize/separator/SeparatorView.module.css index ec418c398b..06c04c5275 100644 --- a/packages/shared-components/src/resize/separator/SeparatorView.module.css +++ b/packages/shared-components/src/resize/separator/SeparatorView.module.css @@ -8,12 +8,33 @@ .separator { /* Necessary to avoid weird focus outlines (doubled on one side, absent on one side etc...) */ outline-offset: -2px; + + /* We set the separator container to relative so that the child container (activeSeparatorContainer) + can be absolutely positioned */ + position: relative; + + display: flex; + justify-content: center; } /* 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); + /* + We reserve 1px of space even though the actual separator is absolute positioned. + This keeps the panel looking the same as when the border was part of the panel. + */ + width: 1px; + + /* We can disable the browser focus ring since we show a thicker separator on focus */ + outline: none; + + .activeSeparatorContainer { + .activeSeparator { + width: 0px; + border-right: 1px solid var(--cpd-color-bg-subtle-primary); + outline: none; + } + } } /* When type=bar, we render the 12px separator */ @@ -28,3 +49,36 @@ .separator:focus-visible { background: var(--cpd-color-bg-action-tertiary-hovered); } + +/* When type=border and the user hovers/focuses the separator, we make the separator thicker */ +.separator[data-separator-type="border"]:is([data-separator="hover"], [data-separator="active"]), +.separator[data-separator-type="border"]:focus-visible { + .activeSeparatorContainer { + .activeSeparator { + width: 0px; + border-right: 2px solid var(--cpd-color-border-interactive-secondary); + outline: none; + } + } +} + +/* Container for the separator when the left panel is expanded */ +.activeSeparatorContainer { + display: flex; + justify-content: center; + + /* + By making this container absolutely positioned an wide enough to cover the entire hit region, + we're able to intercept all the pointer events. + Without this, we'd only capture the events within a narrow 2px/1px border. + */ + position: absolute; + width: 20px; + z-index: 100; + height: 100%; + + .activeSeparator { + /* Fade in/ fade out the border */ + transition: border 0.15s ease-in-out 0.3s; + } +} diff --git a/packages/shared-components/src/resize/separator/SeparatorView.stories.tsx b/packages/shared-components/src/resize/separator/SeparatorView.stories.tsx index 93a589f4b4..d5a3e5f0a7 100644 --- a/packages/shared-components/src/resize/separator/SeparatorView.stories.tsx +++ b/packages/shared-components/src/resize/separator/SeparatorView.stories.tsx @@ -17,14 +17,13 @@ import { Flex } from "../../core/utils/Flex"; type SeparatorViewProps = ResizerViewSnapshot & SeparatorViewActions; const Wrapper = ({ - onFocus, - onBlur, onPointerDown, onPointerMove, onPointerUp, + onDoubleClick, ...snapshot }: SeparatorViewProps): JSX.Element => { - const vm = useMockedViewModel(snapshot, { onFocus, onBlur, onPointerDown, onPointerMove, onPointerUp }); + const vm = useMockedViewModel(snapshot, { onPointerDown, onPointerMove, onPointerUp, onDoubleClick }); return ; }; @@ -35,13 +34,11 @@ const meta = { component: SeparatorViewWrapper, tags: ["autodocs"], args: { - onFocus: fn(), - onBlur: fn(), onPointerUp: fn(), onPointerMove: fn(), onPointerDown: fn(), + onDoubleClick: fn(), isCollapsed: true, - isFocusedViaKeyboard: false, }, parameters: { design: { @@ -118,16 +115,17 @@ export const KeyboardFocused: Story = { decorators: [(Story) => commonDecorator(Story)], args: { isCollapsed: false, - isFocusedViaKeyboard: true, }, play: async ({ canvas, canvasElement }) => { const separator = canvas.getByRole("separator"); separator.focus(); + // Wait for animations to finish + await new Promise((r) => setTimeout(r, 500)); await expect(canvasElement).toMatchImageSnapshot(); }, }; -export const Hover: Story = { +export const HoverWhenCollapsed: Story = { // We'll manually take a screenshot for this story tags: ["autodocs", "!snapshot"], decorators: [ @@ -159,3 +157,47 @@ export const Hover: Story = { await expect(canvasElement).toMatchImageSnapshot(); }, }; + +export const HoverWhenExpanded: Story = { + // We'll manually take a screenshot for this story + tags: ["autodocs", "!snapshot"], + args: { + isCollapsed: false, + }, + decorators: [ + (Story) => ( +
+ +
+ + + LEFT CONTENT + + + + + + MAIN CONTENT + + + +
+ ), + ], + play: async ({ canvas, canvasElement }) => { + const separator = canvas.getByRole("separator"); + separator.dataset.separator = "hover"; + // Wait for animations to finish + await new Promise((r) => setTimeout(r, 500)); + await expect(canvasElement).toMatchImageSnapshot(); + }, +}; diff --git a/packages/shared-components/src/resize/separator/SeparatorView.test.tsx b/packages/shared-components/src/resize/separator/SeparatorView.test.tsx index 2202e42093..fc6ea14191 100644 --- a/packages/shared-components/src/resize/separator/SeparatorView.test.tsx +++ b/packages/shared-components/src/resize/separator/SeparatorView.test.tsx @@ -15,17 +15,16 @@ import * as stories from "./SeparatorView.stories"; import { BaseViewModel } from "../../core/viewmodel"; import { ResizableGroup, Panel, type ResizerViewSnapshot, SeparatorView, type SeparatorViewActions } from ".."; -const { Default, LeftPanelExpanded, KeyboardFocused } = composeStories(stories); +const { Default, LeftPanelExpanded, KeyboardFocused, HoverWhenCollapsed, HoverWhenExpanded } = composeStories(stories); class MockViewModel extends BaseViewModel implements SeparatorViewActions { public constructor(snapshot: ResizerViewSnapshot) { super(undefined, snapshot); } - public onBlur: () => void = vi.fn(); - public onFocus: () => void = vi.fn(); public onPointerUp: () => void = vi.fn(); public onPointerMove: () => void = vi.fn(); public onPointerDown: () => void = vi.fn(); + public onDoubleClick: () => void = vi.fn(); } function renderPanel(initialSnapshot?: Partial): MockViewModel { @@ -57,6 +56,16 @@ describe("", () => { expect(container).toMatchSnapshot(); }); + it("renders HoverWhenCollapsed story", () => { + const { container } = render(); + expect(container).toMatchSnapshot(); + }); + + it("renders HoverWhenExpanded story", () => { + const { container } = render(); + expect(container).toMatchSnapshot(); + }); + it("should call onPointerDown and onPointerUp on pointer events", async () => { const vm = renderPanel(); const separator = screen.getByRole("separator"); @@ -65,12 +74,10 @@ describe("", () => { expect(vm.onPointerUp).toHaveBeenCalledOnce(); }); - it("should call onFocus and onBlur when receiving/loosing focus", async () => { + it("should call onDoubleClick on double click", async () => { const vm = renderPanel(); const separator = screen.getByRole("separator"); - separator.focus(); - expect(vm.onFocus).toHaveBeenCalled(); - separator.blur(); - expect(vm.onBlur).toHaveBeenCalled(); + await userEvent.dblClick(separator); + expect(vm.onDoubleClick).toHaveBeenCalledOnce(); }); }); diff --git a/packages/shared-components/src/resize/separator/SeparatorView.tsx b/packages/shared-components/src/resize/separator/SeparatorView.tsx index 2d95f1940e..6f3169d1eb 100644 --- a/packages/shared-components/src/resize/separator/SeparatorView.tsx +++ b/packages/shared-components/src/resize/separator/SeparatorView.tsx @@ -33,14 +33,9 @@ export interface SeparatorViewActions { onPointerDown: () => void; /** - * onFocus handler for the separator. + * onDoubleClick handler for the separator. */ - onFocus: () => void; - - /** - * onBlur handler for the separator. - */ - onBlur: () => void; + onDoubleClick: () => void; } interface Props { @@ -53,14 +48,14 @@ interface Props { */ export function SeparatorView({ vm, className }: Props): React.ReactNode { const { translate: _t } = useI18n(); - const { isCollapsed, isFocusedViaKeyboard } = useViewModel(vm); + const { isCollapsed } = useViewModel(vm); /** * 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. + * - border: This is just a thin separator; shown when the panel is expanded. */ - const type = isCollapsed || isFocusedViaKeyboard ? "bar" : "border"; + const type = isCollapsed ? "bar" : "border"; const barContent = ( @@ -75,19 +70,29 @@ export function SeparatorView({ vm, className }: Props): React.ReactNode { ); + /** + * This border is: + * - a 1px border that separates the left panel and main content. + * - a 2px border when the panel is expanded and the user is interacting with the separator. + */ + const border = ( +
+
+
+ ); + return ( - {type === "bar" ? barContent : null} + {type === "bar" ? barContent : border} ); } diff --git a/packages/shared-components/src/resize/separator/__snapshots__/SeparatorView.test.tsx.snap b/packages/shared-components/src/resize/separator/__snapshots__/SeparatorView.test.tsx.snap index 695dde68da..6ad257d117 100644 --- a/packages/shared-components/src/resize/separator/__snapshots__/SeparatorView.test.tsx.snap +++ b/packages/shared-components/src/resize/separator/__snapshots__/SeparatorView.test.tsx.snap @@ -80,7 +80,7 @@ exports[` > renders Default story 1`] = `
`; -exports[` > renders KeyboardFocused story 1`] = ` +exports[` > renders HoverWhenCollapsed story 1`] = `
> renders KeyboardFocused story 1`] = ` data-panel="true" data-testid="react-use-id-2" id="react-use-id-2" - style="min-height: 0px; max-height: 100%; height: auto; min-width: 0px; max-width: 100%; width: auto; border: 0px; padding: 0px; margin: 0px; display: flex; flex: 49.751 1 0px; overflow: visible;" + style="min-height: 0px; max-height: 100%; height: auto; min-width: 0px; max-width: 100%; width: auto; border: 0px; padding: 0px; margin: 0px; display: flex; flex: 0 1 0px; overflow: visible;" >
> renders KeyboardFocused story 1`] = ` aria-orientation="vertical" aria-valuemax="99.502" aria-valuemin="0" - aria-valuenow="49.751" + aria-valuenow="0" class="SeparatorView-module_separator Separator" data-separator="inactive" data-separator-type="bar" @@ -142,7 +142,160 @@ exports[` > renders KeyboardFocused story 1`] = ` data-panel="true" data-testid="react-use-id-4" id="react-use-id-4" - style="min-height: 0px; max-height: 100%; height: auto; min-width: 0px; max-width: 100%; width: auto; border: 0px; padding: 0px; margin: 0px; display: flex; flex: 50.249 1 0px; overflow: visible;" + style="min-height: 0px; max-height: 100%; height: auto; min-width: 0px; max-width: 100%; width: auto; border: 0px; padding: 0px; margin: 0px; display: flex; flex: 100 1 0px; overflow: visible;" + > +
+
+ MAIN CONTENT +
+
+
+
+
+
+`; + +exports[` > renders HoverWhenExpanded story 1`] = ` +
+
+
+
+
+
+
+ LEFT CONTENT +
+
+
+ +
+
+`; + +exports[` > renders KeyboardFocused story 1`] = ` +
+
+
+
+
+
+ LEFT CONTENT +
+
+
+