jakubjanczyk commented 2 years ago

Problem/Feature

This PR solves 2 issues we have found in Messages, but they can benefit other usages as well :)

SimpleModal focus outline

Default outline when focusing SimpleModal is hardly visible, at least on Chrome. We wanted to make it better on Messages (https://helpscout.atlassian.net/browse/HSAPPUI-166), but figured out it might be better to improve it in the component itself instead of having custom styling. Here's how it looks after the change (please see comments to the linked JIRA for reason why this color): Screenshot from 2022-05-30 17-43-44

Disabled ActionSelect tooltip

Recently (https://github.com/helpscout/hsds-react/pull/1051) there was made a change that allowed to add tooltip to ActionSelect component (or more generally SelectTag toggler). However, this doesn't quite work if the button is disabled, because events are not fired in this case, so we cannot show tooltip on hover, for example. Following suggestion from here (https://github.com/atomiks/tippyjs-react/issues/123) I wrapped tooltip content with a span in this case. And it's only wrapped when button is disabled, because otherwise this additional layer might be breaking tab order and wouldn't actually show tooltip on a focus (which is not a problem for disabled button, because it cannot be focused).

If SelectTag is not disabled, tooltip works exactly the same as before.

Guidelines

Make sure the pull request:

[ ] Follows the established folder/file structure
[ ] Adds unit tests
[ ] If it is a refactor or change to an existing component, have you verified it won't break existing Cypress tests or have you updated them?
[ ] Did you verify some accessibility (a11y) basics?
[ ] Adds/updates stories. Guidelines
[ ] Adds/updates documentation (ie proptypes) Guidelines
[ ] Has it been tested in Help Scout's supported browsers?
[ ] Requests review from designer of the feature
[ ] Add label (bug? feature?)

plbabin commented 2 years ago

@jakubjanczyk pushed an update to use the focusRing mixin instead of an outline. Same outcome, difference is that there is a fade in animation, and also it's on focus-visible only

CleanShot 2022-06-01 at 13 38 14