Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jun 28, 2025

Description

This PR fixes the share button popover that wasn't opening when clicked.

Problem

The StandardTooltip was wrapping PopoverTrigger, which prevented the popover from opening. This is the same issue that was fixed in PR #5192 for CheckpointMenu.

Solution

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

Changes

Testing

  • Verified share popover opens when clicked
  • Verified tooltip still appears on hover
  • Tested both authenticated and unauthenticated states

Related Issues

Follows the same pattern as #5192


Important

Fixes popover issue in ShareButton.tsx by moving StandardTooltip inside PopoverTrigger.

  • Behavior:
    • Fixes popover not opening in ShareButton.tsx by moving StandardTooltip inside PopoverTrigger.
    • Ensures tooltip still appears on hover.
  • Testing:
    • Verified popover opens on click and tooltip appears on hover.
    • Tested in both authenticated and unauthenticated states.

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

Fixes popover not opening when StandardTooltip wraps PopoverTrigger.
Follows the same pattern as PR #5192 for CheckpointMenu.
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners June 28, 2025 00:09
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Jun 28, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jun 28, 2025

No security or compliance issues detected. Reviewed everything up to c55784d.

Security Overview
  • 🔎 Scanned files: 1 changed file(s)
Detected Code Changes
Change Type Relevant files
Bug Fix ► ShareButton.tsx
    Move StandardTooltip inside PopoverTrigger to fix popover opening
► run.ts
    Change stdin handling and message passing
► run.spec.ts
    Remove and simplify test implementations

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.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jun 28, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 28, 2025
@mrubens mrubens merged commit 867eaf4 into main Jun 28, 2025
22 checks passed
@mrubens mrubens deleted the fix-share-button-popover-tooltip branch June 28, 2025 00:13
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 28, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants