Skip to content

Commit e2b9e2c

Browse files
fix(web): restore focus after narrow sidebar dismiss
Address review findings: keep SelectView unclipped (no overflow-hidden on the header cluster), and use a distinct in-sidebar dismiss control that returns focus to the header open toggle after close. Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
1 parent 24e629a commit e2b9e2c

7 files changed

Lines changed: 107 additions & 38 deletions

File tree

packages/web/src/components/CalendarHeader/CalendarHeader.tsx

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,9 @@ import { type FC } from "react";
33
import { reloadLocation } from "@web/common/utils/browser/browser-navigation.util";
44
import { ArrowButton } from "@web/components/Button/ArrowButton";
55
import { SelectView } from "@web/components/SelectView/SelectView";
6-
import { useIsNarrowSidebarLayout } from "@web/components/Sidebar/hooks/useIsNarrowSidebarLayout";
76
import { useVersionCheck } from "@web/components/Sidebar/SidebarActions/useVersionCheck";
87
import { SidebarToggleButton } from "@web/components/Sidebar/SidebarToggleButton";
98
import { TooltipWrapper } from "@web/components/Tooltip/TooltipWrapper";
10-
import {
11-
selectIsSidebarOpen,
12-
useViewStore,
13-
} from "@web/events/stores/view.store";
149

1510
interface Props {
1611
/** Left-aligned heading text (e.g. "June 2026" or "Wednesday, July 1"). */
@@ -43,18 +38,13 @@ export const CalendarHeader: FC<Props> = ({
4338
showNavigation = true,
4439
}) => {
4540
const { isUpdateAvailable } = useVersionCheck();
46-
const isSidebarOpen = useViewStore(selectIsSidebarOpen);
47-
const isNarrowLayout = useIsNarrowSidebarLayout();
48-
// On narrow layouts the open sidebar hosts its own close control; keep a
49-
// single "Close sidebar" name in the accessibility tree.
50-
const showHeaderSidebarToggle = !isNarrowLayout || !isSidebarOpen;
5141

5242
return (
5343
<div className="flex h-12 w-full shrink-0 items-center gap-3 text-text-muted">
54-
{/* min-w-0 + overflow-hidden lets the title cluster yield space so the
55-
sidebar toggle (shrink-0) stays visible when the main column is
56-
squeezed by an open sidebar on a narrow viewport. */}
57-
<div className="flex min-w-0 flex-1 items-center gap-3 overflow-hidden">
44+
{/* min-w-0 lets the title cluster shrink so the sidebar toggle stays in
45+
layout. Avoid overflow-hidden here — SelectView's menu is absolutely
46+
positioned inside this cluster and must paint below the header. */}
47+
<div className="flex min-w-0 flex-1 items-center gap-3">
5848
{showNavigation && onPrev && onNext && (
5949
<>
6050
<TooltipWrapper shortcut="J">
@@ -90,11 +80,9 @@ export const CalendarHeader: FC<Props> = ({
9080
) : null}
9181
</div>
9282

93-
{showHeaderSidebarToggle ? (
94-
<div className="z-2 flex shrink-0 items-center pr-5">
95-
<SidebarToggleButton />
96-
</div>
97-
) : null}
83+
<div className="z-2 flex shrink-0 items-center pr-5">
84+
<SidebarToggleButton />
85+
</div>
9886
</div>
9987
);
10088
};
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { render, screen, waitFor } from "@testing-library/react";
2+
import userEvent from "@testing-library/user-event";
3+
import { createStoreWrapper } from "@web/__tests__/render-with-store";
4+
import {
5+
selectIsSidebarOpen,
6+
useViewStore,
7+
viewActions,
8+
} from "@web/events/stores/view.store";
9+
import { SidebarCloseButton } from "./SidebarCloseButton";
10+
import { SidebarToggleButton } from "./SidebarToggleButton";
11+
import { beforeEach, describe, expect, it } from "bun:test";
12+
13+
function Harness() {
14+
const isOpen = useViewStore(selectIsSidebarOpen);
15+
return (
16+
<div>
17+
<SidebarToggleButton />
18+
{isOpen ? <SidebarCloseButton /> : null}
19+
</div>
20+
);
21+
}
22+
23+
describe("SidebarCloseButton", () => {
24+
beforeEach(() => {
25+
viewActions.setSidebarOpen(true);
26+
});
27+
28+
it("closes the sidebar and focuses the header open control", async () => {
29+
const user = userEvent.setup();
30+
const { wrapper } = createStoreWrapper();
31+
32+
render(<Harness />, { wrapper });
33+
34+
await user.click(screen.getByRole("button", { name: "Dismiss sidebar" }));
35+
36+
expect(
37+
screen.queryByRole("button", { name: "Dismiss sidebar" }),
38+
).not.toBeInTheDocument();
39+
await waitFor(() => {
40+
expect(
41+
screen.getByRole("button", { name: "Open sidebar" }),
42+
).toHaveFocus();
43+
});
44+
});
45+
});
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { XIcon } from "@phosphor-icons/react";
2+
import { type FC } from "react";
3+
import { TooltipWrapper } from "@web/components/Tooltip/TooltipWrapper";
4+
import { viewActions } from "@web/events/stores/view.store";
5+
6+
/**
7+
* Narrow-layout dismiss control rendered inside the sidebar. Uses a distinct
8+
* accessible name from the header toggle so the two controls do not collide
9+
* while the panel is open, and restores focus to the header "Open sidebar"
10+
* control after close.
11+
*/
12+
export const SidebarCloseButton: FC = () => {
13+
return (
14+
<TooltipWrapper
15+
description="Close sidebar"
16+
onClick={() => {
17+
viewActions.setSidebarOpen(false);
18+
// The header toggle stays mounted and flips to "Open sidebar"; move
19+
// focus there after this in-sidebar control unmounts.
20+
window.setTimeout(() => {
21+
document
22+
.querySelector<HTMLButtonElement>('[aria-label="Open sidebar"]')
23+
?.focus();
24+
}, 0);
25+
}}
26+
shortcut="]"
27+
>
28+
<button
29+
type="button"
30+
aria-label="Dismiss sidebar"
31+
className="c-focus-ring flex h-6 w-6 cursor-pointer items-center justify-center text-text-muted"
32+
>
33+
<XIcon aria-hidden="true" size={16} />
34+
</button>
35+
</TooltipWrapper>
36+
);
37+
};

packages/web/src/components/Sidebar/SidebarShell.test.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,28 +55,28 @@ afterEach(() => {
5555
});
5656

5757
describe("SidebarShell", () => {
58-
it("keeps the close control out of the sidebar on wide layouts", () => {
58+
it("keeps the dismiss control out of the sidebar on wide layouts", () => {
5959
mockViewport(false);
6060
renderShell();
6161

6262
expect(
63-
screen.queryByRole("button", { name: "Close sidebar" }),
63+
screen.queryByRole("button", { name: "Dismiss sidebar" }),
6464
).not.toBeInTheDocument();
6565
expect(screen.getByText("Sidebar body")).toBeInTheDocument();
6666
});
6767

68-
it("shows a close control inside the sidebar on narrow layouts", async () => {
68+
it("shows a dismiss control inside the sidebar on narrow layouts", async () => {
6969
const user = userEvent.setup();
7070
mockViewport(true);
7171
renderShell();
7272

73-
const closeButton = screen.getByRole("button", { name: "Close sidebar" });
73+
const closeButton = screen.getByRole("button", { name: "Dismiss sidebar" });
7474
expect(closeButton).toBeInTheDocument();
7575

7676
await user.click(closeButton);
7777

7878
expect(
79-
screen.queryByRole("button", { name: "Close sidebar" }),
79+
screen.queryByRole("button", { name: "Dismiss sidebar" }),
8080
).not.toBeInTheDocument();
8181
});
8282
});

packages/web/src/components/Sidebar/SidebarShell.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
import { useIsNarrowSidebarLayout } from "./hooks/useIsNarrowSidebarLayout";
99
import { ShortcutsOverlay } from "./ShortcutsOverlay/ShortcutsOverlay";
1010
import { SidebarActions } from "./SidebarActions/SidebarActions";
11-
import { SidebarToggleButton } from "./SidebarToggleButton";
11+
import { SidebarCloseButton } from "./SidebarCloseButton";
1212

1313
interface SidebarShellProps extends HTMLAttributes<HTMLElement> {
1414
children: ReactNode;
@@ -34,8 +34,7 @@ export function SidebarShell({
3434
}: SidebarShellProps) {
3535
const isNarrowLayout = useIsNarrowSidebarLayout();
3636
const isSidebarOpen = useViewStore(selectIsSidebarOpen);
37-
// Only while open: during the collapse transition the shell stays mounted,
38-
// and a still-rendered toggle would duplicate the header's "Open sidebar".
37+
// Only while open: during the collapse transition the shell stays mounted.
3938
const showSidebarClose = isNarrowLayout && isSidebarOpen;
4039

4140
return (
@@ -47,7 +46,7 @@ export function SidebarShell({
4746
>
4847
{showSidebarClose ? (
4948
<div className="flex shrink-0 items-center justify-end px-5 pb-2">
50-
<SidebarToggleButton />
49+
<SidebarCloseButton />
5150
</div>
5251
) : null}
5352
{children}

packages/web/src/components/Sidebar/SidebarToggleButton.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import {
99
} from "@web/events/stores/view.store";
1010

1111
/**
12-
* Shared open/close control for the right sidebar. Used in the calendar
13-
* header and, on narrow viewports, inside the sidebar itself when the
14-
* header control is squeezed out of view.
12+
* Shared open/close control for the right sidebar. Lives in the calendar
13+
* header; on narrow viewports SidebarShell also hosts a close control so the
14+
* panel can be dismissed when this header control is hard to reach.
1515
*/
1616
export const SidebarToggleButton: FC = () => {
1717
const isSidebarOpen = useViewStore(selectIsSidebarOpen);

packages/web/src/views/Life/LifeView.test.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -390,20 +390,20 @@ describe("LifeView", () => {
390390
});
391391

392392
const inSidebarClose = screen.getByRole("button", {
393-
name: "Close sidebar",
393+
name: "Dismiss sidebar",
394394
});
395395
expect(sidebar.contains(inSidebarClose)).toBe(true);
396396

397397
await user.click(inSidebarClose);
398398

399-
// Collapse keeps the panel mounted until the width transition ends; the
400-
// header "Open sidebar" control returning is the observable close signal.
401399
expect(
402-
screen.queryByRole("button", { name: "Close sidebar" }),
400+
screen.queryByRole("button", { name: "Dismiss sidebar" }),
403401
).not.toBeInTheDocument();
404-
expect(
405-
screen.getByRole("button", { name: "Open sidebar" }),
406-
).toBeInTheDocument();
402+
await waitFor(() => {
403+
expect(
404+
screen.getByRole("button", { name: "Open sidebar" }),
405+
).toHaveFocus();
406+
});
407407
});
408408

409409
it("shows the privacy tooltip on the date of birth label", async () => {

0 commit comments

Comments
 (0)