components icon indicating copy to clipboard operation
components copied to clipboard

docs-bug(chips): Tabindex is missing in the documentation

Open ice-blaze opened this issue 3 months ago • 7 comments

Documentation Feedback

Today I struggled about mat-chips being focusable and wanted to make them unfocusable (but not disabled). I thought there was no tabindex until I found this comment which obiously said you can acces it through [tabIndex] https://github.com/angular/components/issues/20883#issuecomment-744759846.

Now the first question: is it expected to have no mentions of the tabIndex in the documentation? Because there is already the id and value inputs for example and I believe the tabIndex could be there too. Also apprently I'm not the only one on the internet struggling knowing it exists. I think it could be a good addition to the documentation.

If you think it makes totally sense 🎉I'm super motivated to update the repository to see them. I already checked in the source code that all these components have a tabindex input:

  • button
  • checkbox
  • chips
  • datepicker
  • expansion-panel
  • radio
  • select
  • slide-toggle
  • tab
  • tree

I also have the feeling that the documentation uses the comments from angular/components and is generated in angular/material.angular.io. So I have the feeling I only need to update angular/components. As far as I understand there is already comments above the tabIndex input so it should be visible in the documentation.

If angular/component seems OK I was wondering maybe in angular/material.angular.io there is some filtering that ignores tabIndex but no mentions at all about tabIndex so I have the feeling that our problem doesn't lay there.

Last point, in the API Report File for "components-srcs" I can see some attributes with // (undocumented) comment above. These attributes aren't visible in the documentation. But still, the tabIndex don't have this kind of comment above.

I really wanted to fix this problem myself but it would be nice if someone could give me some hints on where to look. It would save me some time :)

Affected documentation page

http://localhost:4200/components/chips/overview

ice-blaze avatar Mar 28 '24 18:03 ice-blaze

I think it's awesome you want to contribute to this repository, I would love to help!

It feels to me making the chips in your app to be unfocusable but not disabled goes against accessibility best practices. Typically we want things that are interactive to go into focus.

I am curious about your specific use case for not making it focusable. If it makes sense, then yeah this should be added to the documentation.

amysorto avatar Apr 12 '24 15:04 amysorto

@amysorto The purpose was to use the chips only for the display. When we use the disabled attribute they get greyed out.

I couldn't find any css that could have the same style like the mat-chip.

Nevertheless, if we findout that it makes no sens to use tabindex then I have the feeling we should remove the @Input in the components no?https://github.com/angular/components/blob/e3eef47f80a7c1dda4b538128fed7f530f7cdff1/src/material/chips/chip.ts#L202

ice-blaze avatar Apr 14 '24 19:04 ice-blaze

Hello @ice-blaze ,

I think I see what you're saying. "Disabled" can mean a few different things depending on who is talking about it :). It sounds like you are trying to have chip with disabled appearance, but the chip is not inoperable.

-Zach

zarend avatar Apr 15 '24 15:04 zarend

Hello @zarend, I think I have to be more clear here. It's the other way around I'm looking for. The chip with the standard appearance (vibrant colors) but inoperable. They are not used in an interactive way in my case. I hope it makes more sens.

ice-blaze avatar Apr 15 '24 18:04 ice-blaze

@amysorto @zarend We also want to make our chips non-focusable. Our usecase is EXACTLY as the Autocomplete in the documentation here: https://material.angular.io/components/chips/overview#autocomplete, and as you can see, the chip itself isn't interactable, only the "Remove" button needs focus.

We can't simply disable the chip because then the "Remove" button would stop working.

EliezerB123 avatar May 09 '24 10:05 EliezerB123

Got it, okay that makes sense to me! Chips that are display only (not interactive).

@ice-blaze Yes you are right, our docs get populated from the code comments throughout this repo.

As far as tabIndex, it seems like it isn't being documented explictly https://github.com/angular/components/blob/main/tools/dgeni/common/private-docs.ts#L27, likely because the syntax is just the same as any HTML element and isn't specific to our components https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex.

Although regardless of the tabIndex documentation, that doesn't quite solve making chips display only right? It would just help remove it from keyboard focus but not disable the button.

amysorto avatar May 09 '24 13:05 amysorto

@amysorto Thank you very much for pointing out the place where the documentation decide to ignore the tabIndex (and other elements).

I have the feeling that if we should not display tabindex because they are global attributes, then it should be the same with id https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id and all the aria- attributes. But these are displayed in the material documentation.

Except if I convinced you with my comment I think there is no need to talk about this matter anymore x) or at least not about the tab index. I retried and findout that the "issue" seems only to exist in the legacy material https://v14.material.angular.io/components/chips/examples#:~:text=Favorite%20Fruits-,Chips%20avatar,-link if we use the new one https://material.angular.io/components/chips/overview#static-chips the tab focus don't jump on chips.

Nevertheless I have the feeling it's a bit odd that the keyboard user skip the chips but if you use the mouse you still have some animations if you click on the chips. Shouldn't that be considered as a bug? (If an element isn't interactive there should be no mouse over animations IMO) I haven't seen other issues related to this topic. Also I can find time to fix this small issue if everyone agrees.

ice-blaze avatar May 09 '24 15:05 ice-blaze