💭 I'm looking at the changed web files related to desktop update logic, the toast component, and sidebar update pill/release notes, along with their tests, to understand tooltip and keyboard behaviors in scope. A concern crosses my mind: putting interactive links inside tooltip content is an accessibility antipattern since tooltip content typically isn't focusable and closes on blur/pointer leave, plus there's custom Tab handling to check. I want to review the full SidebarUpdatePill.tsx file next.

1. browse_code — apps/web/src/components/sidebar/SidebarUpdatePill.tsx

2. browse_code — apps/web/src/components/desktopUpdate.toast.tsx

💭 I'm checking whether SidebarUpdateReleaseNotes.tsx should use an existing link primitive instead of raw anchor tags, and looking at whether the tooltip component keeps the popup open when focus moves to a link inside it, since tooltips often close on blur/mouse leave. I should verify the Tab-key handling correctly guards against a null ref when the tooltip isn't open, check how openExternal is used elsewhere for external links, and confirm there's no cursor or accessibility regression with the anchor's click handling for keyboard users.

3. browse_code — apps/web/src/components/ui/*.tsx

4. git_tools — openExternal

💭 I'm wondering whether Base UI's tooltip actually keeps the popup open when focus moves inside it, since it typically closes on trigger blur—which would break the Tab-into-tooltip approach entirely. I want to check tooltip.tsx to confirm this behavior.

5. browse_code — apps/web/src/components/ui/tooltip.tsx

6. browse_code — apps/web/src/components/sidebar/SidebarUpdateReleaseNotes.tsx

💭 I'm checking that the overflow behavior in the tooltip viewport isn't a new issue since it matches the prior component's setup. I'm also considering keyboard focus behavior—since Base UI tooltips close when the trigger loses focus, tabbing into a link inside the popup could be problematic. The subtlety is whether the blur's relatedTarget lands inside the floating element itself: with floating-ui's useFocus/useDismiss logic, focus moving into the popup shouldn't trigger a close, but programmatically calling .focus() after preventDefault() on Tab still fires a blur event with a defined relatedTarget pointing to the newly focused anchor, so I need to trace through whether that gets treated as staying within the floating element or not. I also notice the new test file doesn't actually render the component -- it manually calls element.type(props) on the React element tree, which means it can't validate real focus transitions, tooltip visibility, or keyboard behavior at all, despite superficially looking like coverage for those things. Looking for something more concrete: firstLinkRef only gets attached when index === 0, but if that first release note's URL is null, nothing gets the ref and the Tab handler just silently no-ops since firstReleaseLinkRef.current is null. There's also a deeper issue where the button's onKeyDown intercepts Tab and forces focus into the tooltip's contents, but the tooltip popup isn't necessarily kept mounted -- so if it's closed, the ref it needs would be null too. Beyond that, shift+Tab from the first link correctly returns to the button, but forward-Tab from the last link would move focus to whatever follows the portaled tooltip in the DOM (since it's appended at the end of body), which could break the sidebar's natural tab order. That's a real keyboard navigation risk, though it might be considered outside the current scope. I also notice the anchor's focus-visible style only tweaks text color rather than adding a visible ring like other interactive elements in the codebase use. The pointer-events pattern checks out — positioner is none, popup is auto only when release notes exist, matching the same condition used for rendering the links. I'm now checking the omitted-release-count block, which renders a link to the history URL, and wondering what happens if none of the release notes actually have a releaseUrl set. Moving openDesktopUpdateReleaseNotes into the toast module creates a small coupling with the sidebar component, though the function itself is generic enough that it's a minor concern. I also notice SidebarUpdateReleaseNotes keys its list items by releaseNote.version, which could collide if versions duplicate — a pre-existing issue rather than something new. I'm now looking at the Tooltip Viewport's styling for the current-item indicator width calculation. Checking a few more minor style points around the anchor's spacing and link hover states, none of which seem problematic. I'm considering a more substantive concern: how clicking a link inside the tooltip interacts with Base UI's popup behavior, but since the click target is inside the popup rather than outside it, this shouldn't trigger an unwanted close. The bigger question worth investigating is keyboard focus behavior when tabbing into the tooltip. So focus moving into the floating tooltip element won't trigger a close, meaning tabbing into the link should keep the tooltip open since the browser sets relatedTarget correctly on blur. But then tabbing forward from the last link causes focus to leave the portal, closing the tooltip and sending focus to whatever follows the portal in DOM order — since portals render at the end of body, that could send focus somewhere unexpected. This actually seems like a real keyboard regression: once the tooltip closes and the portal unmounts, the DOM element receiving focus is gone, so focus falls back to document.body entirely, restarting the tab order from the top of the page rather than continuing naturally after the button. That's worth flagging as a concrete usability issue introduced by this pattern. So in Electron this likely means focus lands nowhere once the popup unmounts. It's a plausible finding, but I'd rather locate something more clearly broken, so let me check getDesktopUpdateReleaseUrl next.

7. browse_code — apps/web/src/components/desktopUpdate.logic.ts

8. browse_code — apps/web/src/components/sidebar/SidebarUpdateReleaseNotes.test.tsx

💭 I'm also checking that only the first release link handles shift+Tab back to the button, and confirming the button's onKeyDown for plain Tab is attached correctly through the render prop from TooltipTrigger. But there's a real navigation problem: since Tab always redirects into the release notes popup whenever it's open, and the last link then exits to the portal's DOM position at the end of the body rather than back to the sidebar, users lose the ability to tab past the update button into the next sidebar control in normal order. I'm also thinking the tooltip likely opens on keyboard focus, which makes this trap trigger just from tabbing to the pill—and if Escape is pressed afterward, focus could end up on a removed element. This feels like the strongest, most concrete accessibility issue to flag, especially compared to how Base UI's Popover handles focus natively with initialFocus/returnFocus. The real fix here is that interactive content like links shouldn't live inside a Tooltip at all—it belongs in a Popover, which owns focus and Escape handling instead of requiring hand-rolled Tab routing. Since the repo already has ui/popover.tsx, this aligns with preferring existing primitives, though I need to be careful not to over-recommend a full Tooltip-to-Popover migration since that's a bigger architectural suggestion than a targeted review comment. That's a dead end too. Checking the pointer-events chain confirms it works as intended, and the Tailwind class ordering is just formatter output, not a real issue. The focus-visible styling being only a color change on the anchor might be worth a closer look though.

9. git_tools — focus-visible:ring-2

10. git_tools — focus-visible:

💭 I notice the repo consistently applies focus-visible ring styling to interactive elements, but the new ReleaseLink anchor lacks that ring, only styling hover/focus text color the same way. Since this PR is specifically about adding Tab navigation to these links, that's a real accessibility gap worth flagging, though the anchor still retains the browser's default focus outline since outline-none wasn't set. For screen reader users, the tooltip text becomes an accessible description of the trigger, but the links inside likely aren't reliably reachable via virtual cursor navigation—the custom Tab handling effectively strips them from normal AT semantics. For mouse users though, Base UI's hoverable tooltip behavior with a safe polygon should keep the popup open across the gap, so pointer interaction isn't the real problem—the risk is really about keyboard and assistive tech accessibility. Tabbing forward should open the tooltip and route focus into the links, but this breaks down: tabbing past the last link exits the portal at the document's end rather than continuing sidebar navigation, there's no Escape-to-return-focus handling, and semantically a tooltip role isn't meant to hold interactive links. Since the codebase already has a Popover primitive with proper focus management built in, the more durable fix would be to use that instead of a tooltip for content that needs to hold interactive links, letting the primitive handle focus trapping, Escape, and focus return rather than reconstructing that logic by hand. I also want to flag a smaller issue around the missing focus-visible styling on the new link, though I should weigh whether these findings are strong enough to report versus defaulting to "All clear" if nothing substantive holds up. Before finalizing, I should check whether the repo has existing precedent for putting interactive elements inside a tooltip popup — if that pattern already exists elsewhere, it would weaken my finding here.

11. git_tools — pointer-events-auto

💭 I don't see existing precedent for interactive controls inside a TooltipPopup in this codebase, so I'm noting that. I'll flag the ReleaseLink anchor in SidebarUpdateReleaseNotes.tsx as missing the app's standard focus-visible ring styling. I'm also considering a second issue: when tabbing forward from the last link, focus exits the popup which then unmounts, dropping focus to body with no return-to-trigger behavior on Escape either. The existing keydown handler on ReleaseLink only covers shift+Tab, not forward Tab, so this exit case seems genuinely unhandled — I'm weighing whether it's worth flagging given it would require a multi-hunk fix rather than a simple inline suggestion. For the fix suggestion, I'm adding a rounded focus outline with a visible ring style to the link's className at line 44, keeping proper indentation. I need to confirm the line numbers align correctly with the diff's right-side numbering for the review comment placement. For the second issue, I'm targeting the keydown handler in SidebarUpdateReleaseNotes.tsx around lines 36-40, since it only handles shift+Tab and not forward Tab or Escape — meaning focus can leak into a portal at the end of the body and get lost instead of returning properly to the trigger control. I'm drafting a comment explaining this focus-trap gap and suggesting either intercepting forward Tab on the last link or switching to the existing Popover component for more robust focus management. For overall review status, since I have concrete findings, I should mark this as a failure state with inline comments rather than "all clear." I'm also double-checking whether the first issue about the focus ring is actually valid — the anchor doesn't set outline-none, so the default focus ring should still show, but it's worth confirming against the app's usual focus-ring convention.

12. modify_pr — UI consistency review of the changed web files (SidebarUpdateReleaseNotes.tsx, SidebarUpdatePill.tsx, desktopUpdate.toast.tsx, desktopUpdate.logic.ts). Two findings, both on the newly interactive release-notes panel that now lives inside the tooltip popup.