Skip to content

ECHOES-1451 Apply data-attribute to popover rather than its trigger - #754

Merged
jeremy-davis-sonarsource merged 1 commit into
mainfrom
jay/teachingbubble-data-attribute
Aug 24, 2026
Merged

ECHOES-1451 Apply data-attribute to popover rather than its trigger#754
jeremy-davis-sonarsource merged 1 commit into
mainfrom
jay/teachingbubble-data-attribute

Conversation

@jeremy-davis-sonarsource

@jeremy-davis-sonarsource jeremy-davis-sonarsource commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary by Gitar

  • Components:
    • Added contentProps to PopoverProps to forward extra attributes directly to the popover panel.
    • Updated TeachingBubble to apply feature communication data attributes to popover content instead of its trigger.

This will update automatically on new commits.

@netlify

netlify Bot commented Aug 20, 2026

Copy link
Copy Markdown

Deploy Preview for echoes-react ready!

Name Link
🔨 Latest commit 70aee1e
🔍 Latest deploy log https://app.netlify.com/projects/echoes-react/deploys/6a86c324aac7000008df7993
😎 Deploy Preview https://deploy-preview-754--echoes-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Apply data-attribute to popover rather than its trigger ECHOES-1451 Apply data-attribute to popover rather than its trigger Aug 20, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Aug 20, 2026

Copy link
Copy Markdown

ECHOES-1451

Comment thread src/components/teaching-bubble/TeachingBubble.tsx
Comment thread src/components/popover/Popover.tsx
@jeremy-davis-sonarsource
jeremy-davis-sonarsource force-pushed the jay/teachingbubble-data-attribute branch from 06a0df2 to 70aee1e Compare August 20, 2026 09:04
@gitar-bot

gitar-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Forwards extra attributes to the popover panel via contentProps and updates TeachingBubble to apply feature communication attributes to the popover content instead of its trigger. Consider preserving consumer-provided contentProps and accounting for the attribute being absent while the popover is closed.

✅ 2 resolved
Edge Case: Feature-communication attribute now absent while popover closed

📄 src/components/popover/Popover.tsx:135-138 📄 src/components/teaching-bubble/TeachingBubble.tsx:56-58
The data-feature-communication attribute previously lived on the always-rendered trigger; it now lives on PopoverContent, which Radix renders inside a Portal only while the popover is open. Any analytics/e2e tooling that queries this attribute on a closed TeachingBubble will no longer find it. Confirm consumers of the attribute only inspect it while the bubble is open, since this is a behavioral change from the previous trigger placement.

Bug: TeachingBubble drops consumer-provided contentProps

📄 src/components/teaching-bubble/TeachingBubble.tsx:55-58
contentProps is part of SelectedPopoverProps, so a consumer of TeachingBubble can pass it. Because the explicit contentProps={{...}} is written after {...popoverProps}, any consumer-supplied contentProps is silently overwritten and lost. If TeachingBubble should support extra content attributes, merge them: contentProps={{ ...popoverProps.contentProps, [FEATURE_COMMUNICATION_DATA_ATTRIBUTE]: FeatureCommunicationComponent.TeachingBubble }} (and destructure it out of popoverProps).

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown

@jeremy-davis-sonarsource
jeremy-davis-sonarsource merged commit 33a63f3 into main Aug 24, 2026
12 checks passed
@jeremy-davis-sonarsource
jeremy-davis-sonarsource deleted the jay/teachingbubble-data-attribute branch August 24, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants