Skip to content
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

Add new anchorSelect prop to improve how to attach multiple anchors to the same tooltip #928

Merged
merged 25 commits into from
Feb 16, 2023

Conversation

gabrieljablonski
Copy link
Member

@gabrieljablonski gabrieljablonski commented Jan 30, 2023

Turns out there might've been a much much easier way to do multiple anchors than using the provider. anchorSelect should be a CSS selector to find the anchor elements.

The aim of this pull request:

  • Allow attaching multiple anchor elements to the same tooltip using simply a CSS selector.
  • Maybe deprecate anchorId and point to anchorSelect (keeping both as regular props might be fine as well).
  • If the provider/wrapper method doesn't prove to offer anything more than using selectors, we should deprecate it and point users to the anchorSelect prop (it's important to keep the API so code using the provider/wrapper still works).
  • Warn in non-production environment if selector doesn't match anything/is invalid. Fail silently in production.
  • Evaluate viability for a default selector such as [data-tooltip-for='${tooltipId}']. This way we could have something like v4's data-for attribute.
  • Explain on the getting started page very clearly that anchorId and the provider and wrapper have been deprecated.
  • Update docs to use mainly anchorSelect and data-tooltip-for, while pointing out very clearly that anchorId has been deprecated.
  • Add "Provider/Wrapper (DEPRECATED)" page on the examples section.
  • Update the "Upgrade Guide" section to reflect these new changes, which are very relevant for v4 users
  • Add "Anchor select" page on the examples section.
  • Add link to docs page on @deprecated tags.

Usage might look something like this:

<>
  <span 
    className="react-tooltip-anchor" 
    data-tooltip-content="I am an anchor element!"
  >
    Anchor 1
  </span>
  <span 
    className="react-tooltip-anchor" 
    data-tooltip-content="So am I!"
  >
    Anchor 2
  </span>
  <span 
    className="react-tooltip-anchor" 
    data-tooltip-content="Don't forget about me!"
  >
    Anchor 3
  </span>
  <Tooltip anchorSelect=".react-tooltip-anchor" />
</>

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
@gabrieljablonski gabrieljablonski changed the title Add new anchorSelect to improve how to attach multiple anchors to the same tooltip Add new anchorSelect prop to improve how to attach multiple anchors to the same tooltip Jan 30, 2023

Verified

This commit was signed with the committer’s verified signature.
pacrob Paul Robinson
@gabrieljablonski
Copy link
Member Author

gabrieljablonski commented Feb 2, 2023

The code is slightly messy (but still clean enough) in some places given we shouldn't simply remove the provider and anchorId functionalities. Maybe we could drop it in a future version after some time has passed and most people are already aware of the deprecation warning these changes will trigger.

Now we just have to update the docs. This will take a bit of work since I'd like to change all anchorId to anchorSelect/data-tooltip-for. The "getting started" and the main example pages should have a "warning" style banner indicating anchorId has been deprecated.

@gabrieljablonski gabrieljablonski self-assigned this Feb 2, 2023
@gabrieljablonski gabrieljablonski linked an issue Feb 2, 2023 that may be closed by this pull request
@danielbarion
Copy link
Member

Also, probably when we remove the anchorId prop, we will need to raise the major version as this is a breaking change.

Nice!

@gabrieljablonski gabrieljablonski marked this pull request as ready for review February 14, 2023 16:22
@danielbarion
Copy link
Member

Looks good to me 🚀

@gabrieljablonski gabrieljablonski merged commit ec02ff0 into master Feb 16, 2023
@gabrieljablonski gabrieljablonski deleted the feat/anchor-selector branch February 16, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT REQ] Anonymous multiple anchors in v5
2 participants