Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Jun 25, 2025

Description

Fixes #5090

https://github.com/user-attachments/assets/d67c82b6-0674-4955-87d6-883069ce3844
image

This PR standardizes all tooltip delays across the Roo Code webview to use a consistent 300ms delay, replacing the inconsistent delays that ranged from instant (0ms) to over 2 seconds.

Changes Made

  • Created StandardTooltip component that wraps Radix UI's Tooltip with a fixed 300ms delay
  • Replaced all native HTML title attributes with StandardTooltip across 40+ component files
  • Updated all affected test files to work with the new tooltip implementation
  • Exported StandardTooltip from the UI components index for easy importing

Testing

  • All existing tests pass (487 tests passing)
  • Manual testing completed:
    • Verified tooltips in chat components show after 300ms
    • Verified tooltips in settings components show after 300ms
    • Verified tooltips in marketplace components show after 300ms
    • Verified tooltips in history components show after 300ms
    • Verified tooltips in common components show after 300ms

Verification of Acceptance Criteria

  • All tooltips within the webview now use consistent 300ms delay
  • No more instant tooltips or 2+ second delays
  • Tooltip behavior is consistent across the entire application

Known Limitation

The VSCode activity bar icon tooltip ("Roo Code" when hovering over the extension icon) cannot be customized as it's controlled by VSCode's native UI through the extension manifest. This is a platform limitation that affects all VSCode extensions.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic (StandardTooltip component documented)
  • Documentation updated (component has JSDoc)
  • No breaking changes
  • Accessibility checked (tooltips work with keyboard navigation)

Screenshots/Demo

All tooltips now consistently appear after 300ms hover delay instead of the previous inconsistent behavior.


Important

Standardizes tooltip delays to 300ms across the application by introducing a StandardTooltip component, replacing native title attributes, and updating tests accordingly.

  • Behavior:
    • Introduces StandardTooltip component with a fixed 300ms delay, replacing native HTML title attributes in over 40 component files.
    • Ensures consistent tooltip behavior across the application.
  • Testing:
    • Updates all affected test files to accommodate the new tooltip implementation.
    • Adds tests for StandardTooltip in tooltip.spec.tsx.
  • Misc:
    • Exports StandardTooltip from the UI components index for easy importing.

This description was created by Ellipsis for 7f3a55a. You can customize this summary. It will automatically update as commits are pushed.

- Created StandardTooltip component with consistent 300ms delay
- Replaced all native HTML title attributes with StandardTooltip
- Updated 40+ component files across the webview
- Fixed all test files to work with StandardTooltip
- Ensures consistent tooltip behavior throughout the application

Note: VSCode activity bar tooltips cannot be customized due to platform limitations
Copilot AI review requested due to automatic review settings June 25, 2025 03:21
@hannesrudolph hannesrudolph requested review from cte, jr and mrubens as code owners June 25, 2025 03:21
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. UI/UX UI/UX related or focused labels Jun 25, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jun 25, 2025

We have finished reviewing your PR. We have found no vulnerabilities.

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph marked this pull request as draft June 25, 2025 03:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Standardizes tooltip behavior by creating a single StandardTooltip wrapper with a fixed 300ms delay and replacing all inline title attributes across the codebase. Tests and mocks are updated to accommodate the new component.

  • Introduces StandardTooltip component enforcing a 300ms delay.
  • Replaces native title attributes with StandardTooltip in UI, common, settings, marketplace, history, chat modules.
  • Updates test suites to mock or wrap the new tooltip implementation.

Reviewed Changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
webview-ui/src/components/ui/standard-tooltip.tsx Defines StandardTooltip wrapper enforcing 300ms tooltip delay
webview-ui/src/components/ui/index.ts Exports StandardTooltip for reuse
webview-ui/src/components/ui/select-dropdown.tsx Replaces native title on dropdown triggers with StandardTooltip
webview-ui/src/components/common/IconButton.tsx Removes inline title from icon buttons and wraps them in StandardTooltip
webview-ui/src/components/marketplace/components/MarketplaceItemCard.tsx Switches from Radix Tooltip to StandardTooltip for install/remove controls
Comments suppressed due to low confidence (1)

webview-ui/src/components/marketplace/components/tests/MarketplaceItemCard.spec.tsx:60

  • This test suite doesn’t mock StandardTooltip, so real tooltip logic may run and make tests brittle. Consider adding a StandardTooltip mock (e.g., returning its children) to stabilize these tests.
	return render(<TooltipProvider delayDuration={300}>{ui}</TooltipProvider>)

- Set asChild={true} for StandardTooltip components in progress bar sections
- This prevents extra wrapper divs that were breaking the flex layout
- Progress bar now displays correctly with proper width calculations
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 25, 2025
- Add aria-label to IconButton for accessibility
- Extract STANDARD_TOOLTIP_DELAY as a constant
- Move TooltipProvider to app level for better performance
- Enhance StandardTooltip documentation with examples
- Update all test files to use custom test utils with TooltipProvider
- Add collisionPadding and avoidCollisions props to TooltipContent
- This ensures tooltips automatically reposition when they would overflow the container
- Move collisionPadding and avoidCollisions to base TooltipContent component
- This ensures all tooltips in the app respect viewport boundaries
- Remove duplicate props from StandardTooltip
hannesrudolph added a commit that referenced this pull request Jun 25, 2025
- Remove overflow-hidden class from TooltipContent that prevented wrapping
- Add max-w-[300px] and break-words classes for proper text wrapping
- Add optional maxWidth prop to StandardTooltip for customization
- Add comprehensive tests for tooltip behavior

Addresses feedback from PR #5098 where tooltips were extending beyond
the plugin viewport boundaries when content was too wide.
@hannesrudolph hannesrudolph marked this pull request as ready for review June 25, 2025 04:26
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Jun 25, 2025
- Remove TooltipProvider from StandardTooltip component (performance)
- Fix nested tooltips in ContextWindowProgress by consolidating into single tooltip
- Verify IconButton already has aria-label attribute (accessibility)
@hannesrudolph
Copy link
Collaborator Author

Created PR #5101 to address the tooltip overflow issue. The fix enables text wrapping by removing the overflow-hidden class and adding proper wrapping styles with a default max-width of 300px.

- Remove overflow-hidden class from TooltipContent that prevented wrapping
- Add max-w-[300px] and break-words classes for proper text wrapping
- Add optional maxWidth prop to StandardTooltip for customization
- Add comprehensive tests for tooltip behavior

Addresses feedback from PR #5098 where tooltips were extending beyond
the plugin viewport boundaries when content was too wide.
@hannesrudolph
Copy link
Collaborator Author

Added tooltip overflow fix to this PR. The changes include:

  • Removed overflow-hidden class from TooltipContent that was preventing text wrapping
  • Added max-w-[300px] and break-words classes for proper text wrapping
  • Added optional maxWidth prop to StandardTooltip for customization
  • Added comprehensive tests for tooltip behavior

Tooltips now wrap text within a 300px maximum width by default instead of extending beyond the plugin viewport.

@hannesrudolph
Copy link
Collaborator Author

hannesrudolph commented Jun 25, 2025

I've addressed all three issues from the review:

  1. Performance concern: Removed the individual TooltipProvider from the StandardTooltip component. The provider is now only at the app root level.

  2. Nested tooltips: Fixed the nested tooltips in ContextWindowProgress by consolidating them into a single tooltip that displays both the percentage and token count information.

image
  1. Accessibility: Verified that IconButton already has the aria-label attribute properly set.

All tests are passing and CI/CD checks are green. The changes are ready for re-review.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 25, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 25, 2025
- Check for empty strings and whitespace-only content
- Prevents rendering tooltips with no meaningful content
- Addresses edge case where content could be an empty string
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 26, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 26, 2025
@mrubens mrubens merged commit 3318366 into main Jun 26, 2025
17 checks passed
@mrubens mrubens deleted the fix/issue-5090-tooltip-delays branch June 26, 2025 19:41
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 26, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 26, 2025
daniel-lxs added a commit that referenced this pull request Jun 27, 2025
…flict

The StandardTooltip component was incorrectly wrapping the PopoverTrigger,
which prevented the popover from opening. This was introduced in PR #5098
when standardizing tooltip delays.

The fix moves StandardTooltip inside PopoverTrigger (with asChild prop)
so that PopoverTrigger can properly control its child element while still
providing tooltip functionality.

This pattern is consistent with other components like ShareButton.
hannesrudolph added a commit that referenced this pull request Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer PR - Needs Review size:XL This PR changes 500-999 lines, ignoring generated files. UI/UX UI/UX related or focused

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Standardize ALL tooltips to use consistent 300ms timing across the entire application

4 participants