[WIP] chore(packaging): use explicit tippy.js imports and tooltip updates

[seanforyou23]

What: This PR makes a few enhancements around Popover and Tooltip. The main change here is removing the reliance on emotion to inject the styles, which, has the unfavorable side effect of polluting the DOM with individual style tags for each css scope. Beyond that, it's mostly a matter of making available a couple new options like animation, aria, etc.

FWIW this PR coupled along with patternfly/patternfly-react-seed#49 really puts a big dent in the number of inline styles loaded in the head section of a patternfly-based UI application 👍

Now we get nicely named stylesheets in the css inspector;

While writing examples to exercise these new API parts, an issue was surfaced with the way tippy now references child nodes. There is a note on exactly this limitation. Where it becomes a problem is when you go to utilize aria-labelledby, because the target element is always a span the tooltip text isn't read aloud as you would expect (in VO at least). I found that a way around this is to provide a role attribute to the tooltip content, this queues up the text and reads it after the target element's accessible name is read aloud. So it's more like an aria-describedby than anything, but at least it's not completely broken.

We have at least two options for how to address this. Either require users to use forwardRef for the components they want to wrap in tooltip/popover (and maybe we'd also have to use this same approach with any "wrappable" components in PF?) or we simply don't expose this specific API part and instead only offer the other two values as options (describedby or null).

I added some new examples for Tooltip to showcase some more specific usages.

I think with some more time a solution can be crafted to solve the auto-forwarding of refs, but maybe for now we can think about accepting these enhancements and strategize on sorting out the ref's in a follow-up PR? I could use some help thinking through this one.

Created a patch file that runs automatically as part of the build cycle to ensure we import the es6 module for tippy instead of what's exported by default, which includes the global stylesheet injection. There's also a node script that can do this for us, patch-package seems like a nice enhancement to the repo as a whole so I went with that.

remove reference to tippy styles
move tippy overrides to a dedicated stylesheet
create a patch file for tippy lib for sans auto-css-injection
add helper script for applying the update

Additional issues: #789 and #1655

[jschuler]

Should this file be copied here?

[seanforyou23]

Probably not :)

Yea I saw that, haven't gotten around to tracing it down yet. Still doing some work on this PR, will clean it up!

[seanforyou23]

Ok, so I've had a chance to remind myself about that file. Right, so basically it's being copied because there in the same way that tippy.css and tippyOverrides.css are being exposed as part of the react-core dist, for example;

This way, we're consuming these styles in react-docs the same way an end user would be, which is by a reference from base.css. We could do some swizzling to make this refer to the original course file for us, but not sure we want to do that? I also haven't checked out the developer experience of actually working on those styles, so I'll look into that as well to make sure there's a straight forward way to work on those styles without having to run a build every time.

In the meantime, let me know if you have a concern about exposing these tippy stylesheets like this. There are still several things left to do here so we definitely have time to talk it through if you have other ideas.

[patternfly-build]

PatternFly-React preview: https://patternfly-react-pr-2395.surge.sh

[seanforyou23]

There's also an open question as to which animation strategy we should use as a default. I picked scale for now, but that wasn't based on any other reason than its what I like 😁Maybe we can get some input on this? https://atomiks.github.io/tippyjs/#animations

[dgutride]

What are these new dependencies for?

[seanforyou23]

It automatically applies any patch files in the new "patches" directory at the root https://github.com/patternfly/patternfly-react/pull/2395/files#diff-1a027bc721cd1ce512f626ff77e9f5b3 - does it in a safe and predictable manner. It's basically a better alternative than using hand crafted scripts like https://github.com/patternfly/patternfly-react/pull/2395/files#diff-c5a66feeb31fe3511a6e4d157ef2bfb8 to apply changes to modules after they've landed in node_modules

[seanforyou23]

Oh right, and the whole yarn remove thing not working as expected for regular postinstall scripts https://github.com/ds300/patch-package#why-use-postinstall-postinstall-with-yarn

[seanforyou23]

Looks like even the workaround pattern for component children getting proper references doesn't always work - #2400

[jgiardino]

There was a discussion in a previous accessibility review meeting where we (including @matthewcarleton and @mcarrano) talked about the challenges of enabling tooltips for static elements (i.e. elements that don't normally receive keyboard focus, like buttons, links, form elements, etc...). We discussed not recommending tooltips for static elements in the design documentation. This recommendation is captured here: patternfly/patternfly-org#1108 (comment)

Based on that, can we remove any examples that show the tooltip on static elements (or at least not include new examples that show the tooltip on static elements)?

[seanforyou23]

We recently reverted the recent tippy upgrade which this PR was built on top of. I'm going to close this PR and open a new one with the comprehensive changes required for the upgrade. Thx for the feedback everyone!