Repository navigation
UI Consistency: 4 issues found
The title → shared Tooltip migration is largely consistent with the existing Tooltip / TooltipTrigger render={…} / TooltipPopup idiom, and aria-labels, disabled state and layout are preserved (the Tooltip root emits no DOM, so flex parents are unchanged). Four call sites changed interaction ownership and are flagged inline:
apps/web/src/components/DiffPanel.tsx— the tooltip trigger now wraps the entire base-refCombobox, so hovering/focusing the combobox opens the head→base tooltip, which paints over the combobox popup (z-[140]vsz-[130]).apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx— the condensed branch row trigger enclosesPullRequestBaseFreshnessWarning, an intentional hover popover holding a pressable Update control; hovering the icon now opens both, with the tooltip layered above the popover.apps/web/src/components/settings/ConnectionsSettings.tsx— the pairing-link trigger wraps the whole<p>, including the hover-openedAccessScopeSummarypopover; both open together and the tooltip covers the popover. Inline suggestion provided.apps/web/src/routes/_chat.pull-requests.tsx— the trigger is rendered onto aMenuRadioItemthat isdisabledfor exactly the tooltip-bearing options;data-disabled:pointer-events-none(and skipped keyboard highlight) makes the unavailability reason unreachable.
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.
Methodology:
- Compared each converted call site against the pre-existing usage of
apps/web/src/components/ui/tooltip.tsxat the merge base to confirm therender+ children composition,side/classNameconventions, and thatTooltip(Base UITooltip.Root) renders no DOM node, so wrapping does not disturb flex/grid geometry. - Verified stacking order of the shared primitives: tooltip positioner
z-[140]vs popover/menu/combobox/select popupsz-[130], which is what makes trigger-over-interactive-child compositions layer the tooltip above the child's own popup. - Checked primitive disabled contracts:
Buttonusesdisabled:pointer-events-noneandMenuRadioItemusesdata-disabled:pointer-events-none, which determines whether a converted tooltip can still open. - Confirmed removed
titleattributes that were not replaced are redundant rather than lost:ConnectionStatusDotalready renders aTooltipwith the same text, and the ChatView scroll pill has a visible label plusaria-label. - Reviewed the new oxlint rule and its tests for scope (intrinsic elements only,
iframe/embed/object/frame/mathexempted, custom components andsvg > titleallowed) — no findings there; that file is outside this check's include patterns and was used as context only.
Not flagged: tooltips on disabled Buttons (BrowserDeviceToolbar aspect-ratio lock, mini-player picture-in-picture) behave the same as the native title they replace and match existing repository precedent; conditionally absent TooltipPopup (DraftHeroHeadline) is harmless; class strings and element semantics were otherwise preserved.
🔍 Agent activity — 1 tool call
💭 Looking at the DiffPanel component, I'm noticing a structural issue where the TooltipTrigger is wrapping the entire Combobox, which could cause problems since interactive elements nested inside a tooltip trigger might interfere with each other — specifically, opening the combobox popup could conflict with the tooltip's behavior. In ChatView and ConnectionStatusDot, the title attributes were removed without any tooltip replacement, so I need to verify whether there's an aria-label or Tooltip component present to maintain accessibility. The _chat.pull-requests file has a more serious concern: wrapping MenuRadioItem in a TooltipTrigger inside a MenuRadioGroup could break the menu's keyboard navigation and item composition, since Base UI menus expect items as direct children for typeahead and highlighting to work properly. ThemeColorPicker's nested TooltipTrigger and PopoverTrigger should be fine, but I should confirm Base UI supports that composition. The resize handle in ThreadPreviewMiniPlayer is problematic too — wrapping it in a TooltipTrigger adds pointer event handlers that could conflict with the drag-based resize logic. Looking at the remaining issues: tooltips appearing on disabled buttons won't show since disabled elements don't fire pointer events, though this is a minor issue since title attributes also don't display on disabled buttons in most browsers. The sidebar draft row button uses pointer-events-none until hover, which is fine for tooltips. I also need to check cases like DraftHeroHeadline where TooltipTrigger wraps a MenuTrigger — that's another nested trigger situation. Let me examine the actual tooltip component implementation to understand how it handles these edge cases. ConnectionStatusDot already had a Tooltip built in, so the title attribute was unnecessary. Now I'm checking ChatView.tsx to see why the "Scroll to end" title was removed there. The label for ChatView is already visible as text, so I can safely remove the redundant title attribute. Now I need to examine DiffPanel more closely since that's where the real issues might be. I'm realizing there's a structural problem here: wrapping the entire Combobox inside a TooltipTrigger means the tooltip's hover handlers will fire when the user interacts with the combobox trigger, which could show the tooltip and obscure the dropdown at the same time. Plus, TooltipTrigger typically manages its own
tabIndex, which could conflict with the combobox's focus management. The better approach is to move the Tooltip wrapper to just the truncated text span (the head-ref), keeping the Combobox itself outside the trigger. This preserves the original interaction model while still showing the tooltip for overflow text. There's also the separate issue of the "Remote only" tooltip inside the combobox popup item — a tooltip on a non-interactive span within a listbox item. Now I'm checking how MenuRadioItem is being wrapped in TooltipTrigger in the pull requests file. I'm noticing an issue with how the Tooltip component from Base UI is being positioned within the MenuRadioGroup and MenuRadioItem hierarchy. Base UI's Menu system uses a composite list pattern that registers items based on their DOM order through refs and index mapping, while Floating UI's composite list registers items in mount order instead. This mismatch could be causing the Tooltip to interfere with the proper registration and ordering of menu items. There's also a concern about disabled menu items — Base UI usesaria-disabledanddata-disabledattributes rather than the native disabled attribute, so hover events should still fire on disabled items, but wrapping them in a Tooltip might still cause unexpected behavior. Additionally, the z-index layering between the tooltip popup (140) and the menu popup could create stacking issues, though that's probably manageable. The bigger inefficiency is that whenoption.unavailableis undefined, the code still wraps items with Tooltip and TooltipTrigger machinery unnecessarily, adding extra DOM attributes and trigger logic to every menu item. The real regression is that the nativetitleattribute provided a fallback tooltip for keyboard and screen reader users, whereas the new Tooltip component only appears on hover. Base UI's Tooltip.Trigger renders a button by default but doesn't addtabIndexto custom elements like spans or divs, making the tooltip inaccessible to keyboard users on non-focusable elements. That said, Base UI does applyaria-describedbywhen the tooltip opens, so the accessibility impact versus the original title attribute isn't necessarily worse. There's a more pressing issue with nested triggers: ThemeColorPicker nests TooltipTrigger around a PopoverTrigger, and Base UI supports this composition, but the tooltip stays open when the popover opens since Base UI doesn't auto-close tooltips on popover activation. I'm identifying the concrete problems: DiffPanel wraps the entire Combobox in a tooltip trigger, which fires while interacting with the dropdown and its popup—I should wrap only the head-ref span instead. ThreadPreview has similar issues to address. Now looking at MiniPlayer's resize handle: it has a TooltipTrigger with a render button that attaches pointer handlers for dragging, and Base UI's Trigger also adds its own pointer handlers that merge together, which is fine, but the tooltip appearing during a resize drag feels like a UX regression. There's also a pattern in ThreadPreviewMiniPlayer where TooltipTrigger receives both render and children props, and I'm trying to understand how Base UI handles that composition. Let me search the codebase for existing TooltipTrigger usage patterns to see if this render + children pattern is already being used. The render+children pattern is working consistently across conversions. Now I'm looking at the DiffPanel's TooltipTrigger wrapping the entire Combobox — that's an atypical composition since existing patterns only wrap leaf elements. There's also a nested Tooltip inside for the Remote option, which means hovering the inner one will keep the outer tooltip visible too, though the popup is portaled separately. Checking imports across all the modified files — DiffPanel, PullRequestSummaryTab, pullRequestPresentation, ChangedFilesTree, FilePreviewPanel, PullRequestCodeTab, and ConnectionsSettings all have Tooltip imported. Good. ThemeColorPicker follows the same pattern as Sidebar.tsx with Tooltip wrapping a PopoverTrigger that wraps a button, so that's consistent. ProjectContentSearchDialog's Toggle is wrapped fine. For DraftHeroHeadline, when activeProjectDisplayName is null, there's no TooltipPopup rendered — so the Tooltip.Root exists without a Popup child, which means hovering won't trigger anything. Now looking at ThreadPreviewMiniPlayer's PiP button — it's disabled, which suppresses pointer events entirely in browsers, so the tooltip won't show when disabled. The previous title attribute also wouldn't show on a disabled button, though I'm checking whether Chrome actually renders native titles for disabled elements. Actually, the codebase already has a precedent with PullRequestReviewerPicker wrapping a disabled Button in a TooltipTrigger, so this pattern is accepted practice here. I should check if Base UI's Button uses nativedisabledoraria-disabledto understand the behavior, but given the existing convention in the repo, this is likely already handled correctly. Looking at the other instances: BrowserDeviceToolbar has the same disabled pattern, the Sidebar draft row discard button usespointer-events-nonewith group-hover to control interactivity, and ServerUpdateProgress wraps a truncated span inside the TooltipTrigger's render prop. Now I'm checking for potential layout regressions — the Tooltip Root component shouldn't render any DOM itself, so wrapping elements shouldn't cause layout shifts. In DiffPanel and PullRequestDetailPanel, flex children like theml-autospan remain direct flex children since Tooltip.Root renders nothing. ChangedFilesTree moves the key prop to the Tooltip component itself, and the ComposerPreviewAnnotationCards test verifies the tooltip trigger count. The bigger concern is PullRequestChecksPopover, which nests a Tooltip inside a Popover — that should work fine. Now I'm checking CompactFilterMenu in the chat pull-requests file where Tooltip.Root sits between MenuRadioGroup and MenuRadioItem, and I need to verify how Base UI's Menu handles this. Base UI's CompositeList assigns indexes by DOM order using a provider that registers items during layout effects, then re-sorts them based on document position. The system supports arbitrary wrappers and grouping, so the Tooltip wrapper shouldn't break the composite list's ability to track menu items in order. For the tooltip behavior itself: Base UI's Menu items stay focusable even when disabled (usingaria-disabledrather than nativedisabled), and since MenuRadioItem renders as a div withrole="menuitemradio", hover events still trigger properly. The tooltip will show on disabled items as intended. But for available options without theunavailableflag, the Tooltip wraps them without displaying any popup. Now I need to check the ThemeColorPicker to see if TooltipPopup is correctly nested inside Popover after the PopoverTrigger, and also verify how FilePreviewPanel handles the breadcrumb title attribute. I should also look at ConnectionsSettings to see if PairingLinkListRow's TooltipTrigger wrapping a paragraph with AccessScopeSummary creates any issues, especially if AccessScopeSummary itself renders interactive content or has its own tooltip that could cause nested tooltip triggers. I'm noticing that wrapping the AccessScopeSummary button in a TooltipTrigger creates an overlap issue where hovering the "N scopes" button now triggers both the scopes popover and the outer tooltip for expiry time simultaneously, with both anchored to the top but positioned differently. This is similar to the problem that existed before with the native title attribute showing while hovering the child element. The collision risk is new though — the native title would follow the cursor and render on top, whereas now we have two styled popups that could genuinely overlap. I should consider a lighter-scope fix: wrapping only the expiry text instead of the whole paragraph, and applying the same approach to DiffPanel by wrapping just the head ref span. Let me check the PullRequestDetailPanel change around line 1491 where a TooltipTrigger wraps a span containing PullRequestBaseFreshnessWarning, which is likely an interactive element itself. The branch row now wraps the freshness warning popover trigger inside a TooltipTrigger, so hovering the amber icon opens both the popover with its "Update branch" action and the branch tooltip anchored to the outer span—they'll overlap at the top. The popover needs to exist because it contains a pressable control, though having the tooltip layer overlap it creates some complexity. The tooltip's z-index of 140 sits above the popover at 130, so when hovering over a branch row, the tooltip will visually obscure the freshness popover containing the "Update branch" action. Even though the tooltip is pointer-events-none and won't block clicks, this creates a visual regression where the popover content gets hidden. I'm seeing a similar issue emerging in the ConnectionsSettings PairingLinkListRow component. For DiffPanel, the combobox popup at z-130 sits beneath the tooltip at z-140, creating less overlap since the tooltip appears above while the popup opens below. The real problem is the tooltip displaying while the base-ref dropdown is actively being used. I'm narrowing down the specific findings: in DiffPanel around line 585-704, the TooltipTrigger wraps the entire Combobox control, so hovering or focusing the base-ref combobox now triggers the head→base tooltip above the popup. The fix would be to move the tooltip trigger to just the static head-ref text instead. And in PullRequestDetailPanel around line 1491, the TooltipTrigger wraps the span containing the freshness warning's popover, creating the same layering conflict. For ConnectionsSettings around line 695, the TooltipTrigger wraps a paragraph containing the AccessScopeSummary popover—same issue. These are defensible findings because while the old native title attribute would also show on ancestor hover, it appeared after a delay and disappeared on interaction, whereas the new styled tooltip appears immediately at the anchor's top and layers over the popover. The HTML validity check passes since buttons inside paragraphs are fine. I should verify if there are other candidates beyond these three. Looking at_chat.pull-requests.tsxand the CompactFilterMenu, there's a question about whether wrapping MenuRadioItem in a tooltip trigger breaks keyboard navigation, but that's speculative. The more concrete issue is checking whether Base UI's disabled menu items havepointer-events: noneapplied, which would prevent the tooltip from triggering anyway—I need to examine the MenuRadioItem styling in menu.tsx to confirm. I found the issue —MenuRadioItemappliespointer-events: nonewhen disabled, which prevents hover events from firing on those items. This means the tooltip that should display the unavailable reason never opens, so the migration loses the explanation that the nativetitleattribute used to provide. Actually, the nativetitlewouldn't have worked either sincepointer-events: noneprevents hit-testing entirely, so the browser would show the title of the underlying popup instead. This isn't a regression then, but the new tooltip implementation is more elaborate machinery that fails to achieve its stated goal of preserving the disabled reason. The tooltip becomes dead code as written — worth flagging with a suggestion to either wrap the disabled item in a hoverable container or render the reason inline. Now checking the disabled MenuRadioItem more carefully: Base UI menus skip disabled items in their roving focus pattern, so the item never gets focus, and since the TooltipTrigger renders directly onto the disabled element withpointer-events: none, the tooltip is unreachable by both pointer and keyboard. This is a solid finding. The Sidebar draft discard button works fine though — it haspointer-events: nonein the base class but becomes interactive on group hover, so the tooltip opens properly. I should also verify how native disabled buttons behave in BrowserDeviceToolbar and ThreadPreviewMiniPlayer. The aspect ratio lock button on the BrowserDeviceToolbar will be disabled when there's a pending operation or invalid input, which prevents the tooltip from opening due to pointer events being disabled. Since the tooltip is just informational and the button already had a title attribute before, losing the tooltip on disabled state isn't a regression and aligns with the existing behavior. The readPreviewMiniPlayer PiP button has the same pattern — it's disabled when there's no web content, and there's precedent in the codebase like PullRequestReviewerPicker where disabled buttons wrapped in tooltips don't show either. So this isn't a change-introduced regression. The disabled MenuRadioItem falls into the same category: it's not a regression from the diff, but it is new code that introduces a tooltip that can never open. The real issue is that the reason text becomes inaccessible when the item is disabled, and the fix would be to either render the reason through a hoverable wrapper or keep it visible some other way. Wrapping the content in a span won't work because pointer-events:none on the parent disables hit-testing for descendants too. The proper solution would be a wrapper element around the MenuRadioItem with pointer-events:auto, but that's non-trivial, so I'll just describe the problem and suggest wrapping the item in a hoverable element instead. So my final list of issues to report: the DiffPanel combobox wrapped in a tooltip trigger (interaction regression plus tooltip over popup), the PullRequestDetailPanel branch row wrapping the freshness hover-popover, the ConnectionsSettings pairing-link row wrapping the AccessScopeSummary popover, and the disabled MenuRadioItem in _chat.pull-requests where the tooltip can never open due to pointer-events:none. Let me verify the third one — the outer tooltip anchors to the whole paragraph block, and the scopes popover appears... Now I need to get the accurate diff line numbers for the review comments, specifically the right-side line numbers in the new files. I'm drafting a review comment for the DiffPanel.tsx file around line 585, focusing on how the tooltip trigger now wraps the entire Combobox component so that hovering or focusing it opens the tooltip, with the positioning layer handling the z-index stacking. For the second comment on PullRequestDetailPanel at line 1491/1492, I'm noting that PullRequestBaseFreshnessWarning is a hover popover with an Update control, and wrapping it in the tooltip trigger causes both to open simultaneously with z-index conflicts, suggesting the trigger be scoped to just the branch text. The third comment targets ConnectionsSettings line 696, where AccessScopeSummary is a hover popover that now opens alongside the tooltip when the whole paragraph is wrapped, and I'm recommending wrapping only the expiry label instead to avoid the overlap. For the fourth comment in _chat.pull-requests.tsx around line 1645/1652, MenuRadioItem's disabled state prevents pointer events on the trigger, making the tooltip unreachable for unavailable options since disabled items skip keyboard navigation, so I'm suggesting either a hoverable wrapper or rendering the reason inline. I'll skip flagging the DraftHeroHeadline tooltip issue. Since findings exist, I'm marking this check as a failure per the consistency guidelines and posting the review.
1. modify_pr — Reviewed the title → Tooltip migration for consistency with the shared tooltip primitive. Most conversions match the existing Tooltip / TooltipTrigger render={…} / TooltipPopup idiom and preserve aria-labels and layout (the Tooltip root emits no DOM, so flex parents are unaffected). Four call sites changed interaction ownership rather than just the tooltip mechanism: three now put the trigger around an interactive child that owns its own hover surface (tooltip positioner is z-[140], popover/combobox popups are z-[130]), and one attaches the trigger to an element the design system explicitly makes non-hoverable.
Agent Credits: 219 credits