Skip to content

Commit cde7bb6

Browse files
tyler-daneclaude
andcommitted
fix: sync e2e shortcut hints and guard empty keycaps
- e2e/week-view-switch: assert uppercased view shortcut hints (d/w -> D/W, "Dayd"/"Weekw" -> "DayD"/"WeekW") to match the per-key chip rendering. - MenuItem: render the bare button (no empty tooltip surface) when tooltipContent is omitted, honoring the documented "tooltip disabled" contract. - ShortcutKeys: return null for combos with no keys instead of an empty chip row; add focused tests for per-key split, uppercasing, the cmd alias, and the empty-combo guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c2ff868 commit cde7bb6

4 files changed

Lines changed: 61 additions & 19 deletions

File tree

e2e/navigation/week-view-switch.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { expect, type Page, test } from "@playwright/test";
33
type ViewName = "day" | "week";
44

55
const shortcutByView = {
6-
day: "d",
7-
week: "w",
6+
day: "D",
7+
week: "W",
88
} as const satisfies Record<ViewName, string>;
99

1010
const collectUnexpectedConsoleErrors = (page: Page) => {
@@ -60,7 +60,7 @@ test.describe("View dropdown", () => {
6060

6161
await expect(
6262
page.getByTestId("view-select-dropdown").getByRole("option"),
63-
).toHaveText(["Dayd", "Weekw"]);
63+
).toHaveText(["DayD", "WeekW"]);
6464
await expect(viewOption(page, "day")).toHaveAttribute(
6565
"aria-selected",
6666
"false",
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { render, screen } from "@testing-library/react";
2+
import { ShortcutKeys } from "./ShortcutKeys";
3+
import { describe, expect, it } from "bun:test";
4+
5+
const keycaps = (container: HTMLElement) =>
6+
Array.from(container.querySelectorAll("[aria-hidden='true']"));
7+
8+
describe("ShortcutKeys", () => {
9+
it("renders one keycap chip per key and uppercases lone letters", () => {
10+
const { container } = render(<ShortcutKeys combo="Shift+w" />);
11+
12+
const chips = keycaps(container);
13+
expect(chips).toHaveLength(2);
14+
expect(chips[0]?.textContent).toBe("Shift");
15+
expect(chips[1]?.textContent).toBe("W");
16+
});
17+
18+
it("resolves the `cmd` alias to the Meta (Command) icon", () => {
19+
render(<ShortcutKeys combo="cmd+K" />);
20+
21+
// `cmd` -> Meta, which renders an icon rather than literal text.
22+
expect(screen.getByTestId("meta-icon")).toBeInTheDocument();
23+
expect(screen.getByText("K")).toBeInTheDocument();
24+
});
25+
26+
it("renders nothing when the combo has no keys", () => {
27+
const { container } = render(<ShortcutKeys combo="" />);
28+
expect(keycaps(container)).toHaveLength(0);
29+
30+
const { container: whitespace } = render(<ShortcutKeys combo=" + " />);
31+
expect(keycaps(whitespace)).toHaveLength(0);
32+
});
33+
});

packages/web/src/components/Shortcuts/ShortcutKeys.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export function ShortcutKeys({ combo, title, className }: Props) {
3838
.map((key) => key.trim())
3939
.filter(Boolean);
4040

41+
// Nothing to render -> emit nothing, rather than an empty chip row.
42+
if (keys.length === 0) return null;
43+
4144
return (
4245
<span
4346
title={title}

packages/web/src/views/Forms/ActionsMenu/MenuItem.tsx

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,35 @@ const MenuItem: React.FC<MenuItemProps> = ({
7474
onKeyDown: handleKeyDown,
7575
}) || {};
7676

77-
// With tooltip
77+
const button = (
78+
<button
79+
{...rest}
80+
{...itemProps}
81+
ref={itemRef}
82+
role="menuitem"
83+
tabIndex={tabIndex}
84+
type={type}
85+
className="flex w-full cursor-pointer items-center gap-2 border-0 bg-[var(--actions-menu-item-bg)] px-2 py-1 text-left text-m text-text-dark outline-none hover:[text-shadow:0_0_0.5px_var(--compass-color-text-dark),0_0_0.5px_var(--compass-color-text-dark)] focus-visible:[text-shadow:0_0_0.5px_var(--compass-color-text-dark),0_0_0.5px_var(--compass-color-text-dark)]"
86+
style={{ backgroundColor: bgColor }}
87+
>
88+
{children}
89+
</button>
90+
);
91+
92+
// No combo to show -> render the bare button (no empty tooltip surface).
93+
if (!tooltipContent) {
94+
return button;
95+
}
96+
7897
return (
7998
<Tooltip
8099
open={isTooltipOpen}
81100
onOpenChange={setIsTooltipOpen}
82101
placement="right-end"
83102
>
84-
<TooltipTrigger asChild>
85-
<button
86-
{...rest}
87-
{...itemProps}
88-
ref={itemRef}
89-
role="menuitem"
90-
tabIndex={tabIndex}
91-
type={type}
92-
className="flex w-full cursor-pointer items-center gap-2 border-0 bg-[var(--actions-menu-item-bg)] px-2 py-1 text-left text-m text-text-dark outline-none hover:[text-shadow:0_0_0.5px_var(--compass-color-text-dark),0_0_0.5px_var(--compass-color-text-dark)] focus-visible:[text-shadow:0_0_0.5px_var(--compass-color-text-dark),0_0_0.5px_var(--compass-color-text-dark)]"
93-
style={{ backgroundColor: bgColor }}
94-
>
95-
{children}
96-
</button>
97-
</TooltipTrigger>
103+
<TooltipTrigger asChild>{button}</TooltipTrigger>
98104
<TooltipContent>
99-
<ShortcutKeys combo={tooltipContent ?? ""} />
105+
<ShortcutKeys combo={tooltipContent} />
100106
</TooltipContent>
101107
</Tooltip>
102108
);

0 commit comments

Comments
 (0)