Repository navigation
UI Consistency: 3 issues found
Three consistency/regression findings were posted as inline review comments. Findings from earlier runs (sort trigger rebuilding the outline Button, in-menu search input height and key handling, meta-line separator guard, and the project test coverage) are resolved in this revision.
apps/web/src/components/pullRequest/PullRequestListFilters.tsx(Filters trigger): the leftoveraria-label="Filter pull requests"overrides the newly visible "Filters" text and the active-filter count, so assistive tech never hears the count and voice control no longer matches the visible label (WCAG 2.5.3).relativeon the trigger is also redundant now that the absolutely positioned dot is gone.apps/web/src/components/pullRequest/PullRequestListFilters.tsx(label submenu):Clear selected labelsis aMenuItem, which closes the whole menu on click, while theMenuCheckboxItemrows beside it keep it open — two interaction models in one submenu.closeOnClick={false}matches the neighbours.apps/web/src/components/pullRequest/PullRequestListFilters.tsx(project submenu): moving projects onto the shared radio-group option list drops the always-visible "Unavailable" pill, leaving only a dimmed row and a hover-only tooltip that pointer-coarse users cannot reach. Rendering the marker once inPullRequestFilterRadioGrouprestores it for projects, hosts and servers alike.
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Scope reviewed: apps/web/src/components/pullRequest/PullRequestListFilters.tsx, PullRequestRow.tsx, pullRequestList.logic.ts, PullRequestListFilters.test.tsx, and apps/web/src/routes/_chat.pull-requests.tsx, cross-checked against the shared primitives in apps/web/src/components/ui (button.tsx, menu.tsx, input-group.tsx, badge.tsx) and existing call-site conventions (PreviewMoreMenu.tsx for closeOnClick, ProjectFavicon.tsx for icon defaults).
Checked and found acceptable:
CompactFilterMenu'soutlinedmode now renders throughButton variant="outline"viarender, so focus ring, radius, dark surface and coarse-pointer hit target come from the primitive.- The in-menu author search uses
InputGroup+InputGroupInput size="compact"(control sized, not the wrapper) and only shields printable keys from the menu typeahead, leaving arrow/Home/End navigation to the composite. MenuSubPopup className="w-80"/"w-72"correctly suppressMenuPopup'smin-w-32default via the existing width-utility contract.- Row label chip and the
changedLinesblock are guarded at the call site, soPullRequestMetaLinestill draws no stray separators;displayGroupsguards the empty group label before rendering the heading. labelDotColoris now duplicated only betweenPullRequestSummaryTab.tsxand this file (the row copy was removed); already raised in prior runs and not re-reported.
🔍 Agent activity — 20 tool calls
💭 I'm checking the current PR state against previously flagged issues. The outline variant rebuild in CompactFilterMenu looks resolved now, and the onKeyDown handler appears to have been updated to only stop propagation for single-character keys, which should let navigation keys through as intended. The InputGroup height issue seems fixed with size="compact" replacing the hardcoded height. The labelDotColor duplication is reduced but still exists in two places rather than fully consolidated—PullRequestRow.tsx no longer uses it, but it remains in the filters and summary tab. The test recursion issue and the PullRequestMetaLine null child guard both look addressed. With most prior findings resolved, I should now go through the actual files in the current head to check for any new issues.
1. browse_code — apps/web/src/components/pullRequest/PullRequestListFilters.tsx
2. browse_code — apps/web/src/components/pullRequest/PullRequestRow.tsx
💭 I'm now checking pullRequestPresentation for existing chip components and menu primitives, and revisiting prior findings. The labelDotColor duplication between PullRequestSummaryTab.tsx and PullRequestListFilters.tsx still stands—down to just two copies now. Since this duplication was already flagged twice before, I won't repeat it. Instead I'm comparing chip styling patterns between the row and new filter components (slight aesthetic differences, likely not worth flagging), then moving on to inspect ui/menu.tsx for MenuSubPopup's width-defaulting behavior and checking PullRequestActorAvatar sizing within MenuRadioItem, plus the sort menu and routing logic.
3. browse_code — apps/web/src/components/ui/menu.tsx
4. browse_code — apps/web/src/components/pullRequest/pullRequestPresentation.tsx
💭 I'm checking the routes file for how CompactFilterMenu and sortMenu handle the outlined variant—when outlined is true, the Button's own classes take over since className stays undefined, and I want to verify how the trigger label and icon are rendered together with the outline Button. I notice a subtle accessibility concern: the "Filters" trigger now shows visible text but keeps an aria-label of "Filter pull requests," which technically violates WCAG's Label in Name guidance since "Filters" (plural) isn't a substring match of the accessible name. This is a minor leftover from when the button was icon-only, and the now-unused
relativeclass (previously for an absolute dot indicator) is also dead code worth flagging. I want to double check the Button component's variants to confirm size defaults and whether the icon color styling still applies correctly.
5. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
6. browse_code — apps/web/src/components/ui/button.tsx
💭 Checking the CompactFilterMenu's outline button variant, I confirm the chevron icon keeps its own muted color independent of the button's control-icon-color styling, so that part looks fine. The non-outlined path still uses a raw MenuTrigger with hand-rolled classes, but that's pre-existing rather than newly introduced. Looking at the sort menu, the aria-label "Sort pull requests" doesn't contain the visible text "Sort: Recently updated," but this label-in-name mismatch already existed before my changes, so it's not something I introduced. Similarly, the Filters trigger has a pre-existing aria-label alongside new visible "Filters" text, and the
relativeclass is now vestigial since I replaced the dot indicator with an in-flow count badge — minor but not blocking. Checking SORT_OPTIONS, reusing Rows3Icon for both largest and smallest is just an aesthetic quirk. In PullRequestRow, the diff stat logic looks consistent — it only renders when additions or deletions are nonzero, though the combined "N lines +X -Y" display feels slightly redundant aesthetically. The fixed row height via contain-intrinsic-block-size isn't affected by adding a labels chip to the meta line. For PullRequestRowLabels, I'm checking whether the max-w-40/min-w-0 wrapper needs a shrink-0 modifier — other meta segments use it for fixed-width elements, but here allowing shrink-to-zero with truncate might actually be the intended behavior. I'm also confirming that truncate still works correctly on the inner span despite it being nested inside a flex item, since overflow:hidden forces the flex item's automatic minimum width to zero, letting the ellipsis truncation kick in as expected. One remaining gap I'm noting: the label chip has no aria-label or tooltip for accessibility, just plain visible text. Moving on, I'm checking the author submenu — the avatar component inside MenuRadioItem is a span/img rather than an svg, so the size and margin utility selectors targeting svg elements don't apply to it, which is fine. The "Anyone" radio item using an empty string value also looks correct for Base UI's radio pattern, and the submenu popup widths are set explicitly so the width-detection logic properly suppresses the default minimum width. Now I'm turning attention to the search input component to check its behavior next. ArrowLeft closing the submenu would conflict with caret navigation in the input, but since the prior comment explicitly asked not to block those composite keys, re-flagging Home/End contradicts that request, so I'll skip it. Next I want to check the PullRequestLabelFilter clear item — the MenuItem's onClick closes the menu on click, so I should verify that's the intended behavior for a "clear selected labels" action. I want to verify whether Base UI supports closeOnClick on Item in this version, so I should grep the repo for that prop. I'm also considering other minor issues: disabled MenuItems used for static empty-state text like "No authors found," duplicated group labels in submenus, and checking how PullRequestActorAvatar handles environment routing rules. Now I'm looking at where sortMenu is rendered in the column header relative to other controls, checking the render area around that section to see how the outlined Button fits within the header row.
7. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
8. git_tools — closeOnClick
💭 I'm noting that Base UI's Menu.RadioItem defaults closeOnClick to false while Item defaults to true, and TraitsPicker explicitly sets closeOnClick true on its radio items—so since PullRequestFilterRadioGroup's MenuRadioItem doesn't set this prop, it'll default to false and keep the menu open after selection. The
Clear selected labelsMenuItem defaults to closing the whole menu while everything else in that submenu keeps it open, which is a real but minor inconsistency worth flagging with a suggestedcloseOnClick={false}fix. Now I'm looking at whether typing in the author filter's search input inside a Menu submenu could break due to Base UI's composite handling of keyboard events. I'm also considering whether the sub-popup width (w-80) creates overflow issues relative to its parent menu, but that seems like a minor collision case Base UI likely handles fine. Looking closer at the icon rendering inPullRequestFilterRadioSubmenu, I notice the sharedIconcomponent now drops theshrink-0class it previously had, and there's a type mismatch since the icon component only acceptsclassNamebut is being passedaria-hiddenas well — worth checking if that causes a TypeScript error. Since JSX resolves against the union type and intrinsic elements accept aria-hidden, this should compile fine. I'm also checking whether ProjectFavicon loses its shrink-0 styling in the new flex trigger layout, and noting that the previous "Unavailable" amber badge for disabled projects seems to have been dropped in favor of just a tooltip in the new PullRequestFilterRadioGroup component. On touch devices tooltips aren't reachable, so this actually removes meaningful information the original comment specifically called out—unavailability was meant to be shown right where users pick between projects, not just via dimmed opacity. I want to flag this as a genuine regression worth fixing by restoring the badge in the rendering logic. Checking the old code confirms the amber "Unavailable" badge with tooltip existed for those items, and the new generic radio group only kept the disabled styling and tooltip, dropping the visible badge entirely. Since this is a concrete visual regression introduced by the change and matches the existing convention used elsewhere for unavailable host options, I'll flag it with moderate-to-good confidence. Actually, span truncation looks fine there. But I notice a real accessibility issue: the Filters trigger button hasaria-label="Filter pull requests"alongside visible "Filters" text and a count badge — since aria-label overrides content, the count gets silently dropped from the accessible name. I should suggest removing the aria-label since the visible text now serves that purpose, or otherwise incorporating the count into the announced label. Also worth a minor mention: therelativeclass added to the button is likely redundant since the base button variant already includesrelativepositioning. Now I'm noticing the count is rendered as a hand-rolled span rather than the repo's existingBadgecomponent, which is used elsewhere like in pull request presentation — worth checking whetherBadgehas a fitting compact/count variant that should be used instead.
9. browse_code — apps/web/src/components/ui/badge.tsx
10. git_tools — ProjectFavicon
💭 I'm noting ProjectFavicon already has its own default sizing, so that's fine. I'm also checking whether the hand-rolled count badge inside the Filters button should instead use the existing Badge primitive, and searching the repo for similar count-chip patterns to see if there's precedent for raw buttons versus core UI components. Since Badge isn't explicitly called out in the primitive-reconstruction rule, I'm treating that as lower priority and instead prioritizing the accessibility issue where the Filters trigger's aria-label suppresses the new visible label and count from the accessible name — that's a solid, concrete finding tied to the changed lines. I'm also weighing in the "Clear selected labels" MenuItem closing the whole menu instead of just toggling, the unavailable project badge disappearing, and a duplicated labelDotColor definition as other candidate findings to raise. Checking Base UI's defaults confirms Menu.Item closes on click by default while CheckboxItem and RadioItem don't, so the "Clear selected labels" behavior really is inconsistent with the per-label toggle behavior — worth flagging as a concrete interaction bug. I'm also noting the disabled MenuItem pattern used for empty states like "No authors found" to see if it matches existing conventions elsewhere in the repo, and considering whether capping the author list to 10 results without indicating more matches exist is worth mentioning, though it's more a minor gap than a real inconsistency. I'm also double-checking small padding math on the search input's wrapper divs, verifying whether InputGroupInput actually supports a
size="compact"prop by looking at its source, and confirming that wrapping a PullRequestActorAvatar inside MenuRadioItem's flex span doesn't cause layout issues.
11. browse_code — apps/web/src/components/ui/input-group.tsx
12. git_tools — MenuItem disabled
💭 The empty state pattern with disabled MenuItem is established, and Input supports size="compact" as referenced in prior code. Now I want to check PullRequestReviewerPicker as a comparable in-menu search pattern to see if the new author search follows the same MenuPopup and search conventions for consistency.
13. git_tools — InputGroupInput
💭 I'll search for in-menu search inputs across the app by looking for MenuPopup components near Input elements.
14. git_tools — size="compact"
15. git_tools — stopPropagation
💭 I'm comparing this to the PullRequestReviewerPicker pattern to check consistency, then narrowing down to the strongest findings. The first candidate is an accessibility issue: the Filters trigger still has an aria-label of "Filter pull requests" even though it now shows visible text "Filters" plus a count badge, and the aria-label overrides that visible content for screen readers. For the second finding, I'm checking whether "Clear selected labels" closes the whole menu since Base UI's default MenuItem behavior closes on click while checkbox items stay open by default — I want to verify this against the repo's existing usage patterns before proposing a
closeOnClick={false}fix. Moving on, findings three and four look fine. For finding five, I'm now digging into howdisplayGroupshandles the collapsed "others" group with an empty label — if the render logic still outputs a heading element for that empty label, it could leave a stray visual gap, so I need to check the actual rendering code around where groups get mapped to headers.
16. browse_code — apps/web/src/routes/_chat.pull-requests.tsx
💭 The empty label case looks handled correctly with a null fallback. I want to check where menuFiltered is defined to confirm it accounts for author and labels, and verify updateSearch correctly maps the sort parameter, which it seems to do fine. I'm also noting that sorting by size collapses all groups, which is another UI behavior worth checking.
17. git_tools — menuFiltered
💭 The filter count logic looks reasonable since each selected label counts separately alongside state, involvement, host, server, and project filters. Now I need to finalize the two findings and pin down exact line numbers in the diff so my inline comments land in the right spots. For the aria-label issue in PullRequestListFilters.tsx, I need to figure out whether line 507 falls within the diff hunk context, since GitHub only allows comments on lines present in the diff. It seems safer to anchor the comment on line 512, an added line, and reference the aria-label problem from there, possibly using a multi-line comment spanning the added lines if the hunk covers that range — I should pull the actual patch to confirm the hunk boundaries before committing to line numbers.
18. github_api_read_only — repos/{owner}/{repo}/pulls/{pull_number}/files
💭 I'm settling on the finding for PullRequestListFilters.tsx: the aria-label on the button around line 507-512 overrides the new visible label and count, which seems like the real issue worth flagging since it's a commentable context line within the diff hunk. The aria-label overriding the visible "Filters" text and count is a genuine accessibility regression—the count will never get announced and it violates WCAG 2.5.3 label matching. I'm planning to suggest removing the aria-label attribute entirely, comparing this against other buttons like PullRequestRefreshControl which is icon-only and legitimately needs one. Now I'm working out the cleanest way to post this as a multi-line suggestion covering the Button's props. Now I'm looking at another finding: in PullRequestListFilters.tsx, the "Clear selected labels" menu item closes the whole filter menu on click, unlike the neighboring checkbox rows, so I should suggest adding
closeOnClick={false}— but first I want to confirm this prop is supported by checking how PreviewMoreMenu.tsx uses it on a similar MenuItem. I'm also considering whether to flag the loss of the "Unavailable" badge in the project list now that it uses the same tooltip-only treatment as hosts/servers. I'll skip separately flagging that and instead fold it into one comment on the project submenu, along with noting that a deleted comment explaining an intentional sort ordering constraint should probably have been preserved even though the sort behavior itself didn't change. The fix I'll suggest is renderingoption.unavailableinsidePullRequestFilterRadioGroupso the badge stays available to everyone using that shared component, keeping ownership in one place. This seems within scope since it addresses the actual regression rather than demanding unrelated cleanup — the tooltip-only disabled state leaves touch users with no explanation. I'll frame this as a real regression worth flagging as a third comment, but first I want to double check howPreviewMoreMenu's closeOnClick is used elsewhere.
19. browse_code — apps/web/src/components/preview/PreviewMoreMenu.tsx
💭 I've confirmed MenuItem closes on click by default unless
closeOnClick={false}is set, and that clicking any item closes the entire menu tree including submenus in Base UI. Now I'm drafting a comment about the trigger button's accessibility issue—sincearia-labeloverrides the visible text and count, screen readers will announce the static label instead of the actual filter state. For the fix, I'm suggesting the label be removed so the button's contents name it naturally, and noting therelativeclass is now redundant sincebuttonVariantsalready handles positioning. Then I'm drafting a second comment about the "Clear selected labels" item closing the whole menu inconsistently with the checkbox items that stay open, suggestingcloseOnClick={false}as the fix. Confirming the line-width math checks out for that break. Now moving to the third comment, about the project submenu losing the amber "Unavailable" badge when routed through the shared radio group component — I need to anchor this on the diff lines around theunavailablespread in theprojectOptionsconstruction, since the actual rendering code is unchanged and out of the diff. I'm weighing whether this comment is worth including since it's a real accessibility gap for touch users who can't reach hover tooltips, even though it might read as a minor aesthetic nit — I'll keep it in, framed as a genuine information-loss issue rather than pure polish. Given there are multiple findings including an accessibility concern, and the guidance explicitly defaults to failure when issues exist rather than success, I'll mark the check as failure.
20. modify_pr — UI consistency review: three findings on the new filter/sort controls. The earlier notes on the sort trigger rebuilding the outline Button, the in-menu search input's height and key handling, and the meta-line separator guard all look addressed.
Agent Credits: 217 credits
Token usage: 28 uncached input · 1,244,824 cache-read input · 153,615 cache-write input · 19,144 output
Agent Credits may also include non-token charges from external tools such as web research.