Skip to content

chore: remove ids from tooltips #7254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

uinstinct
Copy link
Contributor

@uinstinct uinstinct commented Aug 20, 2025

Description

Initially TooTips required to have an id associated with 2 elements to display it. This required 2 independent elements one with id and another with data-tooltip-id.
This PR introduces a single Tooltip which removes the usage of id and the displaying element can be passed as children to the tooltip

  • refactors ToolTips to match new props
  • keeps all of the UI and other content the same

resolves CON-3586

AI Code Review

  • Team members only: AI review runs automatically when PR is opened or marked ready for review
  • Team members can also trigger a review by commenting @continue-general-review or @continue-detailed-review

Checklist

  • [] I've read the contributing guide
  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created

Screen recording or screenshot

[ When applicable, please include a short screen recording or screenshot - this makes it much easier for us as contributors to review and understand your changes. See this PR as a good example. ]

Tests

[ What tests were added or updated to ensure the changes work as expected? ]


Summary by cubic

Replaced id-based tooltip wiring with a simpler, id-less API. Tooltips now wrap the trigger element and accept content directly, keeping UI and behavior the same.

  • Refactors

    • New API: {trigger} with auto-generated ids.
    • Removed data-tooltip-id and id usage across components.
    • Preserves existing options (place, className, style, clickable, delays).
    • Simplified helpers (e.g., InfoHover now accepts msg as a string).
  • Migration

    • Wrap the target element with instead of pairing an element with data-tooltip-id and a separate .
    • Move any previous tooltip children into the content prop.
    • Update InfoHover to pass msg as a string.

@uinstinct uinstinct requested a review from a team as a code owner August 20, 2025 12:16
@uinstinct uinstinct requested review from tingwai and removed request for a team August 20, 2025 12:16
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Aug 20, 2025
Copy link
Collaborator

@tomasz-stefaniak tomasz-stefaniak left a comment

Choose a reason for hiding this comment

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

Just a sanity check - have you verified that this doesn't change the layout? It could, result in a slightly different HTML and alignment issues, I want to make sure it's been visually inspected before merging.

@uinstinct
Copy link
Contributor Author

Just a sanity check - have you verified that this doesn't change the layout? It could, result in a slightly different HTML and alignment issues, I want to make sure it's been visually inspected before merging.

Yes! I checked most of them including simple ones, nested divs inside tooltips, hidden properties, clickables, etc.
All seem to be working fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants