Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
289 stars 76 forks source link

Tooltip does not work with disabled button #5318

Closed mpayson closed 1 year ago

mpayson commented 2 years ago

Actual Behavior

Tooltip does not show up on hover when its target button is disabled

Expected Behavior

Tooltip shows up like the doc example

Screen Shot 2022-09-14 at 11 52 25 AM

Additional context from #6461

When a disabled button contains information available by tooltip, using only aria-disabled communicates the disable state while permitting keyboard focus to be moved to the button: https://codepen.io/FelFly/pen/gOdYQQq

Reproduction Sample

Reproduction Steps

  1. Open codepen
  2. Hover over button, notice tooltip does not display

If disabled prop is removed, it works

Reproduction Version

beta.93

Relevant Info

No response

Regression?

No response

driskull commented 2 years ago

@mpayson For this, I think you would need to wrap the button in a div so that you can add the tooltip to that div instead.

Disabled elements don't fire hover events so we can't really do anything here. Its the same if you change <calcite-button> to <button>.

mpayson commented 2 years ago

Ah that makes sense, thanks for clarifying!

driskull commented 2 years ago

@mpayson actually, we may be able to do something here. I'll see if we can but for now wrapping it would be a good solution.

github-actions[bot] commented 2 years ago

Installed and assigned for verification.

driskull commented 2 years ago

@benelan @jcfranco can we use this issue as an area for discussion on if we should remove the pointer-events:none on disabled calcite-button?

benelan commented 2 years ago

Sure, @macandcheese do you have any design thoughts?

jcfranco commented 2 years ago

@benelan FYI, there's some initial discussion in the related PR.

geospatialem commented 1 year ago

Added to the Stalled milestone to discuss and prioritize after January's release.

yannikmesserli commented 1 year ago

Note that we encountered the exact same problem for Calcite Radio Group's items. When disabled, the tooltip explaining why it was disabled was not showing up. Should I open a new issue for it? Or is the discussion you are having (soon?) will include the pointer-event: none removal on any disabled Calcite components?

geospatialem commented 1 year ago

Note that we encountered the exact same problem for Calcite Radio Group's items. When disabled, the tooltip explaining why it was disabled was not showing up. Should I open a new issue for it? Or is the discussion you are having (soon?) will include the pointer-event: none removal on any disabled Calcite components?

@yannikmesserli We can definitely add this into the discussion. For now we'll purpose this issue for both discussions later this month and might add in a new issue once prioritization and next steps are determined.

In the meantime, could the following example suffice for a workaround, where a label with a button referencing the tooltip provides context on the disabled component?

ReneU commented 1 year ago

We're running into the same issue in ArcGIS Velocity where we have many buttons that could be disabled and we want to display an explanatory text in a tooltip. The workaround works for us but makes things more complicated.

geospatialem commented 1 year ago

@ashetland @SkyeSeitz - We're thinking we could leverage similar to native behavior with a title prop, where if someone is seeking to slot a tooltip to a disabled button, context could be provided.

While assistive technologies (AT) wouldn't be able to access either the button or slotted tooltip, they shouldn't need context since AT would bypass the component entirely. So would be to compliment those not using assistive technologies.

Wanted to get both your design perspective, if this makes sense, and if so, if you have any recommendations on an implementation?

Anastasiia-Boleiko commented 1 year ago

He there!

In ArcGIS Urban we have quite a lot of cases where it is really helpful to have a tooltip on disabled elements (ex.: buttons in forms, in toolbars; checkboxes, segmented control items, etc). Currently, we use custom tooltips on disabled elements, and in some places, we use Calcite Tooltip which is triggered by a wrapper of a disabled element (but it's not accessible for AT).

The main purpose of using tooltips on disabled elements is to explain why it's disabled. Basically, we use the same approach as was suggested in the Tooltip docs. I do agree, in some cases, right near the disabled component could be placed a helper text or an element (like an info icon) that has a tooltip with an explanation of why an element is disabled. But in some cases, for sake of keeping UI clear (mainly for those, who do not use AT), it's very useful to have a tooltip exactly on a disabled element (for example a tool in the toolbar or a button in the data table column).

Also, I suppose that some UI elements (even disabled) might be important for some user flows (for a general understanding of app logic). According to @geospatialem (I've tried it as well using VoiceOver), UI elements with a disabled attribute are bypassed by AT, which could be potentially confusing for those who cannot find a needed element using AT. Please, correct me if I'm wrong. So it would be really nice and helpful to have the same Tooltip behavior for all elements (even on disabled): For example, Calcite Tooltip is displayed when a trigger element got focused.

Idea:

Use aria-disabled="true" instead of disabled (it allows adding a tooltip on hover and focus) and apply appropriate styles to have a look and feel of a disabled state. Please, see a codepan example. A bit more info on that topic: A11Y and UX considerations for disabling More about aria-disabled Making Disabled Buttons More Inclusive Worth mentioning, that Calcite Tooltip complies with the criterias:

Success Criterion 2.1.1, Keyboard (Level A) All functionality of the content is operable through a keyboard interface without requiring specific timings for individual keystrokes, except where the underlying function requires input that depends on the path of the user's movement and not just the endpoints. Success Criterion 1.4.13 Content on Hover or Focus (Level AA) Where receiving and then removing pointer hover or keyboard focus triggers additional content to become visible and then hidden, the following are true: Dismissable, Hoverable, Persistent.

To sum up, it looks like Calcite Tooltip fits well to the desired behavior but needs to be available for disabled elements (at least in our cases). While the mentioned title prop seems to cover a part of desired features, following this approach has several cons: a disabled trigger element is bypassed by AT, not hoverable.
And last, but not least, Calcite Tooltip seems to appear quicker than a title, and has consistent look and feel with other tooltips in the application.

P.s.: Sorry for the long-read overwhelming comment.

geospatialem commented 1 year ago

To sum up, it looks like Calcite Tooltip fits well to the desired behavior but needs to be available for disabled elements (at least in our cases).

@Anastasiia-Boleiko Apologies, my message above might have been misleading. The intent is to follow the pattern of title with tooltip, so think we're on the same page.

Really appreciate the correction that the slotted tooltip would, in fact, receive context with aria-disabled (as opposed to disabled, which would not).

We're definitely thinking along the same lines. We'll be looking into the design prior to working on the code change, so will take it into account with our research. Other than the timeout length (set to 500 in the Codepen which has the tooltip hang a bit for mouse users), could be the pattern we'd follow with the enhancement by default - depending on our upcoming design research findings.

ashetland commented 1 year ago

Not sure I have anything else to add at this point. Thanks @Anastasiia-Boleiko for all the insights! Feels like this will come down to implementation details as the overall use case is a strong one.

Anastasiia-Boleiko commented 1 year ago

Thank you all very much! Happy to hear that we are on the same page.πŸ‘

P.s.: @geospatialem I'm sorry for the initial misunderstanding from my side. I do appreciate your detailed explanations.

geospatialem commented 1 year ago

I'm sorry for the initial misunderstanding from my side. I do appreciate your detailed explanations.

Not at all - really appreciate the deep dive and relaying on the findings, @Anastasiia-Boleiko, which are incredibly helpful. Thank you much! πŸ™ŒπŸ»

We're working through issue prioritization shortly, and hope to update the status of this issue with an upcoming 2023 milestone. Stay tuned!

github-actions[bot] commented 1 year ago

Installed and assigned for verification.

geospatialem commented 1 year ago

Verified on master.

On mouseover a tooltip can be referenced: image

Tabbing and AT skips over the button and tooltip πŸ‘πŸ»

Anastasiia-Boleiko commented 1 year ago

Hi there,

@geospatialem Can you please clarify whether the tooltips for disabled components will be accessible for assistive technology (AT) in the future?

Use aria-disabled="true" instead of disabled (it allows adding a tooltip on hover and focus) and apply appropriate styles to have a look and feel of a disabled state. Please, see a codepan example. A bit more info on that topic: A11Y and UX considerations for disabling More about aria-disabled Making Disabled Buttons More Inclusive

aria-disabled will still focus the button and announce that it exists, but that it isn’t enabled yet; the same way you might perceive it visually.

As I mentioned earlier, it could be beneficial since some tooltips serve as relatively important information and might be helpful for those who use AT (not only for mouse users).

Thanks.

geospatialem commented 1 year ago

Hi @Anastasiia-Boleiko - thanks for reaching out, just getting back to messages from UC. We're meeting on this topic next week as a team as it could have some breaking change implications. Will report back our findings in the coming week or so, including next steps.