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

Allow list-item action slots to have an independent disabled state form the list-item #5664

Closed ethanbdev closed 1 month ago

ethanbdev commented 2 years ago

Description

The use case is that a list item (GP tool) is disabled, but we want to show an information icon that the user can interact with either on hover / click to give help to the user on why this item (GP tool) is disabled. This would occur when the licensing is not sufficient to use the tool and we need to direct them to information on getting the license / seeing more information about the licensing level.

Right now, the actions inherit the disabled state from the list-item, so there is no way to provide user interaction on those actions.

Blocked issues: https://github.com/Esri/calcite-design-system/issues/8373

Acceptance Criteria

The scope of the issue would be the list-item respecting the disabled attribute on itself and any slotted actions independently.

Relevant Info

image image

Which Component

calcite-list-item

Example Use Case

https://codepen.io/eborgen/pen/WNyvagJ

Esri team

ArcGIS Map Viewer

driskull commented 2 years ago

@jcfranco @benelan @eriklharper What are your thoughts about how disabling could work here?

Other ideas?

benelan commented 2 years ago

@geospatialem any a11y concerns with a disabled component having non-disabled child(ren)?

geospatialem commented 2 years ago

@geospatialem any a11y concerns with a disabled component having non-disabled child(ren)?

@benelan This sounds like a very solid use case. From the a11y perspective, its not a known or expected pattern, so would need to consult with a few other a11y professionals to determine the best path forward on the enhancement. Can report back here in a few weeks once a few of us can convene together.

driskull commented 2 years ago

Yeah if we disable the component it will get aria-disabled on the host. I'm not sure if that will affect slotted children or not.

nattarnoff commented 2 years ago

Disabled is not a valid attribute on the <li>. What needs to happen here is the disabled attribute needs to be applied to the button. It is also preferred to use this attribute over the aria-disabled attribute. <li> <button disabled><span>ICON</span> TITLE</button> <button class="tooltip" aria-label="NAME">icon</button><button class="tooltip" aria-label="NAME">icon</button> </li>

This will allow you to disabled the control but allow the tooltips and other icon buttons to still be accessible.

karl9006 commented 1 year ago

@driskull @geospatialem Please refer to Nat's response above.

driskull commented 1 year ago

@nattarnoff I'm not following your comment. I don't see a disabled attribute added to any li within the calcite-list-item. There is no <li> used internally on that component.

driskull commented 1 year ago

Since the "content" button is internal to the component and not slotted, we would need to be able to disable via a prop or something like that if we want to support this without breaking changes.

nattarnoff commented 1 year ago

@driskull as the calcite-list-item has role="listitem" on it, it IS a lias far as the browser and assistive technology are concerned. Therefore, you cannot put disabled or aria-disabled on it. Those should be located on the items inside the listitem.

driskull commented 1 year ago

@nattarnoff

Therefore, you cannot put disabled or aria-disabled on it. Those should be located on the items inside the listitem.

Its also a custom element, so we can put disabled on it because we define that attribute as a custom attribute. disabled is not a global reserved attribute so we can use that name for the custom component calcite-list-item.

As for the aria-disabled. We do this for all of our components we we add our disabled attribute on them. cc @jcfranco If we need to change that part, we can. From what I understand, we can add aria-disabled to any custom element regardless of the role. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-disabled

Another reason to need use the aria-disabled attribute over the HTML disabled attribute is if you have created custom controls which need to be marked as disabled, but are not using an element that allows for the disabled attribute.

nattarnoff commented 1 year ago

@driskull I went through the Codepen at the top of the description. The problem with how it is written now is that the second icon in the first calcite-list-item isn't tabbable despite being a button and having aria-disabled="false". If you navigate by arrows, you find that it announces as a button, but is unavailable. image

From the first description paragraph of the linked MDN article (emphasis mine):

The aria-disabled attribute, when set to true, indicates that the element upon which it is set and all of its focusable descendants are meant to be in the disabled state. This declaration will inform people using assistive technologies, such as screen readers, that such elements are not meant to be editable or otherwise operable.

Aria-disabled cascades. Once set, the children cannot be unset. Putting these attributes on the container and not the controls causes problems. I've put together a sample using a basic UL to demonstrate: https://codepen.io/nattarnoff/pen/dyeVgdV?editors=1111

You still need to move the attributes from the parent (rendered LI) to the rendered controls (button). This is what the AT is concerned about regardless if the control is built using a JS framework like Calcite or native HTML.

driskull commented 1 year ago

I went through the Codepen at the top of the description. The problem with how it is written now is that the second icon in the first calcite-list-item isn't tabbable despite being a button and having aria-disabled="false". If you navigate by arrows, you find that it announces as a button, but is unavailable.

Yes, it's working as designed right now. If a component is disabled, all things inside it are disabled as well. I think we would need to engineer a way to disable only specific parts of the component. If we need to rethink how this is engineered, we can do that.

You still need to move the attributes from the parent (rendered LI) to the rendered controls (button). This is what the AT is concerned about regardless if the control is built using a JS framework like Calcite or native HTML.

I think it's up to the user to disable anything they slot into the component. Since they can slot custom components like calcite-action, we don't know all the things that may need to be disabled. In this case, the user would need to set disabled on the calcite-action as well. We could try to query for known slotted components and disable them but I'd prefer to not modify the light DOM in these cases.

nwhittaker commented 1 year ago

The Field Maps web app is looking to use the list component for listing and selecting layers. In some cases a layer is not selectable, but we still want to be able to perform actions on it (e.g. "Delete"), or show accessible error iconography w/ tooltips.

jcfranco commented 1 year ago

@ethanbdev, @nwhittaker Could you provide info on priority/impact for this?

nwhittaker commented 1 year ago

Could you provide info on priority/impact for this?

This issue is one of several that are blocking an effort in Field Maps Designer to refactor the list of layers and group-layers as a <calcite-list>:

Our current implementation is a combination of custom components and <calcite-tree> elements. We'd like to migrate to the <calcite-list> element because it provides built-in filtering, more action/content slots, and grouping/nesting. The impact of moving to this element is that it would hopefully streamline the code and improve the consistency, maintainability, and testability of our layers list UI.

For prioritization, our refactoring issue is beginning to block other issues (that would like to make use of the list-item slots) that are slated for the Summer release (6/27). I imagine we have some flexibility, though, if that date's not feasible.

ethanbdev commented 1 year ago

For web analysis, its blocking a design to communicate licensing information to users.

I would put p3 - want for upcoming milestone if @nwhittaker agrees or wants to put it higher

geospatialem commented 1 year ago

A pattern will be established and should be part of a convention update when decided upon.

github-actions[bot] commented 1 year ago

Installed and assigned for verification.

geospatialem commented 1 year ago

Verified in 1.4.0-next.24 with the new internal component, calcite-stack: https://codepen.io/geospatialem/pen/eYPbbOo

nwhittaker commented 11 months ago

Just curious why this issue was closed? The reported problem still exists in calcite-list-item elements: https://codepen.io/nwhittaker-esri/pen/oNmmMKb.

In this repro, I'd expect the first item's action to be accessible.

geospatialem commented 11 months ago

Just curious why this issue was closed? The reported problem still exists in calcite-list-item elements: https://codepen.io/nwhittaker-esri/pen/oNmmMKb.

In this repro, I'd expect the first item's action to be accessible.

Thanks for flagging this, @nwhittaker!

Reopening this issue, but blocking the effort as a result of https://github.com/Esri/calcite-design-system/issues/8373, which will be a more robust new component that can support list-items, and other similar components, which contain a heading and description. While there is a solution available for some lists, it doesn't support cases where a heading and description is provided, and there are some alignment issues.

image

jcfranco commented 8 months ago

This came up recently and we'll be exploring an option to list items to not be interactive (w/o using disabled). This should help with the provided use case and also allow disabled to behave consistently across components (preventing interaction with the element and any respective children).

macandcheese commented 3 months ago

+1 from UC

driskull commented 3 months ago

Maybe we should add a property like contentDisabled which won't disable interaction but will look disabled?

I'm not sure if it should also prevent selection of the item (just allow actions to be accessed). We could have another prop for selection-disabled if we want to separate them.

macandcheese commented 3 months ago

Yeah I think that’s a nice clean solution and one that should be easy for users to opt-in to. Whether it’s multiple booleans or a single property with multiple values, either way sounds good.

driskull commented 3 months ago

@ashetland @SkyeSeitz @jcfranco any thoughts here?

I think we should proceed with a content-disabled property that will just make the content area visually disabled (add opacity to the content area).

SkyeSeitz commented 3 months ago

Agreed with moving forward with a content-disabled prop to disable interaction on everything (including selection) on the calcite-list-item expect for slotted actions. Was there need for content-disabled to still allow for selection at this time?

ashetland commented 3 months ago

With content-disabled, if slotted Actions still function, would the drag handle and the expand buttons also still function? Second question, would this be a confusing story once paired with disabled and interaction-mode from #8634?

driskull commented 3 months ago

prop to disable interaction on everything (including selection) on

I think in some cases selection may still want to function. Maybe we shouldn't disable this.

ashetland commented 3 months ago

Throwing this out there, could this be addressed via tokens to set the opacity on the content container (or some other way to accomplish this)? If it's not actually disabling anything, and is only a visual treatment, then I'm not sure this prop makes sense.

driskull commented 3 months ago

Throwing this out there, could this be addressed via tokens to set the opacity on the content container (or some other way to accomplish this)? If it's not actually disabling anything, and is only a visual treatment, then I'm not sure this prop makes sense.

Yes, it could. However, that would mean anyone that wants do to this would have to all set the same opacity number or color value. I could see some inconsistency arise here. I don't think this is a one off thing. I would use this property for layers that are out of scale range in the LayerList widget.

If the name is an issue, we could find something better than content-disabled like inactive

I don't think this should prevent selection because there are use cases where the item should still be selectable to open a related panel.

ashetland commented 3 months ago

+1 for inactive

driskull commented 3 months ago

@jcfranco thoughts?

Can we proceed with inactive to set the content to look disabled.

If a further use case arrives for selection-disabled we can address it at that time.

jcfranco commented 3 months ago

I like the proposal for inactive. ✨🏆✨

Would it be possible to style slightly different from disabled to avoid confusion? That'd be my only concern.

geospatialem commented 1 month ago

Thanks all for the great discussions and insights to use cases.

Calcite is proceeding with the above request on list-item via a new boolean property, unavailable, anticipated implementation of Q4 2024 (25.R1).

github-actions[bot] commented 1 month ago

Installed and assigned for verification.

geospatialem commented 1 month ago

Verified with Chromatic build with list-item's new unavailable attr/prop:

verify-5664