feat(lint): ban native title tooltips and migrate to styled Tooltip - #7209
juliusmarminge merged 12 commits into
All clear
All clear
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.
🔍 Agent activity — reasoning, no tool calls
💭 Looking at the PR state at head 774d434, I need to verify that the remaining issues have been resolved. The main concern is whether
PullRequestActorLabelis still being nested insideTooltipTriggerat other call sites beyond the one inPullRequestSummaryTabwheretooltip={false}was added, and I should also confirm that theDiffPaneltrigger is properly scoped to just the span with the head ref and arrow. Beyond those, several components had title attributes removed—the resize handle inThreadPreviewMiniPlayer, the scroll button inChatView, and the status dot inConnectionStatusDot—so I need to verify each one still has an accessible name via aria-label or another mechanism, otherwise those could be accessibility regressions. The Tooltip was already there, so removing the redundant title attribute makes sense. Now I'm tackling the "Scroll to end" functionality in ChatView. The resize handle is preserving its aria-label correctly. Now I'm reviewing the remaining issues in the diff, particularly looking at the BrowserDeviceToolbar component where a Tooltip wraps a disabled Button. Since disabled buttons don't receive pointer events in some browsers, the tooltip won't trigger properly on hover. I need to check whether the Button primitive uses thedisabledattribute or adata-disabledconvention, and see if there's an established pattern in the codebase for handling tooltips on disabled buttons. Then I'll move through the other components — DiffPanel, pullRequestPresentation, ConnectionsSettings, and PullRequestListFilters — to verify their tooltip implementations are sound. For PullRequestListFilters specifically, the className merging withdata-disabled:pointer-events-autoshould work fine since MenuRadioItem supports that, and the prior commit already recommended this approach. I should also scan through other PullRequestActorLabel call sites to make sure adding the tooltip doesn't break anything that was relying on the title attribute for truncated logins, then check DiffPanel's recent changes around those line ranges and see what's been introduced in the commits since the last run. Looking at the recent commits to understand what changed. The tooltip prop was added to PullRequestActorLabel with it disabled in PullRequestSummaryTab, so I need to check if there are other places where this label is nested inside tooltip triggers that might cause conflicts. The resize handle tooltip removal looks straightforward, but I should search for any other related instances. I'm noticing a discrepancy between what the grep shows at the current commit and what the diff indicates should be there — the grep output for PullRequestActorLabel shows notooltipprop on lines 226-232, but the diff clearly showstooltip = truebeing added at line 229, which suggests the grep might have been run against a different state than the tree URL I specified. The grep output was stale, but that's resolved now. I'm checking the PullRequestActorLabel component—with tooltip disabled, it renders as a span containing the avatar and another span, where the CSS selector targets the avatar fallback. When tooltip is enabled at other call sites, the structure stays the same since TooltipTrigger wraps the span with the same class. Now I'm looking for other places where this component is used. I'm finding PullRequestActorLabel inside a button in PullRequestReviewerPicker's menu popup—adding a tooltip trigger span inside a button is valid, though it creates some redundancy since the login is already visible in the row. More importantly, since PullRequestActorLabel now always renders as a tooltip trigger by default (replacing the old native title attribute), I need to check whether any ancestor elements are already tooltip or hover popover triggers, which could cause nested tooltip issues. Let me trace through PullRequestRow to see if it wraps anything in a Tooltip. Looking at PullRequestChecksPopover, the ChecksBody span at line 61 had a title attribute that's now been moved inside the popover as a tooltip—the z-index layering checks out since 140 is above 130. Now I'm scanning the remaining files for any issues that haven't been caught or resolved yet, starting with BrowserDeviceToolbar where there's a tooltip on a disabled button—I need to see if the codebase has an established pattern for how Tooltip and disabled Button should interact together. For ConnectionsSettings around line 695, there's a paragraph containing a Tooltip with a custom span render, which overrides Base UI's default button rendering. The concern is that without the button element, the tooltip trigger might not be keyboard-accessible since it won't get a tabindex, though the original title attribute wasn't keyboard-accessible either so this is acceptable. ProjectContentSearchDialog has a Toggle inside a TooltipTrigger with the aria-label preserved, which looks good. ThemeColorPicker is more complex—it's nesting a TooltipTrigger inside a PopoverTrigger wrapping a button, and I'm checking whether the codebase has an established convention for this kind of nested trigger composition and what the correct ordering should be. There's also a potential issue where the popover opening on click might leave the tooltip visible on top of it, since Base UI doesn't automatically close tooltips when popovers open. I need to search the repo for existing examples of tooltip and menu or popover trigger combinations to understand the established patterns. The multiline grep approach isn't working since git grep operates line-by-line, so I need to search for files that contain both MenuTrigger and TooltipTrigger together. Let me check how many times TooltipTrigger appears in the codebase and look at some existing examples to see if there's already a pattern for combining tooltips with menu triggers. I'm noticing that the patternTooltip > TooltipTrigger render={MenuTrigger}appears consistently in PreviewMoreMenu and ChatMarkdown, which validates the composition approach used in DraftHeroHeadline and ThemeColorPicker. However, I spotted a structural difference — in PreviewMoreMenu the Menu sits outside the Tooltip, but in DraftHeroHeadline the Tooltip wraps the Menu instead. Let me check the actual code in DraftHeroHeadline around lines 100-160 to confirm the component ordering. I'm noticing a potential issue in DraftHeroHeadline where a Tooltip wraps the MenuRadioItem children, creating a duplicate label on hover that mirrors what was already there with the title attribute. The concern is whether the tooltip inside the menu popup might conflict with the menu's hover styling or z-index layering. Looking at DiffPanel, the aria-label on a plain div is likely being ignored since it's not a landmark element, but checking the merge-base version shows this aria-label existed before the title attribute was removed, so it's not a new issue introduced here. I'm now moving through the remaining files to check for similar problems in BrowserDeviceToolbar, PullRequestListFilters, and ChangedFilesTree. The key question is whether MenuRadioItem properly merges className with the consumer's classes coming last, and whether the pointer-events-auto override actually enables tooltips on disabled menu items. Since MenuRadioItem renders as a div with role="menuitemradio" rather than a native disabled button, re-enabling pointer events should allow hover to work. Let me verify how MenuRadioItem handles className merging in the menu component. I'm noticing thatMenuRadioItemwraps its children in a span with flex properties, and whenPullRequestListFiltersusesTooltipTriggerwith a render prop pointing to the fullMenuRadioItemelement, Base UI's render prop mechanism clones and merges the props correctly. There's a potential issue withdata-disabledand pointer events that I need to think through. Tailwind-merge should handle the conflict betweendata-disabled:pointer-events-nonefrom the base anddata-disabled:pointer-events-autofrom the consumer correctly—since they share the same variant prefix and property group, the later one (consumer's) wins. That's good since the consumer className comes after in thecncall. ForBrowserDeviceToolbar, theButtongetsdisabled={pending || !customValid}, which renders a native disabled button element. Native disabled buttons don't fire pointer events, so a tooltip won't display when disabled. The nativetitleattribute does work on disabled buttons in most browsers, but mouse events aren't dispatched to disabled form controls, which complicates tooltip behavior. I'm wondering if this is actually a meaningful regression—the tooltip just repeats what the aria-label already says ("Lock aspect ratio"), so losing it when disabled is minor and probably aligns with other patterns in the codebase. Let me check if there's a convention here by looking for how the repo handles tooltips on disabled controls, like inPullRequestReviewerPicker. I see that the existing pattern already handles tooltips on disabled buttons, like in PullRequestReviewerPicker, so BrowserDeviceToolbar follows the same convention. Now I'm working through the remaining components to verify: ProjectSettingsPanel with its button containing code and copy icons, ServerUpdateAction, FilePreviewPanel, ChangedFilesTree, ConnectionsSettings, ThemeColorPicker, PullRequestCodeTab, PullRequestSummaryTab, and PullRequestDetailPanel's branch labels. Let me look at PullRequestDetailPanel more closely around the branch range section and the copy branch button area to see if there are any issues there. Looking at the tooltip changes in the migration — the button's title attribute previously showed "Copied" or "Copy pull request branch" with a separate nested title on the code element displaying the branch name. Now it's consolidated into a single tooltip that combines both pieces of information as "Copy pull request branch: ", which works fine and keeps everything accessible. Moving on to check how ProjectSettingsPanel handles the TooltipTrigger wrapping around a button with children. I'm noticing the codebase uses both patterns for the render prop — sometimes the button is self-closing inside render with children elsewhere, sometimes it's the other way around. Base UI handles this gracefully by merging the children appropriately. Now I'm looking at ConnectionsSettings around line 695 where the Tooltip.Trigger is rendering onto a<span>instead of a button, which overrides the default button behavior. Since it's not a native button element, Base UI won't addtype="button", which is correct. But I'm wondering if there's a prop likenativeButton={false}that might be needed when rendering onto non-button elements. Looking at what the repo already does elsewhere, I see many existing TooltipTriggers rendering onto spans without any special props, so this pattern is consistent with the existing convention. That's reassuring. The DiffPanel with therole="img"andaria-labelon CheckIcon looks good too. Now I need to quickly verify that the components being modified — ChangedFilesTree, ServerUpdateAction, FilePreviewPanel, and PullRequestCodeTab — already have the necessary Tooltip imports in place rather than needing new ones added. I need to check if there are any remainingtitle=attributes on intrinsic HTML elements in the apps/web/src directory that would violate the lint rule now set to error level, since this could cause CI failures. I'm scanning through several files to identify which title attributes are on intrinsic elements that would trigger the new lint rule. Most of these look like they're on custom components like SettingsRow, but I need to verify a few cases—particularly the ones in ConnectionsSettings where the title might be applied directly to an SVG, image, or QR code component. I'm looking at a lint issue in Sidebar.tsx where atitleprop is being passed to a customButtoncomponent that forwards it down to the native<button>element. The linter doesn't catch this because it only flags intrinsic elements, but the native tooltip still gets applied — which seems to be exactly what the PR was trying to clean up based on the commit message about migrating pass-through title props. The question is whether this counts as in-scope: it's an unchanged line in a changed file, which normally wouldn't be reportable, but the PR directly establishes the no-native-title convention and makes this inconsistency relevant. I'm now scanning through the codebase to see how many other component pass-through titles like this exist across different files. Now I'm going through the specific files to identify which title attributes are legitimate component props versus problematic pass-throughs on native elements. Most of these look fine — SettingsRow titles are intentional, QRCodeSvg title is a proper SVG element, and component props like in PullRequestSummaryTab are expected. The key ones to verify are the native element pass-throughs like the Button in Sidebar and the element in _chat.pull-requests. The remaining instance in Sidebar.tsx wheretitleduplicatesaria-labelon aButtoncomponent appears to be an oversight, especially since the PR already removed similar duplicates elsewhere in the same file and in an earlier commit that cleaned up pass-through titles on other components. SinceButtonforwards props through to the underlying element, this native title tooltip will still render, creating an inconsistency with the styled tooltips used everywhere else. It's worth flagging as a minor consistency issue, though it's on a line the PR didn't directly modify, so it might not be critical. I should check whether there are other similar Button pass-throughs in the codebase outside the changed files to see if this represents a systemic gap the PR left behind. Now I'm going through the specific changes to verify there are no regressions. Looking at the test expectations for server-rendered markup, the button key handling in the tree component, the toggle composition inside tooltips, the CSS selectors for hidden elements, and the breadcrumb structure — everything appears consistent with the original behavior. I'm also checking whether wrapping components like Tooltip around PopoverTrigger creates any z-index or anchor conflicts, and comparing against existing patterns like PreviewMoreMenu where this nesting already works. The components look good — ProjectSettingsPanel's copy-path button, PullRequestCodeTab, and ProjectContentSearchDialog all handle their tooltips correctly now. ServerUpdateAction also properly shows a tooltip on the truncated message text. I'm wondering if there's anything about the oxlint rule itself that needs attention. The rule only catches intrinsic elements and missing pass-through props on shared primitives like Button, Badge, MenuRadioItem, Toggle, and MenuTrigger — which the author already migrated manually. Sidebar.tsx still has an unmigrated instance at line 3520, but since that line isn't in the diff, I can't post an inline comment there; GitHub won't accept review comments outside the changed hunk range. I could mention it in the summary instead, though the instructions prefer inline comments. This feels like a minor leftover rather than a regression the patch introduced, so it might not be significant enough to fail the check. Now I'm reconsidering the check's default behavior — it fails by default and only passes with "All clear" when there are no findings. But my response needs to be JSON per the schema, so I'll express the result through the state and title fields rather than a plain text message. Let me do another careful pass through the recent commits, especially the branch tooltips changes and the pull request detail panel updates. I'm noticing the base and head labels now have identical tooltips showing the full range, which is acceptable since they're just adjacent triggers with the same text. The truncation behavior on the span inside the flex row looks fine too. Looking at the condensed toolbar and the pull request list filters, I'm checking for issues like duplicate keys and missing key warnings. The disabled menu items now have pointer events re-enabled with thedata-disabled:pointer-events-autoclass, which means hovering shows the cursor as not-allowed and the tooltip can appear on pointer interaction. Base UI properly ignores highlight for disabled items, and keyboard navigation still skips them, so the tooltip remains keyboard-unreachable — which matches the limitation of the original title attribute approach. I'm checking whetherMenuRadioItemproperly applies both thedata-disabledanddisabledattributes, and since Base UI renders it as adivwithrole="menuitemradio", thepointer-events-autoshould work. But I need to verify if the tooltip actually opens when the trigger is composed onto the menu item, since hover events might be affected by the menu item's behavior. There's also the question of how Base UI handles therenderprop when cloning elements — it merges className strings, so thedata-disabled:pointer-events-autoclass should persist through the cloning process. I'm noticing there's a leftovertitleattribute on aButtonin Sidebar.tsx that creates a duplicate tooltip and duplicates the aria-label, but since it's not on a diff line, I can't post an inline comment on it through GitHub. Given that this is a pre-existing line the PR didn't change, and the instructions warn against flagging unrelated repository-wide cleanup, I'll treat this as clean — no new regressions introduced. All previously flagged issues have been resolved, so the final response is "All clear".
Agent Credits: 204 credits