Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Tooltips package #9

Open
didoesdigital opened this issue Dec 31, 2019 · 9 comments
Open

Replace Tooltips package #9

didoesdigital opened this issue Dec 31, 2019 · 9 comments
Labels
good first issue Good for newcomers

Comments

@didoesdigital
Copy link
Owner

didoesdigital commented Dec 31, 2019

Overview

Typey Type is currently using a fork of react-tippy that supports custom tag elements for the tooltip trigger. React tippy has some drawbacks and appears to be unmaintained now. It's time to replace it.

image

Details

A replacement tooltip library needs to have/be:

  • Maintained.
  • The tooltip trigger can be abbr, span, div, a, or an icon (e.g. where icon has tabindex=0).
  • Support a hover delay so that you can hover on user settings for 100–200ms without triggering the tooltip.
  • Keyboard accessibility (can focus the trigger element and see content, which may or may not be affected by hover delay).
  • Closes on escape key (see: Keyboard accessible tooltip ReactTooltip/react-tooltip#107)
  • Screen reader accessible. (I think it should be screen reader accessible if you can navigate with the screen reader (not keyboard/focus) from trigger to tooltip as the "next" element in the tree, rather than using announcements; although announcements might be nice for expanding abbreviations in text).
  • Auto positions away from viewport edges when there's not enough space, but is otherwise a "top" tooltip.
  • Animates using a slide fade, 200ms, and triggers on mouseenter, focus, and click.
  • Supports custom styling to look like Typey Type's tooltips.
  • No console errors (when using hover delay with react-tippy the console is full of errors in production).
  • Can wrap the OutboundLink for external links with icons and "opens in new tab" tooltip.
  • Can work inside a modal e.g. is not clipped by modal boundaries.
  • No vulnerabilities.

Does NOT need:

  • No need for focusable content inside a tooltip. I think links or interactive content inside tooltips is poor usability.
  • There's no need for specific positioning like "down" (even though that might be handy on user setting page) IF it supports hover/focus delay.
  • There's no need for extra options like "follow mouse"—tooltips should be stable in their positioning.
  • There's no need for multiple colors or styling options, 1 style for the whole app is enough.

Approach

  1. Find a library that meets these needs and test that it works as described.
  2. Replace existing tooltips with new tooltips.
  3. Remove react-tippy.

Options

Some places to start looking:

@didoesdigital didoesdigital added the good first issue Good for newcomers label Dec 31, 2019
@didoesdigital
Copy link
Owner Author

OR we could replace ALL tooltips with not-tooltips. For example, show a dashed underline on settings that usually trigger tooltips and on click expand descriptions inline.

@MohitBansal321
Copy link
Contributor

I think we can work with react-tooltip
it is the abstract type of react-tippy
easy to implement
your thoughts?

@didoesdigital
Copy link
Owner Author

I think react-tooltip is probably fine but can you please check that it is accessible? Maybe start by creating a branch and add 1 tooltip using react-tooltip and check if you can read its contents with a screen reader and make the tooltip appear using a keyboard? If you can confirm that it will be accessible, then we can continue with that package!

@MohitBansal321
Copy link
Contributor

MohitBansal321 commented Nov 1, 2023

I tried it in the new branch, and it's working fine, the same as react-tippy
can you provide more context on the tooltip appearing using a keyboard (what did you want)
what do you mean by screen reader

@didoesdigital
Copy link
Owner Author

The tooltip needs to be keyboard accessible so a person using only a keyboard and no mouse should be able to access the information in the tooltip. If you use the tab key to tab to the tooltip trigger element, such as a button then the tooltip should appear when the button has focus.

You can read about screen readers here:

@MohitBansal321
Copy link
Contributor

MohitBansal321 commented Nov 3, 2023

The tooltip needs to be keyboard accessible so a person using only a keyboard and no mouse should be able to access the information in the tooltip. If you use the tab key to tab to the tooltip trigger element, such as a button then the tooltip should appear when the button has focus.

You can read about screen readers here:

Got it
when user read mode on then it will read the button text and other text correct? (not hover text when user focus on button)
I mention which one is used in read mode
image

@didoesdigital
Copy link
Owner Author

I think when the user has focus on "Accuracy", the screen reader should read out something like "Accuracy. Assuming accurate words are typed within stroke count targets". Does that make sense?

But I don't really mind how the tooltip package implements the tooltip so long as one way or another people using keyboards and screen readers and other devices can all access the same information. Some packages require the user to press Enter or Spacebar on "Accuracy" to activate and focus the tooltip before reading out "Assuming accurate words are typed within stroke count targets". I would be ok with that approach too, so long as everyone can access all the information.

@MohitBansal321
Copy link
Contributor

Sorry, earlier I was busy with my internship.
But now i can work on it

@didoesdigital
Copy link
Owner Author

@MohitBansal321 the latest code in Typey Type has made some changes that adds more tooltips and other changes that might cause git merge conflicts if you've already been working on a branch for this issue. If you have not started, make sure you pull the latest code before creating a new branch to work on this.

There is also a new issue, #151, to update icons that might interest you. Those icons are also wrapped in Tooltips that would also cause git merge conflicts for any branches that change tooltips at the same time. If that issue interests you, it would be simpler to get 1 PR for one issue merged before creating a new branch for the other issue.

If you run into merge conflicts and if you need help resolving them, this looks like a decent guide: https://www.freecodecamp.org/news/how-to-handle-merge-conflicts-in-git/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants