Prevent default/stop propagation on Tooltip.disclosure click

[caseyWebb]

🔧 Modifying a component

Context

This PR changes the default click behavior for disclosure tooltips to preventDefault and stopPropagation on click. This allows nesting Tooltips within clickable regions such.

https://noredink.slack.com/archives/C02NVG4M45U/p1692639132232799

Closes KRA-1114

Component completion checklist

• I've gone through the relevant sections of the Development Accessibility guide with the changes I made to this component in mind
• Changes are clearly documented
  • Component docs include a changelog
  • Any new exposed functions or properties have docs
• Changes extend to the Component Catalog
  • The Component Catalog is updated to use the newest version, if appropriate
  • The Component Catalog example version number is updated, if appropriate
  • Any new customizations are available from the Component Catalog
  • The component example still has:
    • an accurate preview
    • valid sample code
    • correct keyboard behavior
    • correct and comprehensive guidance around how to use the component
• Changes to the component are tested/the new version of the component is tested
• Component API follows standard patterns in noredink-ui
  • e.g., as a dev, I can conveniently add an nriDescription
  • and adding a new feature to the component will not require major API changes to the component
• If this is a new major version of the component, our team has stories created to upgrade all instances of the old component. Here are links to the stories:
  • not a major version

[tesk9]

Code looks good!

Could you please finish filling out the checklist, though? thanks!

[bcardiff]

I think all the remaining checkboxes can be checked at this point.

elm-explorations unfortunately does not emulates even bubbling. It's impossible to elm-test the actual behaviour.
While pairing on this I thought that adding some behavioral/puppeteer(?) specs would be best. But we would need a small custom app to do that.

I don't think is mandatory for this PR to address that, but might be good to have some direction to have that kind of test. puppeteer/playwright/cypress. WDYT @tesk9 ?

[tesk9]

You are very welcome to add puppeteer tests!

The existing tests are primarily for running axe and percy against every example, but it's easy to amend the code to include additional assertions for a particular component. (You would want to adjust the component catalog example to include an example of the card pattern, I think, in order to test this properly).

Run script/puppeteer-tests.sh Tooltip locally without script/develop.sh running to avoid running all the puppeteer tests.

Add Tooltip to:

const specialProcessing = {
    Message: messageProcessing,
    Modal: modalProcessing,
    Page: pageProcessing,
    AssignmentIcon: iconProcessing,
    UiIcon: iconProcessing,
    Logo: iconProcessing,
    Pennant: iconProcessing,
  };

in order to write custom tests against the Tooltip component. Please be sure that everything that is tested in defaultProcessing is also tested in your new tests.

[bcardiff]

Can we create custom elm apps for this? Having a sample app that will count/show parent click on disclosure tooltip does not seem a good fit for the catalog. Where could that app be placed?

[tesk9]

Having a sample app that will count/show parent click on disclosure tooltip does not seem a good fit for the catalog.

I disagree! I think that if there are common patterns in which components are used, and we want to make sure that these patterns continue to work, the Component Catalog is a great place to do so.

If you don't think that the Tooltip example is where a clickable-card-with-tooltip-disclosure example fits, I think another alternative is to add a new page that is specifically for this pattern.

I made a PR showing how this could be done: #1487

[tesk9]

(I'll be OOO for the next few days. If you need a11ybat input, please talk to Charbel and Kristine 🙏)

[bcardiff]

I went on the route of adding a clickable container in the existing example to keep things simpler. But I think it would be good addition the PR to test interactions between components.

Currently it will look like this, and the counter allow us to test things via puppeteer.

Something else I notices is that puppeteer is VERY outdated. 13.0.1 is 2yrs old and the current release is 21.x. Something to have in mind in the future.

[bcardiff]

@caseyWebb do you agree with the latest push?

[caseyWebb]

I think this is good for now.

Regarding puppeteer being so out of date, I wonder what the upgrade path there is and if we're using puppeteer anywhere else–if this is the only place it's used (and there is significant upgrade effort) I wonder if any effort should be put into migrating them to cypress instead of upgrading puppeteer. Not relevant for this pr, but a thought.

[krinhorn]

(Looping in Charbel on the Puppeteer update thing separately - thanks much for raising!)

[bcardiff]

I don't mind if we use puppeteer/cypress/playwright. I am not that experienced in either 😅 . My problem with the current state is that some documentation I found online was not working with 13.x since it's so outdated.

Let's merge this as is and discuss/prioritize a migration path.

[krinhorn]

#1490 - Feel free to edit/comment/work on that issue

[linear]

KRA-1114 Make preventDefault/stopPropagation default behavior for Tooltip.disclosure