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

Consider moving away from the "title" attributes due to conflict with tooltips #530

Open
xuhdev opened this issue Feb 28, 2024 · 6 comments · May be fixed by #531
Open

Consider moving away from the "title" attributes due to conflict with tooltips #530

xuhdev opened this issue Feb 28, 2024 · 6 comments · May be fixed by #531

Comments

@xuhdev
Copy link

xuhdev commented Feb 28, 2024

MDN doc says:

The HTMLElement.title property represents the title of the element: the text usually displayed in a 'tooltip' popup when the mouse is over the node.

However, react-share uses title for totally unrelated purposes IIUC. This makes it somewhat hard to put a tooltip with a different text on a share button in some cases.

My Particular Example

If using with MUI Tooltip, I would get the following warning:

MUI: You have provided a title prop to the child of <Tooltip />.
Remove this title prop Share this page or the Tooltip component.

Code:

              <Tooltip title="content of tooltip">
                <TwitterShareButton url="https://example.com" title="Share this page">
                  <TwitterIcon round />
                </TwitterShareButton>
              </Tooltip>

I see this warning legitimate due to the cited MDN doc above.

@xuhdev xuhdev changed the title Consider moving away from the "title" attributes for tooltips Consider moving away from the "title" attributes due to conflict with tooltips Feb 28, 2024
@nygardk
Copy link
Owner

nygardk commented Feb 28, 2024

Hey! Yeah, you're absolutely right, using title for this purpose was not a good decision 😕. It could be changed in a major release some time. However, it's such a small change that I'm not sure if it alone warrants a breaking change release. It's a bummer to hear about that problem with the MUI tooltip. Could there be any other workaround for now 😕?

@xuhdev
Copy link
Author

xuhdev commented Feb 28, 2024

Thanks for the quick reply. I don't think it'll be a breaking change -- what's needed is a change in the underlying implementation that forwards the title param and sets it as an HTML attribute. An easy (or maybe not-so-easy) way to fix it is to rename title to something else, like shareTitle when setting the HTML attribute. My gut feeling is that the fix can be applied some way around

/**
* Passes as the native `title` atribute for the `button` element.
*/
htmlTitle?: HTMLButtonElement['title'];
, but I'm not so sure. If you can provide some hints, I'm happy to send a PR.

I don't think the problem is only with the MUI Tooltip. As the MDN link suggests, using title as it is used now is incompatible with the standard and I would expect more problems with tooltips outside the MUI context.

For me, the best workaround is probably some patch that fixes the issue 😄

@nygardk
Copy link
Owner

nygardk commented Feb 28, 2024

Ahha, I see it now! I misunderstood you at first. Makes sense.

Please check #531 if you think it would fix the issue for you!

@xuhdev
Copy link
Author

xuhdev commented Feb 28, 2024

This doesn't solve the MUI warning but I saw the title prop has been removed from the button element 👍 . According to the MDN doc in the issue, I believe it is a proper fix on the react-share side -- looks like MUI also has a bug to properly detect title. I'll talk to them on their side 😄

Thanks for the fix!

@xuhdev
Copy link
Author

xuhdev commented Feb 28, 2024

Looks like, MUI Tooltip assumes that the component's props are spread to the underlying HTML element. It looks like the fix has to rename title to something else. I'm thinking that providing an alternative name to title might be a nice non-breaking fix, but it's up to you how much you would like to be compatible with MUI. If being compatible with MUI is not an objective, I'll figure out some other ways :)

Having said these, #531 is also a useful fix probably outside the MUI context.

@xuhdev
Copy link
Author

xuhdev commented Feb 28, 2024

To summarize, the whole story is:

  1. MDN doc says:

The HTMLElement.title property represents the title of the element: the text usually displayed in a 'tooltip' popup when the mouse is over the node.

  1. Therefore, MUI Tooltip uses title prop as the content of the tooltip.
  2. However, MUI Tooltip assumes that the component's props are spread to the underlying HTML element. Meanwhile, even after fix: prevent title from being passed to the button element #531, react-share uses title prop for some of the share buttons. This causes warnings when a share button's title is specified and is wrapped with a MUI Tooltip.

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 a pull request may close this issue.

2 participants