From e8d20521db45d4cf60ee5164792962894a379ea4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 12 Aug 2026 22:46:58 +0000 Subject: [PATCH] fix(web): focus current time for picker arrow keys react-select opens the menu with reference equality, so a separately constructed value left keyboard focus on 12:00 AM. Resolve to the matching option (or insert custom times) so Up/Down move one interval. Co-authored-by: Tyler Dane --- .../TimePicker/TimePicker.test.tsx | 70 ++++++++++++++++--- .../DateTimeSection/TimePicker/TimePicker.tsx | 12 +++- .../resolveTimePickerSelection.test.ts | 47 +++++++++++++ .../TimePicker/resolveTimePickerSelection.ts | 44 ++++++++++++ 4 files changed, 161 insertions(+), 12 deletions(-) create mode 100644 packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.test.ts create mode 100644 packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.ts diff --git a/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.test.tsx b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.test.tsx index 83425bfa4e..21656d7786 100644 --- a/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.test.tsx +++ b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.test.tsx @@ -1,24 +1,27 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { useState } from "react"; import { type SelectOption } from "@web/common/types/component.types"; +import { getTimeOptions } from "@web/common/utils/datetime/web.date.util"; import { TimePicker } from "./TimePicker"; import { describe, expect, it } from "bun:test"; -const options: SelectOption[] = [ - { value: "13:00", label: "1 PM" }, - { value: "13:15", label: "1:15 PM" }, - { value: "13:30", label: "1:30 PM" }, -]; +const options = getTimeOptions(); +const fiveThirty = { label: "5:30 PM", value: "5:30 PM" }; -function Harness() { +function Harness({ + initialValue = options[0], +}: { + initialValue?: SelectOption; +}) { const [isMenuOpen, setIsMenuOpen] = useState(false); - const [value, setValue] = useState(options[0]); + const [value, setValue] = useState(initialValue); return (
{ + const activeId = combobox.getAttribute("aria-activedescendant"); + expect(activeId).toBeTruthy(); + const option = document.getElementById(activeId!); + expect(option).toBeTruthy(); + return option!; +}; + describe("TimePicker", () => { it("closes its menu when focus moves elsewhere in the form, instead of staying open forever", async () => { const user = userEvent.setup(); @@ -42,4 +53,45 @@ describe("TimePicker", () => { expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); }); + + it("focuses the current time on open so arrow keys move one interval", async () => { + const user = userEvent.setup(); + // Pass a separately constructed value object (same shape the form uses). + render(); + + const combobox = screen.getByRole("combobox", { name: "End time" }); + await user.click(combobox); + + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(focusedOptionName(combobox)).toHaveTextContent("5:30 PM"); + + await user.keyboard("{ArrowUp}"); + expect(focusedOptionName(combobox)).toHaveTextContent("5:15 PM"); + + await user.keyboard("{ArrowDown}"); + expect(focusedOptionName(combobox)).toHaveTextContent("5:30 PM"); + + await user.keyboard("{ArrowDown}"); + expect(focusedOptionName(combobox)).toHaveTextContent("5:45 PM"); + }); + + it("keeps a custom time selectable and arrow-navigable from nearby intervals", async () => { + const user = userEvent.setup(); + render(); + + const combobox = screen.getByRole("combobox", { name: "End time" }); + await user.click(combobox); + + const listbox = screen.getByRole("listbox"); + expect( + within(listbox).getByRole("option", { name: "5:33 PM" }), + ).toBeInTheDocument(); + expect(focusedOptionName(combobox)).toHaveTextContent("5:33 PM"); + + await user.keyboard("{ArrowUp}"); + expect(focusedOptionName(combobox)).toHaveTextContent("5:30 PM"); + + await user.keyboard("{ArrowDown}{ArrowDown}"); + expect(focusedOptionName(combobox)).toHaveTextContent("5:45 PM"); + }); }); diff --git a/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.tsx b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.tsx index e37424dbd8..7462d2e413 100644 --- a/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.tsx +++ b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePicker.tsx @@ -6,6 +6,7 @@ import { type SelectOption } from "@web/common/types/component.types"; import { type TimeOption } from "@web/common/types/util.types"; import { parseUserTime } from "@web/common/utils/datetime/web.date.util"; import { useFloatingLayer } from "@web/shortcuts/floating-layer"; +import { resolveTimePickerSelection } from "./resolveTimePickerSelection"; export interface Props extends Omit { isMenuOpen: boolean; @@ -48,6 +49,9 @@ export const TimePicker = ({ const layerId = useId(); useFloatingLayer(`timePicker:${layerId}`, isMenuOpen); + const { value: selectValue, options: selectOptions } = + resolveTimePickerSelection(value, options); + const cancelScrollToSelected = () => { if (scrollRafRef.current !== null) { cancelAnimationFrame(scrollRafRef.current); @@ -85,7 +89,7 @@ export const TimePicker = ({ className={selectClassName} classNamePrefix={TIMEPICKER} styles={timePickerTextStyles} - value={value} + value={selectValue} maxMenuHeight={4 * 41} blurInputOnSelect menuIsOpen={isMenuOpen} @@ -120,13 +124,15 @@ export const TimePicker = ({ setIsMenuOpen(false); }} openMenuOnFocus={true} - options={options} + options={selectOptions} tabSelectsValue={false} isValidNewOption={(inputValue) => { const parsed = parseUserTime(inputValue, value?.value); if (!parsed) return false; // Don't show create row if the parsed time is already in options - if (options?.some((o) => (o as TimeOption).value === parsed.value)) { + if ( + selectOptions?.some((o) => (o as TimeOption).value === parsed.value) + ) { return false; } return true; diff --git a/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.test.ts b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.test.ts new file mode 100644 index 0000000000..adfd1123b2 --- /dev/null +++ b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.test.ts @@ -0,0 +1,47 @@ +import { getTimeOptions } from "@web/common/utils/datetime/web.date.util"; +import { resolveTimePickerSelection } from "./resolveTimePickerSelection"; +import { describe, expect, it } from "bun:test"; + +describe("resolveTimePickerSelection", () => { + const options = getTimeOptions(); + + it("returns the option object from the list when values match", () => { + const constructed = { label: "5:30 PM", value: "5:30 PM" }; + const { value, options: nextOptions } = resolveTimePickerSelection( + constructed, + options, + ); + + expect(value).toBe(options.find((option) => option.value === "5:30 PM")); + expect(value).not.toBe(constructed); + expect(nextOptions).toBe(options); + }); + + it("inserts a custom time so react-select can focus it by reference", () => { + const custom = { label: "5:33 PM", value: "5:33 PM" }; + const { value, options: nextOptions } = resolveTimePickerSelection( + custom, + options, + ); + + expect(value).toBe(custom); + expect(nextOptions).not.toBe(options); + expect(nextOptions?.find((option) => option.value === "5:33 PM")).toBe( + custom, + ); + + const index = nextOptions!.findIndex( + (option) => option.value === "5:33 PM", + ); + expect(nextOptions![index - 1]?.value).toBe("5:30 PM"); + expect(nextOptions![index + 1]?.value).toBe("5:45 PM"); + }); + + it("passes through when options are missing", () => { + const value = { label: "5:30 PM", value: "5:30 PM" }; + expect(resolveTimePickerSelection(value, undefined)).toEqual({ + value, + options: undefined, + }); + }); +}); diff --git a/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.ts b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.ts new file mode 100644 index 0000000000..d937401ea8 --- /dev/null +++ b/packages/web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/resolveTimePickerSelection.ts @@ -0,0 +1,44 @@ +import { YMDHAM_FORMAT } from "@core/constants/date.constants"; +import dayjs from "@core/util/date/dayjs"; +import { type SelectOption } from "@web/common/types/component.types"; +import { type TimeOption } from "@web/common/types/util.types"; + +const timeValueToMinutes = (timeValue: string): number => { + const parsed = dayjs(`2000-01-01 ${timeValue}`, YMDHAM_FORMAT); + return parsed.hour() * 60 + parsed.minute(); +}; + +/** + * react-select's openMenu focuses the selected option via reference equality + * (`options.indexOf(value)`). Compass often passes a separately constructed + * value object, so resolve to the matching option (or insert a custom time) + * before handing props to CreatableSelect. + */ +export const resolveTimePickerSelection = ( + value: SelectOption, + options: TimeOption[] | undefined, +): { value: SelectOption; options: TimeOption[] | undefined } => { + if (!options?.length) { + return { value, options }; + } + + const exactMatch = options.find((option) => option.value === value.value); + if (exactMatch) { + return { value: exactMatch, options }; + } + + const valueMinutes = timeValueToMinutes(value.value); + const insertAt = options.findIndex( + (option) => timeValueToMinutes(option.value) > valueMinutes, + ); + const nextOptions = + insertAt === -1 + ? [...options, value as TimeOption] + : [ + ...options.slice(0, insertAt), + value as TimeOption, + ...options.slice(insertAt), + ]; + + return { value, options: nextOptions }; +};