design-system icon indicating copy to clipboard operation
design-system copied to clipboard

Add new tooltip component to DS

Open contolini opened this issue 1 year ago • 1 comments

We have two cf.gov apps that use tooltips but limited documentation on best practices or how to use them in new projects. This PR adds one implementation to the DS. It requires a third party library called Tippy.js.

It requires code in the following format:

Some sentence that requires additional clarity <span data-tooltip="relevant-tooltip">{% include 'icons/help-round.svg' %}</span>
<template id="relevant-tooltip">
    <div class="tippy-heading">Here's the tooltip heading</div>
    <div class="tippy-body">And the tooltip body</div>
</template>

Creating the CFPB theme requires augmenting the tippy-* classes. There's no CSS that's usable without the Tippy library so I didn't create an o-tooltip organism.

Testing

  1. Visit the tooltip page at the PR preview website.

Screenshots

tooltip

Notes

  • It doesn't feel great adding Tippy.js to the main package.json when its code will be used on few pages.

Todos

  • Add more documentation about usage and design guidelines.

Checklist

  • [ ] PR has an informative and human-readable title
  • [ ] Changes are limited to a single goal (no scope creep)
  • [ ] Code can be automatically merged (no conflicts)
  • [ ] Code follows the standards laid out in the CFPB development guidelines
  • [ ] Passes all existing automated tests
  • [ ] Any change in functionality is tested
  • [ ] New functions are documented (with a description, list of inputs, and expected output)
  • [ ] Placeholder code is flagged / future todos are captured in comments
  • [ ] Visually tested in supported browsers and devices (see checklist below :point_down:)
  • [ ] Project documentation has been updated
  • [ ] Reviewers requested with the Reviewers tool :arrow_right:

Testing checklist

Browsers

  • [ ] Chrome on desktop
  • [ ] Firefox
  • [ ] Safari on macOS
  • [ ] Edge
  • [ ] Safari on iOS
  • [ ] Chrome on Android

Accessibility

  • [ ] Keyboard friendly
  • [ ] Screen reader friendly

Other

  • [ ] Is useable without CSS
  • [ ] Is useable without JS
  • [ ] Flexible from small to large screens
  • [ ] No linting errors or warnings
  • [ ] JavaScript tests are passing

contolini avatar Sep 18 '24 14:09 contolini

Thanks for the improvements! Browse a preview of your changes using the link below.

Name Link
Latest commit 0a07cbc032f5fad4e56b13e848eb670217e90565
Latest deploy log https://app.netlify.com/sites/cfpb-design-system/deploys/674f5c95202b48000879b646
Deploy Preview https://deploy-preview-2054--cfpb-design-system.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Sep 18 '24 15:09 netlify[bot]

@anselmbradford after trying a handful of techniques to conditionally load the tooltip third-party dependency only when a tooltip is used, I ultimately decided to not bundle the tooltip code with the rest of the DS and instead added it as a separate entrypoint using the exports field. This allows developers to selectively add tooltips to their project by importing @cfpb/cfpb-design-system/tooltips. I added a note to the variation code snippet with usage instructions.

contolini avatar Oct 24 '24 15:10 contolini

When resolving conflicts, you'll want to delete the npm-packages-offline-cache folder.

anselmbradford avatar Oct 29 '24 17:10 anselmbradford

Should we have a cursor change on hover?

anselmbradford avatar Oct 29 '24 17:10 anselmbradford

If JS is off, it seems like the (?) icons should be hidden.

anselmbradford avatar Oct 29 '24 17:10 anselmbradford

Should we have a cursor change on hover?

I say no because a pointer cursor indicates something will happen on click (and nothing does in our case).

If JS is off, it seems like the (?) icons should be hidden.

I originally didn't do this because I figured there are scenarios where a user might want some words in the middle of a sentence to trigger a tooltip and it'd be bad if those words disappeared when JS is disabled. But our guidance explicitly states that help icons should trigger them so I'll add this feature and include a note about it in the guidance section.

contolini avatar Oct 31 '24 00:10 contolini

I say no because a pointer cursor indicates something will happen on click (and nothing does in our case).

If you're hovered on the (?), the tooltip shows. If you then click the (?), the tooltip is dismissed—so that's the action I was thinking the cursor change would indicate.

If not a cursor change, should there be a hover change on roll over? Or is that too button like? I noticed https://designsystem.digital.gov/components/tooltip/ has both a hover color change and a cursor change, but they also show the tooltip on buttons, so I'm unclear if that's completely covered by the buttons themselves, or if some of that is for the tooltip. Maybe @jenn-franklin can tell us how it should be and that's that.

anselmbradford avatar Oct 31 '24 02:10 anselmbradford

:+1: to a cursor change + click to close. No hover change. Thanks for calling this out!

jenn-franklin avatar Nov 01 '24 14:11 jenn-franklin

Just in case you missed it—npm-packages-offline-cache still needs to be deleted.

anselmbradford avatar Nov 05 '24 15:11 anselmbradford