noredink-ui icon indicating copy to clipboard operation
noredink-ui copied to clipboard

Tooltip tail affixation

Open caseyWebb opened this issue 1 year ago • 3 comments

:wrench: Modifying a component

Context

This PR makes Nri.Tooltip tails always affix to the trigger element. While the API remains the same, this is a breaking change with regards to visual behavior.

The customOffset value passed to alignStart/alignEnd now defines the additional distance to offset the tail from the respective end, rather than the tooltip as a whole.

TODO:

  • components that rely on this and accept additional tooltip attributes should be versioned
  • custom offset should be negated automatically when it makes sense. a value of 100 should push the tail 100px inward (not arbitrarily right), regardless of positioning.

The changes are best demonstrated via video, see below:

:framed_picture: What does this change look like?

V3 (Current)

https://github.com/NoRedInk/noredink-ui/assets/5419074/21dbf312-e947-4684-b1aa-44dde90d77c3

V4 (New)

https://github.com/NoRedInk/noredink-ui/assets/5419074/0b7c5fb8-c348-49f9-b3a2-81509620d131

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:
    • add your story links here OR just write this is not a new major version

caseyWebb avatar Aug 22 '23 02:08 caseyWebb

@caseyWebb just to double-check, you're aware that if Kraken makes this change, Kraken will be responsible for updating all tooltip uses to the new version? The upgrade path here seems a little challenging, so I just want to be sure the team has enough bandwidth to get everything done!

tesk9 avatar Aug 22 '23 15:08 tesk9

Yep, trust me it's a bit daunting 😅. We don't have too many uses of alignStart/alignEnd, but afaict the upgrade path will involve checking each one so definitely thank you for raising early.

The upside is this isn't terribly high priority, and I can work asynchronously to upgrade existing ones before needing to merge this, and if it truly is a complete headache this isn't strictly necessary, just very nice to have. The other slight saving grace is that the custom offset is most likely (I hope) accomplishing what this PR does automatically, so should generally involve setting it to zero (I think).

caseyWebb avatar Aug 24 '23 20:08 caseyWebb

If you do end up going for it, I recommend testing your changes with Nri.Writing.UITour as early as you can. It's a weird/non-standard use of tooltips (and a pattern that shouldn't be replicated anywhere else), but it's the area where I think this change might be particularly tricky. I could be wrong, but that's my guess!

tesk9 avatar Aug 30 '23 15:08 tesk9