nrkno / core-components

Accessible and lightweight Javascript components
https://static.nrk.no/core-components/latest/
MIT License
117 stars 10 forks source link

core-suggest: disabled state #661

Closed vegarcodes closed 1 year ago

vegarcodes commented 2 years ago

Hi, tried looking around a bit for something on this issue.

If an input has a core-suggest component bound to it, and said input is disabled either by disabled or aria-disabled, clicking or attempting to focus on the input field should not trigger the suggestion list to pop up. With a regular <datalist>, disabling the input prevents the list from appearing, but no such luck with core-suggest as far as I've been able to tell. Ideally there would be a way to disable it when using aria-disabled as well somehow.

Is this a missing feature, or is there a way to disable the component manually that I haven't been able to find? Thanks for all the great work being done!

skjalgepalg commented 2 years ago

Thanks for pointing this out. Made a PR with fix to solve this for click events on connected input with disabled.

We presently don't see a use-case for handling aria-disabled as separate from disabled, seeing how an input with disabled should cover the effects of aria-disabled. Happy to look into it if you have a generic use-case or can elaborate further.

vegarcodes commented 2 years ago

It is mostly because disabled makes it so that you cannot put focus on the input element, which makes it impossible to i.e. have a descriptive tooltip/text that can be announced when the input is focused. aria-disabled makes that usecase possible. In that case, it would be nice to be able to programmatically disable the core-suggest component somehow. I'm not sure if that is a narrow usecase, however.

skjalgepalg commented 2 years ago

Any chance you are able to build a standalone example in e.g. a codepen to demonstrate?

@kristofferlium and I did some testing and are left with a feeling that using aria-disabled does not feel like a good option as it already has a purpose (to indicate that the element is perceivable but disabled, so it is not editable or otherwise operable) and we could risk misleading both users and coders alike.

vegarcodes commented 2 years ago

Sure thing, I'll see about getting that in order next week, and then I'll update the issue with the link 👍

skjalgepalg commented 1 year ago

Closing this as resolved through #662. If this is still an issue for you, please reopen!