`IconButton` should include `Tooltip` by default

[iansan5653]

Summary

There is no documented example of how to render an icon-only button with an accessible tooltip in a way that would meet all of the following standard requirements:

• Render as an icon-only button with a single focus target
• Read the label one time in a manner that ties it to the button, for screen reader users
• Show the label on hover, for mouse users
• Show the label on focus, for keyboard users

If this is a supported behavior, then some documentation is needed to show how to do it. If this is not supported, it would be very useful functionality to add to Primer.

The IconButton documentation provides an example of how to label the component in an accessible manner:

<IconButton aria-label="Search" icon={SearchIcon} />

And the Tooltip documentation has the same:

<Tooltip aria-label="Hello, Tooltip!">Text with a tooltip</Tooltip>

Both of these examples use aria-label however, which provides no obvious way to create an icon button that shows its accessible label on hover as a tooltip.

Use Cases

This is a very common requirement. See for example these instances on github.com:

Notifications bell:

Markdown editor toolbar:

Code view:

This is recommended as a best practice by Primer's own tooltip guide:

Attempts

I've tried all of the following examples but I haven't been able to find a satisfying way to do this:

Label on tooltip: Tooltip does not appear on button focus and the button appears as unlabelled in the accessibility tree.

<Tooltip aria-label="Bold">
  <IconButton icon={BoldIcon} />
</Tooltip>

Label on button only: Tooltip does not work at all.

<Tooltip>
  <IconButton icon={BoldIcon} aria-label="Bold" />
</Tooltip>

Label on both: The label will be read twice to screen readers (something like "Tooltip: Bold, Button: Bold") which is unnecessarily redundant. Also the tooltip still doesn't appear on focus.

<Tooltip aria-label="Bold">
  <IconButton icon={BoldIcon} aria-label="Bold" />
</Tooltip>

Tooltip as button: Seems like a promising idea but Tooltip does not support the as prop.

<Tooltip aria-label="Bold" as={IconButton} icon={BoldIcon} />

Button as tooltip: Renders an unfocuseable span instead of a button.

<IconButton icon={BoldIcon} as={Tooltip} aria-label="Bold" />

Tooltip in button children: Does not work because the IconButton ignores children.

<IconButton icon={BoldIcon} aria-label="Bold">
  <Tooltip aria-label="Bold" role="presentation">Bold</Tooltip>
</IconButton>

Tooltip as button icon: This is horribly inelegant, but it does generate a good accessibility tree and still shows the tooltip on hover. It unfortunately still doesn't show the tooltip on focus, but it's the best I have for now.

<IconButton
  icon={() => <Tooltip aria-label="Bold" role="presentation"><BoldIcon /></Tooltip>}
  aria-label="Bold"
/>

Proposed Solution

My ideal solution would be to just show the IconButton's aria-label as a tooltip by default.

To configure this, I propose adding two new props to the IconButton component:

• disableTooltip: Turn off the tooltip.
• tooltipProps: Configure the tooltip by passing props to the underlying Tooltip component.

Then I could achieve my goal simply with:

<IconButton icon={BoldIcon} aria-label="Bold" />

[siddharthkp]

My ideal solution would be to just show the IconButton's aria-label as a tooltip by default

+1 to that. I like the idea of having a tooltip by default.

disableTooltip: What are the instances where you would want to disable the tooltip?

We'd need to workshop tooltipProps a bit before committing to an API 🤔

[iansan5653]

What are the instances where you would want to disable the tooltip?

I figured there might be a case where the built-in tooltip conflicts with another tooltip or hover effect, but I can't think of a concrete example. Maybe it's actually not necessary to support disabling it.

[lesliecdubs]

I'm going to run this idea of having a tooltip by default at tomorrow's Primer Patterns working group session to get any feedback or concerns before we run with that idea.

In the meantime - @khiga8 could you please take a quick look at this issue and the proposal to adding a tooltip-by-default on IconButton to ensure this approach is in line with our accessibility needs?

[khiga8]

Hi @lesliecdubs!

@hectahertz can speak more to the work on IconButton in PVC! I believe we were leaning towards having tooltip by default for all IconButton for accessibility purposes. Discussion can be found in: primer/view_components#1062.

As a side note, I notice in all the examples listed in this issue, the tooltip is always semantically associated as a label. I'm not sure if this is out of scope, but in the long run, the tooltip should also be allowed to be associated to the icon button as a supplementary description.

You can check out the markup of the markdown toolbar in dotcom as an example. These were recently updated to use tooltips that are associated semantically using aria-describedby with aria-label directly set on the button with a brief accessible name. This was done (at least in dotcom) because the tooltip content is kind of long and contains keyboard shortcuts which makes the tooltip inappropriate as an accessible name. Accessible names should be brief. Therefore you'll notice we have a label like Bold directly on the button and a tooltip associated as description that contains, Add bold text, <Cmd+b>.

We don't have this in dotcom, but similarly in the Notifications button example, it may be appropriate to set aria-label as Notifications, and set a tooltip as supplementary description with You have unread notifications.

We've recently worked on a bunch of accessibility doc improvements on the PVC and Primer Design side to encourage proper tooltip usage with valid tooltip use examples. Here are some references that may be helpful as Primer React team works on tooltip:

• PVC: Tooltip
• Tooltip alternatives

[lesliecdubs]

Thank you for weighing in @khiga8, this is very helpful context to inform our next steps!

It does indeed look like PVC came to the same decision about always showing a Tooltip on Icon Button in primer/view_components#1062:

After primer/view_components#1062 (comment), we decided to always include a Tooltip on IconButton. The aria-label` property is always required, and this will improve the experience for sighted users. Hopefully, this approach will simplify the API and will keep the instances of IconButton consistent and accessible.

@hectahertz can you weigh in on any other API decisions that have been made in PVC that may be relevant? Perhaps @hectahertz and @siddharthkp can sync in these comments on whether we want to provide support for props disableTooltip or tooltipProps as proposed in this issue, and whether there are other specific learnings from the PVC work that should inform PRC's approach.

[siddharthkp]

From: primer/view_components#1062, IconButton has an argument called tooltip_direction which is the only customisation tooltip allows. (tooltip arguments)

@hectahertz @khiga8 Are there plans on expanding this list of arguments?

As a side note, I notice in all the examples listed in this issue, the tooltip is always semantically associated as a label. I'm not sure if this is out of scope, but in the long run, the tooltip should also be allowed to be associated to the icon button as a supplementary description.

Thanks for the example, this is really helpful! It was one of the questions I had :)

[siddharthkp]

tl;dr: we should copy what @hectahertz did in primer/view_components#1062

 

long version:

The tooltip from view_components only has one relevant attribute for the conversation: tooltip_direction, but the Tooltip in primer/react is more customisable, so the API could look end up looking slightly different:

 

Here are 2 use cases:

1. IconButton with an aria-label:

   

   Without tooltip:

   <IconButton icon={KebabIcon} aria-label="More options" />

    

   Option 1: wrap IconButton in a Tooltip

   <Tooltip text="More options" direction="sw"  align="left">
     <IconButton icon={KebabIcon} /> // need to make aria-label optional
   </Tooltip>

   👍 If you've used the Tooltip component before, composing it this way is intuitive
   👍 Tooltip attributes go on the tooltip as you would expected

   👎 Need to make aria-label on IconButton optional so that it is not repeated twice. Can result in an IconButton without a Tooltip

    

   Option 2: bake in tooltip with IconButton (use aria-label used for tooltip text)

   <IconButton icon={KebabIcon} aria-label="More options" tooltipDirection="sw" tooltipAlign="left" />
   // or
   <IconButton icon={KebabIcon} aria-label="More options" tooltipProps={{direction: 'sw', align: 'left'}} />

   👍 Can ensure there will always be a tooltip with IconButton

   👎 tooltipProps on the IconButton aren't intuitive
   👎 while unintuitive, it's hopefully rare to want to customize the tooltip's position. we can attempt to restrict the options to just tooltipDirection to avoid the more awkward tooltipProps API

   While option 1 is more composable, we should bake in the tooltip in Option 2 to make the recommended path the default one.

    

2. For the markdown editor with aria-label and aria-description

   • aria-label is "Bold"
   • aria-description = tooltip text is "Add bold text, <Cmd+b>"
   

   Option 1:

   <Tooltip text="Add bold text, <Cmd+b>" direction="s">
     <IconButton icon={BoldIcon} aria-label="Bold" />
   </Tooltip>

   👍 If you've used the Tooltip component before, composing it this way is intuitive
   👍 Tooltip attributes go on the tooltip as you would expected
   👍 (compared to previous example) Don't need to make aria-label optional, this is perfect.

   Option 2:

   <IconButton 
     icon={BoldIcon}
     aria-label="Bold"
     aria-description="Add bold text, <Cmd+b>"  // aria-description used for tooltip
     tooltipDirection="s"
   />

   👍 collocating aria-description right next to aria-label on the IconButton itself makes a lot of sense while reading the code (for someone who works with accessible markup in mind, this is very intuitive. I have secondary information about this button, so that goes on aria-description)
   👎 It's not immediately obvious that the tooltip will pick up aria-description instead of aria-label (for someone who is approaching this visually, it isn't intuitive. I want to add a tooltip, but instead of using a Tooltip component, i should add aria-description because this is an IconButton)
   👎 2 ways of doing the same thing - if someone writes the code from Option 1, that would also work and give the same exact output 😅

 

Personal opinion

If we look at the library as a whole and not just IconButton, it makes more sense to go with Option 1, re-use Tooltip and ask folks to wrap IconButton with a Tooltip. It's easy to miss, but we can enforce this with help of our linter or/and by adding warnings in dev mode. example of warning, sandbox

However, I think being accessible out of the box makes Option 2 very attractive despite the awkwardness around customising tooltip position. If we know developers would always have to wrap an IconButton with a tooltip, we should do it for them.

Having 2 ways of doing one thing from the markdown example feels wrong, but is an okay tradeoff to make as long as the output for both approaches is accessible.

[vdepizzol]

I'm going to run this idea of having a tooltip by default at tomorrow's Primer Patterns working group session to get any feedback or concerns before we run with that idea.

@lesliecdubs thank you for bringing this issue to the Primer Patterns working group attention 👋

@siddharthkp said:

(...) I think being accessible out of the box makes Option 2 very attractive despite the awkwardness around customising tooltip position. If we know developers would always have to wrap an IconButton with a tooltip, we should do it for them.

I agree we should provide components that are accessible by default.

A few considerations:

aria-label + aria-describedby

I wonder if we could abstract the aria-label into a more generic label prop for IconButton, while still generating aria-label + aria-describedby at the end. That way, it's not that aria-label also gets rendered as a tooltip, but that the IconButton label is initially used both as an aria-label, and as a tooltip:

<IconButton
  icon={HeartIcon}
  label="Sponsor"
/>

aria-describedby, as the HTML attribute specifies, asks for an element ID. In this case, we're passing a string directly. That could cause some confusion. Maybe we could call it tooltipText or tooltipDescription, to make it obvious it's a variation of the label specifically for the tooltip text.

<IconButton
  icon={BoldIcon}
  label="Bold"
  tooltipText="Add bold text"
/>

Optional tooltips

Although tooltips should come by default and be present in most IconButton components, it might make sense to decouple the tooltip behavior if necessary.

I can imagine larger components using IconButton for convenience (such as expanders, split buttons, etc) which wouldn't need a tooltip based on their context (but would need an aria-label, of course!).

An example I can think about is the TreeGrid I'm currently working on. The placement and context of the icon are enough for describing behavior. In that case we could have something like disableTooltip:

<IconButton
  icon={ChevronRightIcon}
  label="Expand folder"
  disableTooltip
/>

That could also be useful when a custom rendering is used, or when there's a need for a custom tooltip with extra parameters:

<Tooltip text="Add bold text" direction="s">
  <IconButton
    icon={HeartIcon}
    label="Sponsor"
    disableTooltip
  />
</Tooltip>

Or, if an IconButton is a child of Tooltip, could we enforce a disableTooltip directly?

<Tooltip text="Add bold text" direction="s">
  <IconButton
    icon={HeartIcon}
    label="Sponsor"
  />
</Tooltip>

Note that the label prop remains required.

Shortcuts

I think we should have a better way to reference shortcuts in the tooltips, instead of bundling them together with the description.

@maximedegreve has done some explorations for improved <kbd>, which I think could be brought into tooltips as well:

Here's a quick test:

An ideal scenario could have a Tooltip prop with common keys (Command, Shift, etc) automatically switching to their correct glyphs, and supporting multiple OSs:

<IconButton
  icon={ChevronRightIcon}
  label="More actions"
  shortcutPreview={{
    mac: ['Command', 'Shift', 'M'],
    win: ['Ctrl', 'Shift', 'M']
  }}
/>

In the example above I'm using shortcutPreview to make sure the prop doesn't make it sound like it's actually adding a shortcut to the action. 😁

@siddharthkp let me know your thoughts! 🙌

[siddharthkp]

aria-describedby, as the HTML attribute specifies asks for an element ID.

Oops, I should have used aria-description, not aria-describedby. My bad, fixed in the code now.

I wonder if we could abstract the aria-label into a more generic label prop for IconButton,

I mostly agree with using descriptive names over html attributes here.

<IconButton
  icon={BoldIcon}
- aria-label="Bold"
+ label="Bold"
- aria-description="Add bold text"
+ tooltip="Add bold text"
/>

I think we should have a better way to reference shortcuts in the tooltips, instead of bundling them together with the description.
An ideal scenario could have a Tooltip prop with common keys (Command, Shift, etc) automatically switching to their correct glyphs, and supporting multiple OSs

I agree with the latter but not with the first part. API based on an config object work well when we have tight control over the contents in the library, but not when the text is passed by the application layer.

I'd prefer to build a tiny reusable Key component that can be used with Tooltip:

import {Key} from '@primer/react'

<Tooltip 
  text={<>Add bold text, <Key meta>B</Key></>}
  direction="sw"
>
  <IconButton icon={BoldIcon} label="Bold" />
</Tooltip>

---

[siddharthkp]

I've slept over this and have a slightly different approach: opt-in composition

But first, Revisiting option 1 and option 2 again after @vdepizzol's comments:

Option 1 - Composition
Option 2 - Collocation / config-esque

1. IconButton with tooltip

// option 1
// note: as label and tooltip text are same, we would add only one of them.
<Tooltip text="More options" direction="sw">
  <IconButton icon={KebabIcon} label="More options" />
</Tooltip>


// option 2 
<IconButton icon={KebabIcon} label="More options" tooltipDirection="sw" />
// note: tooltipDirection is an optional customisation option

// ⭐️ we like option 2 more because it has the tooltip baked in.

2. IconButton with tooltip text that's different than label

// option 1: to add a different tooltip text than label, 
// wrap the iconbutton in a tooltip.
// note: label is still required
<Tooltip text="You have no unread notifications" direction="sw" >
  <IconButton icon={BellIcon} label="Notifications" />
</Tooltip>


// option 2: to add a different tooltip text than label, 
// pass a tooltip prop. predictable API smell of having tooltip text + tooltip prop on anchor component.
<IconButton
  icon={BellIcon}
  label="Notifications"
  tooltip="You have no unread notifications"
  tooltipDirection="sw"
/>

// option 2 isn't ideal for this use case, but the tradeoff isn't too bad to optimise for use case 1 above.

3. IconButton with no tooltip

// option 1: nothing to do here, because there was no tooltip
<IconButton icon={ChevronRightIcon} label="Expand folder" />

// option 2: disable tooltip by passing false instead of text
// note: label is still required 
<IconButton icon={ChevronRightIcon} label="Expand folder" tooltip={false} />

// both options are pretty good.

4. IconButton with rich content tooltip

import {Key} from '@primer/react}

// option 1:
<Tooltip 
  text={<>Add bold text, <Key meta>B</Key></>}
  direction="sw"
>
  <IconButton icon={BoldIcon} label="Bold" />
</Tooltip>

// option 2:
<IconButton
  icon={BoldIcon}
  label="Bold"
  tooltip={<>Add bold text, <Key meta>B</Key></>} 
  tooltipDirection="sw"
/>

// both options are pretty similar. option 1 feels a tiny bit better because the long form content is separated out.

 

Option 3: Opt-in composition

Best of both worlds

1. IconButton with tooltip

// by default, we use label to add tooltip. 
// ⭐️ accessible by default
<IconButton icon={KebabIcon} label="More options" />

2. IconButton with tooltip text that's different than label

// if you want to change the tooltip text or direction,
// opt-in into composition and wrap the IconButton in a tooltip component
<Tooltip text="You have no unread notifications" direction="sw" >
  <IconButton icon={BellIcon} label="Notifications" />
</Tooltip>

// good things: Tooltip props are on the Tooltip. If you've used the Tooltip before, this is very intuitive.

3. IconButton with no tooltip

// silly little prop to disable default behavior 😅
<IconButton icon={KebabIcon} label="More options" tooltip={false} />

4. IconButton with rich content tooltip

import {Key} from '@primer/react'

// opt-into composition
<Tooltip 
  text={<>Add bold text, <Key meta>B</Key></>}
  direction="sw"
>
  <IconButton icon={BoldIcon} label="Bold" />
</Tooltip>

// note: only Tooltip supports rich text, IconButton label is still boring old string.

[iansan5653]

I like 'Option 3: Opt-in composition' - it provides significant power for complex scenarios. Having label be always required and of type string ensures that the button's aria-label is always valid.

I can see the last example generating a structure akin to:

<div>
  <button aria-label="Bold" aria-describedby="tooltipId"><!--bold icon--></button>
  <span id="tooltipId">Add bold text, <kbd>⌘B</kbd></span>
</div>

From a CSS perspective, it wouldn't be too hard to have a containing Tooltip prevent the aria-label tooltip from appearing and to have the container show its tooltip on focus-within.