springload / react-portal-popover

A popover library for React, using Portals for better positioning.
https://springload.github.io/react-portal-popover/
MIT License
21 stars 5 forks source link

Conditional tooltips and Multiple tooltips #8

Open oyeanuj opened 7 years ago

oyeanuj commented 7 years ago

Hi @JoshBarr @thibaudcolas! I had a couple of questions as I was trying to see if this would work for my needs -

  1. For some of my usecases, like showcasing errors in forms, I need to popovers/tooltips to show up only on certain conditions. Is it possible to do so with this library?

  2. Are there any gotchas in case one uses multiple tooltips, like in your demo? Do you recommend using the same popover/tooltip component or new one for each case?

Thank you for open sourcing this library!

thibaudcolas commented 7 years ago

Hey @oyeanuj, I'll try my best to answer those. My answers come with the caveat that I haven't used that many other react tooltip/popover libraries, so I'm not sure what's out there.

  1. Conditional rendering

One of the fundamental principles of this component is that you don't have programmatic control over whether the tooltip is shown or not – this is all handled for you. It is great for simple use cases where you just want the tooltip to show on clicks, but if you want to programmatically decide whether the tooltip should open itself (something like showTooltip: true in a setState), it won't be possible.

For the forms, would it be enough to conditionally render the whole container?

{showTooltip ? (
      <OverlayTrigger overlay={toolTip} label={'Excerpt'} showLabel={'Show'} hideLabel={'Hide'}>
        <button>Toggle</button>
      </OverlayTrigger>
) : null}
  1. Multiple tooltips

Honestly I haven't used this enough to respond on that one. This tooltip is used in production on some of our sites, but it is by no means as battle tested as other popular React components.

For your point about reusing tooltips, here there is no reuse – the tooltips are always separate. I'm not sure whether there is a tradeoff we've made here.

Hope this helps!

oyeanuj commented 7 years ago

@thibaudcolas Thank you for the detailed response!

On conditional rendering, unfortunately, I'd like the button always rendered and tooltip show up on error, something like this example.

screen shot 2016-08-31 at 4 39 08 pm

I wonder, given that you are using React-Portal, your thoughts on exposing the three ways React-Portal allows -

  • can be opened by the prop isOpened
  • can be opened after a click on an element that you pass through the prop openByClickOn (and then it takes care of the open/close state)
  • provides its child with this.props.closePortal callback

I understand, that you want this library to handle all the details for the developer, but if there is a way to allow developers to override those options, then it could allow for another set of possibilities :)

thibaudcolas commented 7 years ago

Hey @oyeanuj – sorry for the late reply. This makes perfect sense to me, I think it would be a great addition.

I have to say that even us don't use this component often enough in our projects to justify working on this (at least for now). If you or someone else wants to pick this up however, I'm more than happy to receive a PR and review / collaborate on it..

oyeanuj commented 7 years ago

@thibaudcolas thanks for responding! I'll try to give it a shot sometime soon. I'm curious if you prefer any one of the three options above? And if you have any additional tips/guidance?

thibaudcolas commented 7 years ago

Hmmm I think the easiest and most interesting would be passing isOpened through, since this is what gives the most control to the end user.

This could be as simple as setting open is the OverlayTrigger state according to a prop, updating the state when new props are received. https://github.com/springload/react-portal-popover/blob/master/src/components/OverlayTrigger.js#L8

oyeanuj commented 7 years ago

@thibaudcolas Thanks for the tip! And if I want it to show up on hover, etc, I imagine OverlayTrigger is where that would go, similar to how onClick is implemented?

thibaudcolas commented 7 years ago

Yup!