tippyjs

tippyjs copied to clipboard

Problem

In my experience, the only reason for surfacing an onClickOutside prop to the user, is so they can fire a callback to close the modal if they're controlling the visibility of it directly.... that's generally the primary reason this prop is ever used.

If you consider the context of most popovers (sort of mini modals) on most websites, you can either close it by hitting some sort of x icon in the modal, clicking outside of the modal, OR pressing the esc key. I'd like to argue that having a prop just for clicking outside, without it also taking esc into account (or providing a different prop for it) doesn't cover the majority use case of why this prop would be desirable to use in the first place.

https://atomiks.github.io/tippyjs/v6/plugins/#hideonesc

I'd like to draw attention to this hide on esc plugin. "currently", if you try to use this plugin if you're manually controlling the visibility of the popover, esc obviously won't update the underlying state that you're passing:

https://codesandbox.io/s/focused-sammet-spk1o?file=/src/App.js

Though it will still hide the popover. As a result (refer to my codesandbox) you show the popover, you hide the popover with esc, and when you click the button it won't show the popover until you click it a 2nd time.

Now technically, this can be solved by creating a function which accepts this callback, and then generating the plugin from there, using useMemo or something. But again if we assume anyone can add any functionality they want, if we consider this prop more or less for convenience, I think 99% of the time people will want this in situations where they're passing a boolean to visibility directly and they want to update that boolean on outside click or esc press. I think the current ergonomics for this, assuming my assertion is correct, is a bit cumbersome.

(As a side note, I "do" think it's somewhat awkward that you can hide tippy even though there's a direct boolean being applied. hide on esc on my example probably should only fire a callback to change the boolean itself and that's it).

Solution

Any of the following would do:

• onClickOutside triggers on esc press (rename or not, doesn't matter to me)
• Separate prop for esc press
• new prop that triggers onClickOutside and on esc press.

For reference, here's what it looks like if you create the implementation yourself using onClickOutside and the hideOnEsc.

https://codesandbox.io/s/trusting-euler-1dck2?file=/src/App.js

There's a bunch of not so convenient logic to create for what could be interpreted as happy path functionality. So this feature request isn't necessary granting something someone can't do, but I think it would potentially improve DX and guarantee people default to doing what is good user experience, more commonly.

It looks like being able to dismiss a tippy using esc is required by WCAG and so this could possibly live in the aria object as a new option.

Looking at the react-onclickoutside package, "click outside" doesn't encompass the esc key, so I'm not sure if it makes sense to invoke onClickOutside when pressing esc. More like onDismiss, but I think it's too late for that name.

Something like this could work:

aria: {
  esc: boolean | Function
}

If you pass a Function, it's what gets called when esc is pressed, allowing the Controlled Mode tippy to update the state.

const [visible, setVisible] = useState(false);
return (
  <Tippy visible={visible} aria={{esc: () => setVisible(false)}}>
    <button onClick={() => setVisible(v => !v)} />
  </Tippy>
);

Why do you say it's too late for an onDismiss function? It seems like if you add that, it can be called in all the places and scenarios that onClickOutside is called, and then also add the ESC functionality, and then if someone wants to do something special, they can use onClickOutside and then the aria prop

Too late for the current major version*, but if we can streamline the lifecycle hooks/callbacks for v7 then it can be added: https://github.com/atomiks/tippyjs/discussions/991