feat: tooltip with support for renderprops

[HendrikThePendric]

The purpose of this PR is mainly to demonstrate a generic approach to dealing with components that need to be able to render any content, but do require this content to implement certain props.

This is an example for the Tooltip, but we see the same/a very similar problem appear in the Menu/SubMenu too.

The main idea is that we offer two methods of rendering:

• For default scenarios we accept children in the normal way (CONVENIENT)
• And if further customization is required we switch to the renderProps approach (FLEXIBLE)

So for the Tooltip this means:

• Normally children are simply wrapped in a span and the ref, OnMouseOver, onMouseOut props are attached to that in the Tooltip. If the app wants to use a different tag, this can be customised via the tag prop (see demo CustomElementViaTagProp)
• If further customisation is needed, the app can switch to renderProps (see CustomBuiltInComponent and CustomComponent)

An important note for that last example, CustomComponent:
In the example you see that I am adding a wrapper around the Button. This is for two reasons:

1. The Button is currently not able to hold a ref. But we can see in @dhis2/ui that this is going to change...
2. The Tooltip anchor needs to implement not only a ref, but also a onMouseOver and onMouseOut, and our Button has not implemented those methods either.

There's nothing wrong with a wrapper element per se, but if we look into the future, i.e. @dhis2/ui , where the *Base components can all hold a ref and forward props, we could also solve this in a different way, using the ButtonBase:

quick and dirty:

<p>
    I am a{' '}
    <Tooltip content="Some extra info">
        {tooltipProps => <ButtonBase {...tooltipProps}>paragraph</ButtonBase>}
    </Tooltip>{' '}
    that contains a Button with a tooltip.
</p>

dedicated component:

const TooltipButton = ({ children, content, placement, onClick }) => (
    <Tooltip content={content} placement={placement}>
        {tooltipRenderProps => (
            <ButtonBase {...tooltipRenderProps} onClick={onClick}>
                {children}
            </ButtonBase>
        )}
    </Tooltip>
)

<p>I am a paragrapg that contains a <TooltipButton content="Some extra info">tooltip button</TooltipButton>.</p>

If I've understood the feedback I've received recently correctly, then one of the main problems that has been mentioned is the lack of transparency. Does the current approach with renderProps solve that in your opinions?

[Mohammer5]

I like this approach more than the current one, but it made me think about a scenario, that I'd like even more:

export const CustomComponent = () => {
    const { ref, onMouseOver, onMouseOut, showTooltip } = useTooltip()

    return (
        <p>
            I am a{' '}
            <span ref={ref} onMouseOver={onMouseOver} onMouseOut={onMouseOut}>
                <Button>paragraph</Button>
            </span>
            that contains a Button with a tooltip.

            <Tooltip show={showTooltip} anchor={ref}>Some extra info</Tooltip>

            <style jsx>{`
                span {
                   display: inline-flex;
                }
            `}</style>
        </p>
    )
}

[HendrikThePendric]

That's quite an interesting idea.... The biggest advantage would be that both the Tooltip and the anchor element could just render children. To me the downside is that you sort of loose the visual connection/relationship between the tooltip and the anchor...

I'd be interested to find out what @ismay and @varl think.

One minor detail that I'd like to change from your suggestion is how we actually toggle the Tooltip visibility. I'd just use conditional rendering:

{showTooltip && <Tooltip anchor={ref}>Some extra info</Tooltip>}

[HendrikThePendric]

🤔 after reviewing a number of other react tooltip implementations, it does seem to be that the approach I went with initially, i.e. wrapping the anchor in the tooltip seems to be the most common pattern. So, perhaps that'd be an argument for sticking to the current approach.

[varl]

If the hook-based approach allows us to use any component (HTML or React) as an anchor, and we can ignore the display mode (inline vs. block) then I like that approach.

[Mohammer5]

@varl yeah it'd work with any element (built-in or custom).

I'd like to slightly change what I've proposed though:

export const CustomComponent = () => {
    const { ref, onMouseOver, onMouseOut, tooltip } = useTooltip()

    return (
        <p>
            I am a{' '}
            <span ref={ref} onMouseOver={onMouseOver} onMouseOut={onMouseOut}>
                <Button>paragraph</Button>
            </span>
            that contains a Button with a tooltip.

            <Tooltip {...tooltip}>Some extra info</Tooltip>

            <style jsx>{`
                span {
                   display: inline-flex;
                }
            `}</style>
        </p>
    )
}

This way we can add/remove props we want/need to pass to the tooltip without breaking apps.
Apps also don't have to care about open/close states anymore.

---

To me the downside is that you sort of loose the visual connection/relationship between the tooltip and the anchor...

The tooltip could be placed inside the anchor. It's rendered in a portal anyways, isn't it?

export const CustomComponent = () => {
    const { ref, onMouseOver, onMouseOut, tooltip } = useTooltip()

    return (
        <p>
            I am a{' '}
            <span ref={ref} onMouseOver={onMouseOver} onMouseOut={onMouseOut}>
                <Button>paragraph</Button>
                <Tooltip {...tooltip}>Some extra info</Tooltip>
            </span>
            that contains a Button with a tooltip.

            <style jsx>{`
                span {
                   display: inline-flex;
                }
            `}</style>
        </p>
    )
}

[HendrikThePendric]

If the hook-based approach allows us to use any component (HTML or React) as an anchor, and we can ignore the display mode (inline vs. block) then I like that approach.

@varl but so would the renderProps approach. These two alternatives are completely on par in terms of that so it's just a matter of which markup makes more sense.

[HendrikThePendric]

Let's make the choice fully explicit. The following examples are functionally identical and just use slightly different syntax:

OPTION A:

export const CustomComponent = () => (
    <p>
        I am a{' '}
        <Tooltip content="Some extra info">
            {tooltipAnchorProps => (
                <span {...tooltipAnchorProps}>
                    <Button>paragraph</Button>
                    <style jsx>{`
                        span {
                            display: inline-flex;
                        }
                    `}</style>
                </span>
            )}
        </Tooltip>{' '}
        that contains a Button with a tooltip.
    </p>
)

OPTION B:

export const CustomComponent = () => {
    const { ref, onMouseOver, onMouseOut, tooltip } = useTooltip()

    return (
        <p>
            I am a{' '}
            <span ref={ref} onMouseOver={onMouseOver} onMouseOut={onMouseOut}>
                <Button>paragraph</Button>
                <Tooltip {...tooltip}>Some extra info</Tooltip>
            </span>{' '}
            that contains a Button with a tooltip.

            <style jsx>{`
                span {
                   display: inline-flex;
                }
            `}</style>
        </p>
    )
}

[ghost]

So I minimized the above comment after talking a bit about the implementation with @HendrikThePendric since I see things a bit differently now. So, the main difference between the two proposals seems to be if we use renderProps or hooks. Functionally they basically allow the same thing. But that's not the crux of the matter maybe, I'll elaborate below.

I looked around and found this https://kentcdodds.com/blog/react-hooks-whats-going-to-happen-to-render-props, and that led us to talking about whether here the renderProp pattern fits with the usecase where Kent still recommends renderProps. And I agree with @HendrikThePendric, seems like renderProps (or function as a child in this case) are a nice fit here. If I understand correctly, we'll have:

1. A convenient option for users that just need a default Tooltip. They can pass elements as children and just use the defaults.
2. For those who need more customization, they can pass a function and have more freedom in what gets rendered exactly. So an escape hatch for more complicated situations. That seems quite user friendly to me.

Whereas with the hooks they'll have to always use the hook, even though they might just want a regular tooltip. That would probably lead to them creating a default tooltip locally for the regular tooltips, which seems like duplicated effort. Plus, none of our other components are currently hooks, so it would be a bit uncommon for our lib atm.

So long story short, I see @HendrikThePendric's point. I think that his suggestion (option A) would be what I'd prefer from the perspective of a dev using this component.

[cypress]

---

Test summary

258 • 0 • 0 • 0

---

Run details

Project	ui-core
Status	Passed
Commit	598c981
Started	Mar 25, 2020 11:40 AM
Ended	Mar 25, 2020 11:48 AM
Duration	07:38 💡
OS	Linux Ubuntu Linux - 18.04
Browser	Electron 78

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[varl]

I think the style jsx tag needs to be moved to top-level in Option A, other than that good examples.

• A convenient option for users that just need a default Tooltip. They can pass elements as children and just use the defaults.

• For those who need more customization, they can pass a function and have more freedom in what gets rendered exactly. So an escape hatch for more complicated situations. That seems quite user friendly to me.

Given that these two are correct, I am swayed as well.

All examples here have omitted the imports which for Option A would be slightly simpler than Option B.

Will this decision affect other components as well? Seems like we are establishing a pattern here.

[Mohammer5]

Given that these two are correct, I am swayed as well.

Agreed. Good points @ismay !

[HendrikThePendric]

Will this decision affect other components as well? Seems like we are establishing a pattern here.

I was thinking about doing something similar for a nested Menu, so we end up with something as flexible as @Mohammer5 suggested on Slack, but a bit more convenient. I still have to work out what the best structure would be, but the problem we need to solve there is identical to the Tooltip:

• A MenuItem can be a custom component
• A MenuItem can also be the anchor element for a sub-menu
• We'll need a single ref that we can a) apply to the MenuItem, and b) pass to the Popper

So I'd say here renderProps will help too because it would offer an easy way to provide the ref to both of these...

Ideally I'd also like to end up with something similar to the Tooltip, so we have a convenient way to render a default MenuItem plus an escape hatch via renderProps for custom stuff.

[dhis2-bot]

🎉 This PR is included in version 4.16.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀