GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix(site/src): expose the date range picker trigger as a combobox by jeremyruppel · Pull Request #31940 · coder/coder

Conversation

jeremyruppel commented Nov 9, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

The DateTimeRangePicker trigger acts like a select, but screen readers announced it as a plain button named after its current value, with no select role or state. It now follows the APG select-only combobox pattern, with the popover as a dialog:

The trigger has role="combobox" and a stable name: the label prop, or "Time range". The visible text ("Last 24 hours") is announced as the combobox value. aria-expanded reflects the open state, and aria-controls points at the popover while it is open.
ArrowDown and ArrowUp open the picker, and focus lands on the checked quick range. The quick ranges are still a radiogroup, so the selected state and the "n of m" position come from aria-checked on each radio.
The popover dialog shares the trigger's name. The radiogroup is renamed to "Quick ranges" so the two names don't repeat.

Every caller gets the fix: AI Gateway sessions, AI spend, and Users "Last seen". Tests and stories that found the trigger as a button by its value text now find the combobox by name.

Closes DEQ-184

Ticket	Surface	Visible change
DEQ-184	Date range picker trigger and quick-range list (AI Gateway sessions; also AI spend and Users "Last seen")	None. Role, name, and keyboard only, so no screenshots.

Accessibility self-review (axe-core 4.11.1, run against the Closed, OpenShowsOnlyQuickPicks, CustomRangeExpanded, and ListSessionsFilter/Default stories): no violations. The closed state has no incompletes. While open, the only incomplete, aria-valid-attr-value, is axe saying it can't confirm the aria-controls target exists. I checked by hand that the target exists.

Review findings not acted on:

TS1 and TS5 nits: existing play assertions in DateTimeRangePicker.stories.tsx (ClosedWithCustomRange, IntraDayTriggerLabel, SelectQuickPick). This PR only changed their queries. Moving them is out of scope.
color-contrast incomplete in CustomRangeExpanded: this is on main too, in the calendar, which this PR doesn't touch.

Grouping and decision log

One ticket, one root cause: the shared DateTimeRangePicker trigger. 26 non-test lines, all under site/.
Options considered: (A) a combobox trigger that opens a dialog, keeping the radiogroup; (B) A, plus converting the radios to listbox/option; (C) closing the ticket as already fixed by fix(site/src): make keyboard-inaccessible controls operable by keyboard #31656, which moved focus onto the checked radio. A was chosen. Radios already expose the selected state and position, and the popover also holds a calendar and time inputs, which a listbox can't contain. C was rejected because the trigger still had no select role or state.
A combobox takes its name from the author, not its contents, so the name is always aria-label. The old ${label}: ${value} form tied the name to the value. Now the name stays fixed and the value is the text.
aria-expanded and aria-controls are set explicitly because oxlint requires them on role="combobox". Like Radix, aria-controls is set only while the popover is open, so it never points at a missing id.
Round 1 review (@chrifro): merged main to resolve the UsersPage.test.tsx conflict, defaulted label in the destructuring, made aria-controls conditional, named the trigger key handler, dropped the aria-expanded test checks, added an ArrowUp test, and trimmed a stale story comment.
ArrowDown and ArrowUp open the picker, per the APG select-only combobox. Enter and Space still open it through Radix.
frontend-review FE1 to FE10: all pass. frontend-deep-review, 8 roles: no blocking or should-fix findings. Three roles noted that the dialog and the radiogroup shared a name, which is fixed. JSDoc was added on label, and the trigger comment was shortened.

🤖 Generated by Coder Agents on behalf of @jeremyruppel.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeremyruppel added 2 commits

November 9, 2026 21:20


          fix(site/src): expose the date range picker trigger as a combobox

fd06478

Refs DEQ-184


          fix(site/src): give the quick range group a distinct name

5e00ab9

jeremyruppel temporarily deployed to main

November 9, 2026 21:30

— with

GitHub Actions Inactive

jeremyruppel temporarily deployed to main

November 9, 2026 21:30

— with

GitHub Actions Inactive

linear-code Bot commented Nov 9, 2026

Copy link

Copy Markdown

DEQ-184

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot assigned jeremyruppel

Nov 9, 2026

jeremyruppel requested review from aqandrew, chrifro and tracyjohnsonux

November 9, 2026 21:37

jeremyruppel marked this pull request as ready for review

November 9, 2026 21:41

jeremyruppel temporarily deployed to main

November 9, 2026 21:41

— with

GitHub Actions Inactive

chrifro reviewed

Nov 10, 2026

View reviewed changes

chrifro left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combobox approach is sound. I checked it in Storybook: the trigger is announced as combobox "Time range" [collapsed]: Last 24 hours, ArrowDown, ArrowUp, Enter, and Space open it, focus lands on the checked radio, and the dialog and radiogroup names no longer collide. Keeping the radiogroup instead of a listbox is the right call, since the popover also holds the calendar.

This PR can't merge yet: it conflicts with main in UsersPage.test.tsx (blocking), and there is 1 should-fix plus 4 nits across 6 inline comments. Bounced as round 1 on DEQ-184. No follow-up tickets.

Dropped after cross-check: removing the explicit aria-expanded/aria-controls as duplicates of Radix. oxlint jsx-a11y/role-has-required-aria-props fails without them, as the PR body says.

🤖 Generated by Coder Agents on behalf of @chrifro.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

site/src/pages/UsersPage/UsersPage.test.tsx


		await user.click(await screen.findByRole("button", { name: "Last seen" }));
		await user.click(
		await screen.findByRole("combobox", { name: "Last seen" }),

chrifro Nov 10, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking This file conflicts with main, so GitHub can't merge the PR (merge gate)

#31943 (8fca118991) changed the last assertion in this test to await screen.findByRole("button", { name: /Over 30 days/ }). Your branch rewrites the same lines. Please rebase on main, keep your combobox query and the async wait, and re-run CI.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeremyruppel Nov 10, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged origin/main into the branch (no rebase, since this is a single PR). Kept the combobox query and the async waitFor on its text content. Fixed in 53a014d.

🤖 Generated by Coder Agents on behalf of @jeremyruppel.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

site/src/components/DateTimeRangePicker/DateTimeRangePicker.tsx Outdated

		activePreset?.label ??
		formatCustomLabel(value.start, value.end);

		const accessibleName = label ?? "Time range";

chrifro Nov 10, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should-fix accessibleName is just label with a default (Simplicity SD3)

The same destructuring already defaults props inline (size = "sm"), and label has no other use now. Could this be label = "Time range" in the destructuring, with aria-label={label} on the trigger and on PopoverContent? The new JSDoc already documents the default.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeremyruppel Nov 10, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: label = "Time range" in the destructuring, and aria-label={label} on the trigger and on PopoverContent. Fixed in 53a014d.

🤖 Generated by Coder Agents on behalf of @jeremyruppel.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

site/src/components/DateTimeRangePicker/DateTimeRangePicker.tsx Outdated

		role="combobox"
		aria-label={accessibleName}
		aria-expanded={open}
		aria-controls={contentId}

chrifro Nov 10, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit aria-controls points at an id that isn't in the DOM while the popover is closed (Reuse RO1, Styling SA6, Simplicity SD5)

I checked in Storybook: after Escape, aria-controls is still set and document.getElementById(...) returns null. That's the aria-valid-attr-value incomplete you mention. Radix sets it only while open. aria-controls={open ? contentId : undefined} keeps oxlint passing (I checked) and matches what Radix does.

While you're here: could the inline onKeyDown become a named handleTriggerKeyDown next to handleQuickPickKeyDown? (React RP6)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeremyruppel Nov 10, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: aria-controls={open ? contentId : undefined}, and the handler is now handleTriggerKeyDown, next to handleQuickPickKeyDown. axe no longer marks the closed story as incomplete. Fixed in 53a014d.

🤖 Generated by Coder Agents on behalf of @jeremyruppel.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

site/src/components/DateTimeRangePicker/DateTimeRangePicker.test.tsx Outdated

		await user.tab();
		await user.keyboard("{Enter}");
		const trigger = screen.getByRole("combobox", { name: "Time range" });
		expect(trigger).toHaveAttribute("aria-expanded", "false");

chrifro Nov 10, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit These attribute assertions technically fall under FE1's attribute rule (Tests TS5)

I'm not blocking on this: the ARIA state is the fix itself, and Pixel can't capture it. Two optional tweaks: drop the aria-expanded checks (Radix sets that anyway) and keep the aria-controls to dialog id link. ArrowUp, which this PR also adds, has no test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeremyruppel Nov 10, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped the aria-expanded checks, kept the aria-controls-to-dialog link, and added an "opens with ArrowUp" test. Fixed in 53a014d.

🤖 Generated by Coder Agents on behalf of @jeremyruppel.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

site/src/pages/AIBridgePage/ListSessionsPage/ListSessionsFilter.stories.tsx Outdated

		@@ -61,9 +61,7 @@ export const Default: Story = {

		// The picker trigger renders the committed preset label and opens

chrifro Nov 10, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit This comment says the trigger "renders the committed preset label", but the query below now finds it by its fixed name (Comments CD4)

Trim it to what the play still does, or delete it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeremyruppel Nov 10, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimmed it to "The picker opens with the committed preset selected." Fixed in 53a014d.

🤖 Generated by Coder Agents on behalf of @jeremyruppel.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

chrifro marked this pull request as draft

November 10, 2026 08:34

jeremyruppel added 2 commits

November 10, 2026 14:14


          Merge remote-tracking branch 'origin/main' into a11y/select-missing-r…

760177a

…ole-state

# Conflicts:
#	site/src/pages/UsersPage/UsersPage.test.tsx


          fix(site/src): address date range picker combobox review

53a014d

jeremyruppel temporarily deployed to main

November 10, 2026 14:21

— with

GitHub Actions Inactive

jeremyruppel temporarily deployed to main

November 10, 2026 14:22

— with

GitHub Actions Inactive

jeremyruppel marked this pull request as ready for review

November 10, 2026 14:30

jeremyruppel temporarily deployed to main

November 10, 2026 14:30

— with

GitHub Actions Inactive

bartekgatzcoder approved these changes

Nov 10, 2026

View reviewed changes

bartekgatzcoder left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 53a014d. The date range picker trigger is now a select-only combobox: combobox "Time range" with the range as its value, ArrowDown/ArrowUp/Enter/Space open it with focus on the checked radio in "Quick ranges", and aria-controls points at the named dialog only while open (verified in Storybook on ListSessionsFilter and DateTimeRangePicker). Round 1 findings are addressed; I confirmed oxlint requires the explicit aria-expanded/aria-controls. Nits, not blocking: the existing play assertions in DateTimeRangePicker.stories.tsx and ListSessionsFilter.stories.tsx could move to Vitest later, and the comment at ListSessionsFilter.stories.tsx:62 restates the code. Follow-ups filed: none.

🤖 Generated by Coder Agents on behalf of @bartekgatzcoder.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bartekgatzcoder merged commit 51e2717 into main

Nov 10, 2026

37 checks passed

Uh oh!

There was an error while loading. Please reload this page.

bartekgatzcoder deleted the a11y/select-missing-role-state branch

November 10, 2026 16:29

bartekgatzcoder temporarily deployed to main

November 10, 2026 16:29

— with

GitHub Actions Inactive

github-actions Bot locked and limited conversation to collaborators

Nov 10, 2026

This branch was previously deployed

1 inactive deployment

main — 53a014d6 Deployed Nov 10, 2026 by bartekgatzcoder via cla #253551

Reviewers

chrifro

chrifro left review comments

bartekgatzcoder

bartekgatzcoder approved these changes

aqandrew

Awaiting requested review from aqandrew

tracyjohnsonux

Awaiting requested review from tracyjohnsonux

Uh oh!

There was an error while loading. Please reload this page.

Assignees

jeremyruppel

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

jeremyruppel commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear-code Bot commented Nov 9, 2026

Uh oh!

chrifro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bartekgatzcoder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeremyruppel commented Nov 9, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.