Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disabled Tag can be removed with keyboard nav #7080

Open
abeljohn opened this issue Sep 24, 2024 · 4 comments · May be fixed by #7394
Open

Disabled Tag can be removed with keyboard nav #7080

abeljohn opened this issue Sep 24, 2024 · 4 comments · May be fixed by #7394
Labels
bug Something isn't working

Comments

@abeljohn
Copy link
Contributor

Provide a general summary of the issue here

When a React Aria Components TagList contains only disabled items, the last disabled Tag can be removed by tabbing backwards into the list and pressing delete.

🤔 Expected Behavior?

Disabled Tags should not be removed using the delete key.

😯 Current Behavior

Disabled Tags can be focused by tabbing backwards, then removed by pressing delete.

See gif and CodeSandbox below
2024-09-24 10 02 32

💁 Possible Solution

This seems to be caused by a combination of two issues in useTag:

  1. disabled Tags do not set tabIndex to -1
  2. the onKeyDown event handler does not check if the tag is disabled before calling onRemove

🔦 Context

I can work around this by validating the removed Tag IDs in my onRemove event handler, but it would be nice if this worked without needing validation, like how the inner remove button automatically has isDisabled set to prevent removing disabled Tags via clicking.

🖥️ Steps to Reproduce

https://codesandbox.io/p/sandbox/loving-golick-8mnndy

Version

"react-aria-components": "1.3.3"

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macOS Sonoma 14.5

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response
@snowystinger
Copy link
Member

This appears to be fixed on main and will be in the next release.
Here's a sandbox on the nightly. https://codesandbox.io/p/sandbox/autumn-cloud-f4pk33 Please let me know if I've messed up the instructions

@abeljohn
Copy link
Contributor Author

Thanks for looking into this. I'm still able to reproduce the issue on the nightly build. It only happens when all the remaining Tags are disabled.
I've simplified the sandbox to have all keys disabled and added clearer instructions: https://codesandbox.io/p/sandbox/compassionate-jasper-2rks9m

@snowystinger snowystinger added the bug Something isn't working label Sep 25, 2024
@snowystinger
Copy link
Member

Awesome, thank you for providing that. I can reproduce it now.

For whoever fixes this, the focus shouldn't go to the last Tag; it should go to the group as it does for forward navigation. In addition, hitting delete should also not remove the tag, but theoretically, if we fix the first issue, the other should become not possible (though another bug could make it so, so it is better to defensively handle that if possible)

@github-project-automation github-project-automation bot moved this to ✏️ To Groom in RSP Component Milestones Oct 2, 2024
@LFDanLu LFDanLu moved this from ✏️ To Groom to 🩹 To Triage in RSP Component Milestones Oct 30, 2024
@minzzang144
Copy link

Can I try it?

@minzzang144 minzzang144 linked a pull request Nov 19, 2024 that will close this issue
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
RSP Component Milestones
Status: 🩺 To Triage
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: useTag hook minzzang144/react-spectrum
3 participants
@snowystinger @abeljohn @minzzang144