-
-
Notifications
You must be signed in to change notification settings - Fork 812
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: make remove button in sl-tag focusable #1741
base: next
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I'm not sure what the behavior here should be, but I can tell you why it is how it is today and we can discuss it. In the past, we've seen requests for things like the clear icon in If the input is empty, no clear button is shown so one tab will exit the field. But if the input is not empty, a clear button is shown and one tab will not exit, but focus the cursor on a destructive action instead. Native [search] inputs work the same way. Tabbing doesn't move to the clear button. The focus behavior is always consistent. The reason for this is that keyboard users already have a number of ways to clear an input, some of which are arguably faster or just as easy as tabbing to a clear button:
Note that, despite being out of the tab order, clear buttons are still discoverable and accessible to screen reader users through the virtual cursor. The thought behind this decision comes from Scott O'Hara's explainer: https://scottaohara.github.io/clear-text-field-button/ The reason I bring all this up is because the tag's remove button is a lot like a clear button, and accepting this PR would cause a variable number of tabs when used with While you could take the time to tab through each option to remove it, it's probably easier to access it through the menu where activating any selected item will toggle the selection and remove the tag. For this use case, we probably don't want to focus on the remove button. However, there may be other use cases where this makes sense. I guess the question is, how are you using tags so they need this focus? And if we're going to focus the remove button, shouldn't be also make the tag itself focusable? That might even be a different component altogether. Maybe we shouldn't use tags in |
Getting the problem. The tag is an interesting element – it visually fits most to the concept of Chips in Material Design. (https://m3.material.io/components/chips/overview) But sl-tag doesn't provide any interactivity – besides the remove button, which at least would be fine for some use cases like "activated filters" in shops or similar, IF the remove button would be focusable. 😀 What do you think about extending the tag that it can (!) be a button or link, receiving interactive styles etc.? Would you be open for a (new, so we can close this one) PR like that? |
I'd like to get @lindsaym-fa's opinion on this to see if we want to keep using tags in |
Definitely a lot of ways we can tackle this, but I'm of the mind that tags are well-suited for It makes sense to me for Within a contained input context, however, arrow keys are a more familiar way to control which element has "focus" while maintaining true focus on the input. That's the pattern I'd expect to see in |
That makes sense. So could the following be an approach:
I could at take over the first part inside this PR and could create an issue then for the second part? |
A very interesting discussion.
|
Current behaviour
Remove buttons in tags are currently not reachable via keyboard, see https://shoelace.style/components/tag#removable
CleanShot.2023-11-23.at.17.40.39-converted.mp4
New behaviour
Remove buttons are reachable via keyboard, see https://shoelace-git-fork-mariohamann-next-font-awesome.vercel.app/components/tag#removable
CleanShot.2023-11-23.at.17.45.30.mp4
As test is provided which fails before the change. I wanted to utilize window.activeElement but I believe it doesn't play well with ShadowDOM, so I went with the emit solution.