Skip to content

Commit 3771d24

Browse files
HirenGajjartyler-daneclaude
authored
fix(web): clean up now view shortcuts — labels, key sequences, and platform tooltip (#1807)
* fix(web): update shortcut labels, keys, and tooltip for now view - Remove Logout shortcut from day/week sidebar (accessible via cmd palette) - Rename 'Toggle sidebar' to 'Close sidebar' - Rename 'Toggle shortcuts' to 'Show shortcuts' - Change edit description shortcut from e to e d sequence - Change edit reminder shortcut from r to e r sequence - Move reminder shortcut from global to now view - Show platform-aware key (⌘ or Ctrl) on save tooltip - Remove unused isAuthenticated from getShortcuts and callers * fix(web): add appLocked guard to hotkey sequences and fix stale tests - Add `useAppHotkeySequence` wrapper that checks `document.body.dataset.appLocked` before firing, matching the guard already on `useAppHotkey` - Use `useAppHotkeySequence` for E D and E R in `useNowShortcuts` instead of raw `useHotkeySequence`, which bypassed the app-lock check - Add missing negative test: E alone must not trigger the E R reminder sequence - Fix `shortcuts.data.test.ts` assertions to match removed isAuthenticated param, removed z/r global shortcuts, renamed labels, and new e d / e r now shortcut keys Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(web): fix TaskDescription tooltip test for platform-aware shortcut rendering The tooltip shortcut was changed from the string "Mod+Enter" to ShortCutLabel components that render a platform-specific modifier icon (SVG) plus an Enter span. The old getByText("Mod+Enter") query found nothing on CI (Linux renders control-icon, not the text "Mod"). Update to query by the data-testid attributes that ShortCutLabel already provides, using getModifierKeyTestId() so the assertion works on both Mac and Linux without hard-coding a platform. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): let Escape blur description field instead of navigating away TanStack hotkeys defaults ignoreInputs:false for Escape, so the global Escape handler in useNowShortcuts was firing even when the description textarea had focus, causing navigation to the Day view instead of the expected a11y behavior (blur/revert the field). Pass ignoreInputs:true so the global handler is suppressed whenever any input-like element is focused. TaskDescription already owns its own onKeyDown Escape handler that reverts the value and exits editing mode; the global handler now only fires when no input has focus (idle Now view). Also: - Rename the existing Escape test to clarify it covers the no-input case - Add a test that dispatches Escape from a focused textarea (using the pressKey target argument) to verify the handler is suppressed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): switch Escape to keydown so ignoreInputs fires before textarea unmounts The previous keyup-based Escape handler had a race condition: 1. Escape keydown → TaskDescription.handleKeyDown → setIsEditing(false) queued 2. React flushes state between keydown and keyup → textarea unmounts → focus moves to body 3. Escape keyup fires with event.target = document.body (browser reassigns target when the focused element is removed) 4. isInputElement(body) = false → ignoreInputs:true check passes → handler runs → navigate Switching to keydown ensures TanStack fires during the same event cycle as the React onKeyDown handler, before any state flush. The textarea is still mounted and focused at that point, so event.target = textarea, isInputElement = true, and the handler is correctly suppressed. This is consistent with every other Escape handler in the codebase (useGlobalShortcuts, Reminder) which also use keydown. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: sync focus so Escape after E→D blurs description instead of navigating When the user pressed E→D then Escape quickly, the global Escape handler would navigate to /day instead of letting the textarea handle the key. Root cause: React schedules state updates via MessageChannel (a separate macrotask). The browser delivers D keyup and Escape keydown before that macrotask fires, so the textarea was never rendered or focused when Escape arrived — meaning ignoreInputs:true couldn't suppress the global handler. Fix: call flushSync when handling FOCUS_TASK_DESCRIPTION so React renders synchronously within the D keydown macrotask, and switch the focus call to useLayoutEffect so it runs inside the same synchronous flush. By the time D keydown returns, the textarea is in the DOM and focused, so any following Escape keydown has event.target = textarea and is correctly suppressed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(web): keep Escape inside now description editor * chore: remove unnecessary comment in TaskDescription test --------- Co-authored-by: Tyler Dane <tyler@switchback.tech> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 3f68ac0 commit 3771d24

12 files changed

Lines changed: 158 additions & 55 deletions

File tree

packages/web/src/common/hooks/useAppHotkey.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import {
22
type ConflictBehavior,
3+
type HotkeySequence,
34
type RegisterableHotkey,
5+
type UseHotkeySequenceOptions,
46
useHotkey,
7+
useHotkeySequence,
58
} from "@tanstack/react-hotkeys";
69

710
export interface UseAppHotkeyOptions {
@@ -53,3 +56,20 @@ export const useAppHotkeyUp = (
5356
handler: (event: KeyboardEvent) => void,
5457
options?: Omit<UseAppHotkeyOptions, "eventType">,
5558
) => useAppHotkey(hotkey, handler, { ...options, eventType: "keyup" });
59+
60+
export function useAppHotkeySequence(
61+
sequence: HotkeySequence,
62+
handler: () => void,
63+
options: UseHotkeySequenceOptions = {},
64+
) {
65+
useHotkeySequence(
66+
sequence,
67+
() => {
68+
if (document.body.dataset.appLocked === "true") {
69+
return;
70+
}
71+
handler();
72+
},
73+
options,
74+
);
75+
}

packages/web/src/common/utils/shortcut/data/shortcuts.data.test.ts

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@ describe("shortcuts.data", () => {
1010
{ k: "n", label: "Now" },
1111
{ k: "d", label: "Day" },
1212
{ k: "w", label: "Week" },
13-
{ k: "r", label: "Edit reminder" },
14-
{ k: "[", label: "Toggle sidebar" },
15-
{ k: "?", label: "Toggle shortcuts" },
16-
{ k: "z", label: "Log in" },
13+
{ k: "[", label: "Close sidebar" },
14+
{ k: "?", label: "Show shortcuts" },
1715
{ k: "Mod+k", label: "Command Palette" },
1816
]);
1917

@@ -34,12 +32,6 @@ describe("shortcuts.data", () => {
3432
});
3533
});
3634

37-
it("shows logout when authenticated", () => {
38-
const shortcuts = getShortcuts({ isAuthenticated: true });
39-
40-
expect(shortcuts.globalShortcuts[6]).toEqual({ k: "z", label: "Logout" });
41-
});
42-
4335
it("should show 'Scroll to now' when currentDate is today", () => {
4436
const shortcuts = getShortcuts({
4537
isToday: true,
@@ -121,21 +113,25 @@ describe("shortcuts.data", () => {
121113
k: "d",
122114
label: "Day",
123115
});
124-
expect(shortcuts.nowShortcuts).toHaveLength(5);
116+
expect(shortcuts.nowShortcuts).toHaveLength(6);
125117
expect(shortcuts.nowShortcuts[0]).toEqual({
126-
k: "e",
118+
k: "e d",
127119
label: "Edit description",
128120
});
129121
expect(shortcuts.nowShortcuts[1]).toEqual({
122+
k: "e r",
123+
label: "Edit reminder",
124+
});
125+
expect(shortcuts.nowShortcuts[2]).toEqual({
130126
k: "Mod+Enter",
131127
label: "Save description",
132128
});
133-
expect(shortcuts.nowShortcuts[2]).toEqual({
129+
expect(shortcuts.nowShortcuts[3]).toEqual({
134130
k: "j",
135131
label: "Previous task",
136132
});
137-
expect(shortcuts.nowShortcuts[3]).toEqual({ k: "k", label: "Next task" });
138-
expect(shortcuts.nowShortcuts[4]).toEqual({
133+
expect(shortcuts.nowShortcuts[4]).toEqual({ k: "k", label: "Next task" });
134+
expect(shortcuts.nowShortcuts[5]).toEqual({
139135
k: "Enter",
140136
label: "Mark complete",
141137
});

packages/web/src/common/utils/shortcut/data/shortcuts.data.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,20 @@ import { type Shortcut } from "@web/common/types/global.shortcut.types";
44

55
interface ShortcutsConfig {
66
isHome?: boolean;
7-
isAuthenticated?: boolean;
87
isToday?: boolean;
98
isNow?: boolean;
109
currentDate?: dayjs.Dayjs;
1110
}
1211

1312
export const getShortcuts = (config: ShortcutsConfig = {}) => {
14-
const {
15-
isAuthenticated = false,
16-
isHome = false,
17-
isToday = true,
18-
isNow = false,
19-
currentDate,
20-
} = config;
13+
const { isHome = false, isToday = true, isNow = false, currentDate } = config;
2114

2215
const globalShortcuts: Shortcut[] = [
2316
{ k: VIEW_SHORTCUTS.now.key, label: VIEW_SHORTCUTS.now.label },
2417
{ k: VIEW_SHORTCUTS.day.key, label: VIEW_SHORTCUTS.day.label },
2518
{ k: VIEW_SHORTCUTS.week.key, label: VIEW_SHORTCUTS.week.label },
26-
{ k: "r", label: "Edit reminder" },
27-
{ k: "[", label: "Toggle sidebar" },
28-
{ k: "?", label: "Toggle shortcuts" },
29-
{ k: "z", label: isAuthenticated ? "Logout" : "Log in" },
19+
{ k: "[", label: "Close sidebar" },
20+
{ k: "?", label: "Show shortcuts" },
3021
{ k: "Mod+k", label: "Command Palette" },
3122
];
3223

@@ -73,7 +64,8 @@ export const getShortcuts = (config: ShortcutsConfig = {}) => {
7364
}
7465
if (isNow) {
7566
nowShortcuts = [
76-
{ k: "e", label: "Edit description" },
67+
{ k: "e d", label: "Edit description" },
68+
{ k: "e r", label: "Edit reminder" },
7769
{ k: "Mod+Enter", label: "Save description" },
7870
{ k: "j", label: "Previous task" },
7971
{ k: "k", label: "Next task" },

packages/web/src/views/Day/view/DayViewContent.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { memo, useCallback, useMemo } from "react";
22
import dayjs from "@core/util/date/dayjs";
3-
import { useSession } from "@web/auth/compass/session/useSession";
43
import { useCompassRefs } from "@web/common/hooks/useCompassRefs";
54
import { useEventDNDActions } from "@web/common/hooks/useEventDNDActions";
65
import { useGridOrganization } from "@web/common/hooks/useGridOrganization";
@@ -38,7 +37,6 @@ import { Styled, StyledCalendar } from "@web/views/Week/styled";
3837
export const DayViewContent = memo(() => {
3938
const dispatch = useAppDispatch();
4039
const isSidebarOpen = useAppSelector(selectIsSidebarOpen);
41-
const { authenticated } = useSession();
4240

4341
const selectionActions = useMainGridSelectionActions();
4442
const { timedEventsGridRef } = useCompassRefs();
@@ -64,7 +62,6 @@ export const DayViewContent = memo(() => {
6462
const dateInView = useDateInView();
6563
const shortcuts = getShortcuts({
6664
currentDate: dateInView,
67-
isAuthenticated: authenticated,
6865
});
6966

7067
const {

packages/web/src/views/Now/components/TaskDescription/TaskDescription.test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
CompassDOMEvents,
99
compassEventEmitter,
1010
} from "@web/common/utils/dom/event-emitter.util";
11+
import { getModifierKeyTestId } from "@web/common/utils/shortcut/shortcut.util";
1112
import { TaskDescription } from "./TaskDescription";
1213

1314
function renderTaskDescription({
@@ -59,7 +60,8 @@ describe("TaskDescription", () => {
5960
await user.hover(saveButton);
6061

6162
await waitFor(() => {
62-
expect(screen.getByText("Mod+Enter")).toBeInTheDocument();
63+
expect(screen.getByTestId(getModifierKeyTestId())).toBeInTheDocument();
64+
expect(screen.getByTestId("enter-icon")).toBeInTheDocument();
6365
});
6466
});
6567

packages/web/src/views/Now/components/TaskDescription/TaskDescription.tsx

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
CompassDOMEvents,
77
compassEventEmitter,
88
} from "@web/common/utils/dom/event-emitter.util";
9+
import { ShortCutLabel } from "@web/common/utils/shortcut/shortcut.util";
910
import { Textarea } from "@web/components/Textarea";
1011
import { TooltipWrapper } from "@web/components/Tooltip/TooltipWrapper";
1112

@@ -140,6 +141,14 @@ const SaveButton = styled.button`
140141
}
141142
`;
142143

144+
const saveDescriptionShortcut = (
145+
<span className="inline-flex items-center gap-1">
146+
<ShortCutLabel k="Mod" size={12} />
147+
<span>+</span>
148+
<ShortCutLabel k="Enter" size={12} />
149+
</span>
150+
);
151+
143152
export const TaskDescription: React.FC<TaskDescriptionProps> = ({
144153
description = "",
145154
onSave,
@@ -199,22 +208,26 @@ export const TaskDescription: React.FC<TaskDescriptionProps> = ({
199208
};
200209
}, [isEditing, saveDescription]);
201210

202-
const handleClick = () => {
211+
const startEditing = () => {
203212
setIsEditing(true);
204213
};
205214

206-
const handleBlur = () => {
215+
const saveDescriptionOnBlur = () => {
207216
saveDescription();
208217
};
209218

210-
const handleChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
219+
const updateDraftDescription = (
220+
e: React.ChangeEvent<HTMLTextAreaElement>,
221+
) => {
211222
const newValue = e.target.value;
212223
if (newValue.length <= MAX_DESCRIPTION_LENGTH) {
213224
setValue(newValue);
214225
}
215226
};
216227

217-
const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
228+
const cancelEditingOnEscape = (
229+
e: React.KeyboardEvent<HTMLTextAreaElement>,
230+
) => {
218231
if (e.key === "Escape") {
219232
setValue(originalValueRef.current);
220233
setIsEditing(false);
@@ -230,9 +243,9 @@ export const TaskDescription: React.FC<TaskDescriptionProps> = ({
230243
<StyledDescription
231244
ref={textareaRef}
232245
value={value}
233-
onChange={handleChange}
234-
onBlur={handleBlur}
235-
onKeyDown={handleKeyDown}
246+
onChange={updateDraftDescription}
247+
onBlur={saveDescriptionOnBlur}
248+
onKeyDown={cancelEditingOnEscape}
236249
placeholder="Add a description..."
237250
maxLength={MAX_DESCRIPTION_LENGTH}
238251
id={TASK_DESCRIPTION_ID}
@@ -242,7 +255,10 @@ export const TaskDescription: React.FC<TaskDescriptionProps> = ({
242255
<CharacterCount isNearLimit={isNearLimit}>
243256
{value.length}/{MAX_DESCRIPTION_LENGTH}
244257
</CharacterCount>
245-
<TooltipWrapper description="Save description" shortcut="Mod+Enter">
258+
<TooltipWrapper
259+
description="Save description"
260+
shortcut={saveDescriptionShortcut}
261+
>
246262
<SaveButton
247263
aria-label="Save description"
248264
onClick={saveDescription}
@@ -256,7 +272,7 @@ export const TaskDescription: React.FC<TaskDescriptionProps> = ({
256272
</>
257273
) : (
258274
<DescriptionText
259-
onClick={handleClick}
275+
onClick={startEditing}
260276
className={value.length < 1 ? "empty" : ""}
261277
>
262278
{value.length < 1 ? "Add a description..." : value}

packages/web/src/views/Now/context/NowViewProvider.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { ensureStorageReady } from "@web/common/storage/adapter/adapter";
77
import { type Task } from "@web/common/types/task.types";
88
import { getDateKey } from "@web/common/utils/storage/storage.util";
99
import { getIncompleteTasksSorted } from "@web/common/utils/task/sort.task";
10+
import { viewSlice } from "@web/ducks/events/slices/view.slice";
11+
import { useAppDispatch } from "@web/store/store.hooks";
1012
import { useAvailableTasks } from "../hooks/useAvailableTasks";
1113
import { useFocusedTask } from "../hooks/useFocusedTask";
1214
import { useNowShortcuts } from "../shortcuts/useNowShortcuts";
@@ -35,6 +37,7 @@ export function NowViewProvider({
3537
children,
3638
onToggleSidebar,
3739
}: NowViewProviderProps) {
40+
const dispatch = useAppDispatch();
3841
const navigate = useNavigate();
3942
const { availableTasks, allTasks, hasCompletedTasks } = useAvailableTasks();
4043
const { focusedTask, setFocusedTask } = useFocusedTask({ availableTasks });
@@ -135,13 +138,23 @@ export function NowViewProvider({
135138
setFocusedTask,
136139
]);
137140

141+
const handleEscape = useCallback(() => {
142+
navigate(ROOT_ROUTES.DAY);
143+
}, [navigate]);
144+
145+
const handleEditReminder = useCallback(() => {
146+
dispatch(viewSlice.actions.updateReminder(true));
147+
}, [dispatch]);
148+
138149
useNowShortcuts({
139150
focusedTask,
140151
availableTasks,
141152
onPreviousTask: handlePreviousTask,
142153
onNextTask: handleNextTask,
143154
onCompleteTask: handleCompleteTask,
144155
onToggleSidebar,
156+
onEscape: handleEscape,
157+
onEditReminder: handleEditReminder,
145158
});
146159

147160
const value: NowViewContextValue = {

packages/web/src/views/Now/shortcuts/useNowShortcuts.test.tsx

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe("useNowShortcuts", () => {
3030
);
3131
});
3232

33-
it("uses E to focus the task description", async () => {
33+
it("uses E D sequence to focus the task description", async () => {
3434
const onFocusDescription = mock();
3535
compassEventEmitter.on(
3636
CompassDOMEvents.FOCUS_TASK_DESCRIPTION,
@@ -39,13 +39,14 @@ describe("useNowShortcuts", () => {
3939
renderHook(() => useNowShortcuts(), { wrapper });
4040

4141
pressKey("e");
42+
pressKey("d");
4243

4344
await waitFor(() => {
4445
expect(onFocusDescription).toHaveBeenCalledTimes(1);
4546
});
4647
});
4748

48-
it("does not use D for the task description shortcut", async () => {
49+
it("does not focus description when pressing D alone", async () => {
4950
const onFocusDescription = mock();
5051
compassEventEmitter.on(
5152
CompassDOMEvents.FOCUS_TASK_DESCRIPTION,
@@ -70,4 +71,58 @@ describe("useNowShortcuts", () => {
7071
expect(onToggleSidebar).toHaveBeenCalledTimes(1);
7172
});
7273
});
74+
75+
it("navigates to day view when Escape is pressed outside an input", async () => {
76+
const onEscape = mock();
77+
renderHook(() => useNowShortcuts({ onEscape }), { wrapper });
78+
79+
pressKey("Escape");
80+
81+
await waitFor(() => {
82+
expect(onEscape).toHaveBeenCalledTimes(1);
83+
});
84+
});
85+
86+
it("does not navigate when Escape is pressed inside a textarea", async () => {
87+
const onEscape = mock();
88+
renderHook(() => useNowShortcuts({ onEscape }), { wrapper });
89+
90+
const textarea = document.createElement("textarea");
91+
document.body.appendChild(textarea);
92+
93+
try {
94+
textarea.focus();
95+
96+
pressKey("Escape", {}, textarea);
97+
98+
await waitFor(() => {
99+
expect(onEscape).not.toHaveBeenCalled();
100+
});
101+
} finally {
102+
document.body.removeChild(textarea);
103+
}
104+
});
105+
106+
it("uses E R sequence to edit reminder", async () => {
107+
const onEditReminder = mock();
108+
renderHook(() => useNowShortcuts({ onEditReminder }), { wrapper });
109+
110+
pressKey("e");
111+
pressKey("r");
112+
113+
await waitFor(() => {
114+
expect(onEditReminder).toHaveBeenCalledTimes(1);
115+
});
116+
});
117+
118+
it("does not edit reminder when pressing E alone", async () => {
119+
const onEditReminder = mock();
120+
renderHook(() => useNowShortcuts({ onEditReminder }), { wrapper });
121+
122+
pressKey("e");
123+
124+
await waitFor(() => {
125+
expect(onEditReminder).not.toHaveBeenCalled();
126+
});
127+
});
73128
});

0 commit comments

Comments
 (0)