fix(desktop): restore styled tooltips on right sidebar file tree buttons and rows - #438
fix(desktop): restore styled tooltips on right sidebar file tree buttons and rows#438hashbender wants to merge 1 commit into
Conversation
|
Review Complete Files Reviewed: 2 By Severity:
A minor accessibility regression in the desktop app's right sidebar: the refresh button loses its tooltip during loading states after migrating from native Files Reviewed (2 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟢 Low (18/100) — 1 low finding · 203 LOC across 2 files
Summary
PR #438 migrates tooltips in the desktop app's right sidebar header from native HTML title attributes to Radix UI <Tip> wrappers. The migration is correct for most cases, but introduces a minor accessibility regression for the refresh button during loading states.
Findings
Disabled refresh button loses tooltip after title→Tip migration
- File:
apps/desktop/src/app/right-sidebar/index.tsx(lines 155–166) - Severity: low | Confidence: 85%
- Root cause: Radix UI's
TooltipTriggercomponent does not activate on disabled HTML elements (documented limitation). Whenloading=true, the refresh<Button>is disabled, yet it becomes visually visible via CSSgroup-hoverrules that restorepointer-events-autoandopacity. A sighted user hovering over the visible-but-disabled button sees no tooltip, whereas the old nativetitle={r.refreshTree}attribute worked regardless of disabled state. - Screen reader users are unaffected —
aria-labelis preserved. - Fix: Wrap the disabled button in a
<span>element whenloadingis true, so the tooltip trigger targets the span wrapper rather than the disabled button directly. Alternatively, restore the nativetitleattribute as a fallback alongside<Tip>.
| <Tip label={r.refreshTree} side="left"> | ||
| <Button | ||
| aria-label={r.refreshTree} | ||
| className={HEADER_ACTION_LABEL_REVEAL} | ||
| disabled={loading} | ||
| onClick={onRefresh} | ||
| size="icon-xs" | ||
| variant="ghost" | ||
| > | ||
| <Codicon name="refresh" size="0.8125rem" spinning={loading} /> | ||
| </Button> | ||
| </Tip> |
There was a problem hiding this comment.
🟢 Disabled refresh button loses tooltip after title->Tip migration (bug)
The refresh button in the right sidebar header (index.tsx:155-166) is wrapped in instead of using a native title attribute. When loading=true, the button is disabled. Radix UI's TooltipTrigger does not activate on disabled elements (documented Radix limitation). The button is still visually revealed via CSS group-hover (HEADER_ACTION_LABEL_REVEAL restores pointer-events-auto and opacity), creating a state where a visible disabled button shows no tooltip -- the old title={r.refreshTree} worked in this state. The collapse button at lines 167-178 has a similar pattern but is pointer-events-none opacity-0 when disabled, so the gap is moot there. Screen reader users are unaffected because aria-label is preserved.
💡 Suggestion: Per Radix documentation, disabled elements need a wrapper for the tooltip trigger. Wrap the in a when loading is true and apply the to the span; when not loading, wrap the button directly as before. Alternatively, add the native title back as a fallback alongside .
📋 Prompt for AI Agents
In apps/desktop/src/app/right-sidebar/index.tsx, lines 155-166, the refresh button is wrapped in a component. Radix UI TooltipTrigger does not activate on disabled elements. When the button is disabled (loading=true), the tooltip will not appear. Fix by conditionally wrapping the button in a when disabled so the tooltip trigger targets the span instead of the disabled button. For example: store the button JSX in a variable, and conditionally render {button} when loading, or {button} when not loading.
1 participant
What does this PR do?
Fixes the Refresh and Collapse All buttons in the right sidebar's file tree header, and each file/folder row in the file tree itself, showing plain browser-native tooltips instead of the app's styled dark tooltip.
This restores work from NousResearch#51315 ("replace native title tooltips with styled Tip component"), which wrapped these buttons in
<Tip>. NousResearch#49037 rewroteFilesystemTabas part of the Projects feature, and its merge reintroduced the nativetitle=attributes, reverting the tooltip fix from NousResearch#51315. NousResearch#49037 also removed the Open Folder button from that same file — its PR description states the freeform folder picker was retired in favor of project/worktree-based workspace switching, so that change is intentional and out of scope here. Only the two remaining buttons need the tooltip restored.Additionally,
ProjectTree(./files/tree.tsx) rendered each file/folder row with a nativetitle={node.data.id}showing the full path, which had the same plain-tooltip regression. This wasn't touched by NousResearch#51315 originally, so it's addressed here with the same fix.Related Issue
Fixes #
Type of Change
Changes Made
apps/desktop/src/app/right-sidebar/index.tsx— replaced nativetitle=with<Tip label={...} side="left">on the Refresh and Collapse All buttons inFilesystemTab(regressed by feat(desktop): first-class projects — sidebar, coding rail, review pane, and agent project tools NousResearch/hermes-agent#49037; Open Folder was intentionally removed by that PR and is not restored here)apps/desktop/src/app/right-sidebar/files/tree.tsx— replaced nativetitle={node.data.id}with<Tip label={node.data.id} side="left">on each file tree row inProjectTreeRowHow to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A
Screenshots / Logs
Mirror-of: NousResearch#56520
NousResearch#56520