📝 Improved documentation on Button #2742
Conversation
| Example All button combinations. | ||
| ### With tooltip | ||
| To display a tooltip on the `Button` you can wrap it in a `Tooltip`. If you need to disable the Button, the button should be disabled using | ||
| `aria-disabled` instead of the `disabled` prop because Chrome doesn't expose the hover event on disabled buttons. |
There was a problem hiding this comment.
This is a bit too specific, and therefore not entirely correct. Tooltip also does not work in edge, and in safari there is an onmouseover event, but no onmouseleave event, so the tooltip gets stuck always on. Instead i suggest:
Title: "Disabled buttons and tooltip"
text: "Due to inconsistent browser support for mouse events on disabled buttons, Tooltip should not be used in combination with disabled on Button. Instead use aria-disabled and manually block the button onclick action. This has the added benefit of making the Tooltip accessible for people using keyboard navigation, as the button is still focusable."
There was a problem hiding this comment.
Good! Will change
| ### Accessibility | ||
| The `aria-disabled` attribute makes it possible to semantically disable the `Button` without hiding it from assistive technologies, such as screen readers. | ||
| In EDS, the `Button` is visually styled as disabled, but it is up to the developer to actually disable the button. | ||
| An alternative workaround is wrapping the `Button` inside a fitted `span`. |
There was a problem hiding this comment.
Remove the suggestion to use a span since I don't see a scenario where you can't just use aria-disabled, and we shouldn't suggest non-accessible solutions in the documentation unless we really have to
| aria-disabled={isSubmitting ? true : false} | ||
| onClick={!isSubmitting ? onSubmit : undefined} | ||
| > | ||
| {isSubmitting ? <Progress.Dots /> : 'Fetch data'} |
There was a problem hiding this comment.
The progress indicator should have color="primary" as discussed in #2682 (It has been tentatively agreed with Birte this is the better solution)
| <> | ||
| <Button | ||
| aria-disabled={isSubmitting ? true : false} | ||
| onClick={!isSubmitting ? onSubmit : undefined} |
There was a problem hiding this comment.
Should add aria-label={isSubmitting ? 'loading data' : null} whenever there is icon only indicating the state
| <Tooltip title="This is what a tooltip looks like"> | ||
| <Button>Hover me</Button> | ||
| </Tooltip> | ||
| <Tooltip title="Tooltip doesn't show in Chrome"> |
There was a problem hiding this comment.
can we change text to: "This tooltip only shows for people using firefox and using mouse. Don't do this!"
| <Tooltip title="Tooltip doesn't show in Chrome"> | ||
| <Button disabled>Disabled button</Button> | ||
| </Tooltip> | ||
| <Tooltip title="Tooltip shows in Chrome with aria-disabled"> |
There was a problem hiding this comment.
Can we change text to: "Tooltip works in all browsers and with keyboard navigation when using aria-disabled"
| <Tooltip title="Tooltip shows in Chrome with aria-disabled"> | ||
| <Button aria-disabled>Aria-disabled button</Button> | ||
| </Tooltip> | ||
| <Tooltip title="Tooltip shows in Chrome because Button is wrapped in span"> |
There was a problem hiding this comment.
Delete the example using span (for reasons explained above)
| @@ -71,3 +71,14 @@ Long list of elements that have a tooltip. Used for testing if tooltip "lag" whe | |||
| Inputs of type radio, checkbox and switch will have the tooltip applied to the input element. A good way to get the tooltip to also apply to the label is to simply wrap the input in a span. | |||
|
|
|||
| <Story id="data-display-tooltip--radio-and-checkboxes" /> | |||
|
|
|||
| ### Tooltip on Button | |||
| To display a tooltip on the `Button` you can wrap it in the `Tooltip`. If you need to disable the `Button`, the button should be | |||
There was a problem hiding this comment.
Do the same changes here as I suggested in button docs
|
|
||
| export const TooltipOnButton: Story<TooltipProps> = () => ( | ||
| <> | ||
| <Tooltip title="This is what a tooltip looks like"> |
There was a problem hiding this comment.
Do the same changes here as I suggested in the button stories
Resolves #2724