Skip to content

[BUG] when first rendered, Tooltip component does not recognize existing anchors #1020

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

Closed
yurigenin opened this issue May 3, 2023 · 12 comments
Labels

Comments

@yurigenin
Copy link

Describe the bug
This worked in v4 of the component.
I want to be able to create a Tooltip component dynamically per anchor only when the mouse enters the anchor, and destroy it when the mouse leaves it. I want to create it dynamically because every tooltip instance should support unique set of property values, so I cannot use just one shared instance. But I do not want to always create a tooltip instance when I create an anchor as it will have performance issues. I had the following logic that worked in v4 (this is my custom AnchorWithTooltip component):

render() {
<>
        <div
          onPointerEnter={() => {
              this.setState({isShowing: true});
          }}
          onPointerLeave={() => {
              this.setState({isShowing: false});
          }}
          data-tooltip-id={uniqueAnchorId}
        >
          {children}
        </div>
        {isShowing && (
            <Tooltip
              id={uniqueAnchorId}
              content={this.getContent()}
            >
            </Tooltip>
        )}
      </>
}

What happens here is that when the mouse enters the anchor, the isShowing state property is set to true, so the Tooltip component renders. When the mouse leaves the anchor, the isShowing state property is set to false and the Tooltip component is unmounted.
When I looked at the code of the TooltipController, it looks like it does not recognize the pre-existing anchor associated with the Tooltip by id so the tooltip is not shown.
I realize that this is probably by design but I would really appreciate if you could make the necessary change to enable such behavior.
And thanks again for refactoring the component. It looks great.

Version of Package
latest

To Reproduce
See the explanation above

Expected behavior
See the explanation above

Desktop (please complete the following information if possible or delete this section):

  • OS: [MacOS]
  • Browser [all]
  • Version [latest]
  • Frameworks [React 18]
@yurigenin yurigenin added the Bug label May 3, 2023
@gabrieljablonski
Copy link
Member

gabrieljablonski commented May 3, 2023

Independently of when it gets rendered, the tooltip should recognize existing anchors, so I'm not sure what might be the problem in your case.

Anyway, I strongly encourage you to use a set amount of tooltip instances (i.e. don't render them dynamically alongside your items), since although it might seem like it will be easier to render different content for each item this way, you're likely to run into other problems later.

I want to create it dynamically because every tooltip instance should support unique set of property values, so I cannot use just one shared instance.

As I believe you've already seen, there are some features such as the render prop which help in rendering completely different content using the same tooltip instance (or a small set of predefined ones).

@yurigenin
Copy link
Author

yurigenin commented May 3, 2023

@gabrieljablonski , the problem I'm trying to solve here is actually due to a different issue. The way the current component is designed, it does not re-render when the anchor width/height changes. You see, the tooltip is often shown dynamically based on whether the anchor DOM box fits the text. For instance, initially a grid cell may be wide enough to fit the entire text but when a user makes the grid column narrower, the grid cell does not fit that text any longer so a tooltip showing the entire text is desirable. Tooltip component currently only rerenders/retrieves its content when any of the data-tooltip-* attributes change. In my case, I want to dynamically set the tooltip content based on the width of the anchor which would trigger the tooltip on or off. The way I solved it before I described above by rendering the tooltip component dynamically based on mouse entering/leaving the anchor. I think if you could trigger the recalculation of the content of the tooltip when the mouse enters/leaves the anchor, it would solve my use case.
Another issue I noticed is that currently the tooltip component preemptively retrieves the content of the tooltip for each associated anchor, not when the mouse first enters the anchor which is also inefficient since in my case if I associate one instance of the tooltip component with all anchors it may be a big performance hit.
Please let me know if my explanation is clear.

@gabrieljablonski
Copy link
Member

gabrieljablonski commented May 4, 2023

Just to make sure I understand correctly.

You have an anchor element which should have a tooltip only when its width is smaller than a certain threshold, correct?

Here's an idea that might work (click on the text to toggle the width)
https://stackblitz.com/edit/react-ts-o6139e?file=App.tsx

You can just set data-tooltip-content={''} when you don't want the tooltip to show. In the near future, we might have a special data-tooltip-* attribute to hide the tooltip explicitly, as #1003 suggests.

@yurigenin
Copy link
Author

yurigenin commented May 4, 2023 via email

@gabrieljablonski
Copy link
Member

The issue I still have is that data-tooltip-content or data-tooltip-html only take a string but I need to render React elements as content sometimes.

Check this brief example on the README. renderToStaticMarkup() should be able to render any React component to a string.

@yurigenin
Copy link
Author

yurigenin commented May 4, 2023 via email

@yurigenin
Copy link
Author

To follow up on my previous comment, the reason the tooltip does not pop up when created dynamically when the mouse enters an anchor associated with it through its id is because the anchor's mouseenter event handler is only set up when the Tooltip is first rendered:

https://github.com/ReactTooltip/react-tooltip/blob/master/src/components/Tooltip/Tooltip.tsx#L335

By that time, it is already too late since I use the mouseenter event on the anchor to render the Tooltip. Thus, upgrading from version 4 to version 5 does not work for me, unfortunately.

And I cannot use a global instance of Tooltip because I have to support the content which is of type ReactNode|ReactFragment that cannot be set on data-tooltip-html property.

I can use a separate instance of Tooltip per anchor but if every cell of my datagrid is a potential anchor it creates a performance nightmare when my datagrid has hundreds of cells rendered.

@yurigenin
Copy link
Author

@gabrieljablonski , I think my use case would be resolved if the TooltipProvider/TooltipWrapper pattern (which is now marked deprecated for some reason) added a setContent method to the React context (useTooltip) which I could call with a new content in my own mouseenter event handler added to an anchor. setContent method would cause rerendering of the global tooltip.

@gabrieljablonski
Copy link
Member

Yes but it still does not resolve the case of rendering the type ChildrenType (Element | ElementType | ReactNode) which is the render's method return type

Not sure what you mean. You should either use data-tooltip-html by itself, or the render prop. I don't think you'd achieve anything useful by using both.

the reason the tooltip does not pop up when created dynamically when the mouse enters an anchor associated with it through its id is because the anchor's mouseenter event handler is only set up when the Tooltip is first rendered

It wasn't clear before, but now I understand the issue, and there's not much that can be done about this. At the instant you render the tooltip component, there's no way to know if the user is currently hovering an anchor element. It could be almost achieved by using mousemove alongside mouseenter, but that's far from ideal, and sure to introduce a whole set of other problems.

And I cannot use a global instance of Tooltip because I have to support the content which is of type ReactNode|ReactFragment that cannot be set on data-tooltip-html property.

I don't understand what you mean. You should be able to do something like this without any problems. <MyTooltipContent /> is a React component.

<span
  data-tooltip-id="my-tooltip"
  data-tooltip-html={ReactDOMServer.renderToStaticMarkup(<MyTooltipContent />)}
>
  my cell content
</span>
<Tooltip id="my-tooltip" />

I can use a separate instance of Tooltip per anchor

Definitely don't.

I think my use case would be resolved if the TooltipProvider/TooltipWrapper pattern (which is now marked deprecated for some reason)

We've decided to drop support for this feature since it is very cumbersome to use, and adds a ton of unnecessary complexity to the component. It will be removed in a future version.


My suggestions:

  1. Make sure ReactDOMServer.renderToStaticMarkup() doesn't work for you (I honestly have no idea why it wouldn't though, I use it without any issues on my personal projects with decently complex React components as the content)
  2. Are you REALLY SURE that ReactDOMServer.renderToStaticMarkup() doesn't work for you? If that's the case, you can control the tooltip state manually (have a look at the docs). The idea is that you can set the isOpen state manually on your onPointerEnter and onPointerLeave event handlers.

But IMO you should really avoid controlling the state manually, as it is likely to give you other headaches in the future.

@yurigenin
Copy link
Author

yurigenin commented May 4, 2023 via email

@gabrieljablonski
Copy link
Member

If I want to replace render/children property on the Tooltip with a data-tooltip-html I would have to do:
data-tooltip-html={renderToStaticMarkup(ChildrenType)} which does not work

Just wrap it with an HTML element.

const content: ChildrenType = ...
<span
  ...
  data-tooltip-html={renderToStaticMarkup(<div>{content}</div>)}
/>

@gabrieljablonski
Copy link
Member

If this is still a problem, feel free to reopen with more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants