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.

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

🐛 Selection change when selectable list focuses #7512

Open
alirezamirian opened this issue Dec 12, 2024 · 1 comment · May be fixed by #7513
Open

🐛 Selection change when selectable list focuses #7512

alirezamirian opened this issue Dec 12, 2024 · 1 comment · May be fixed by #7513

Comments

@alirezamirian
Copy link
Contributor

alirezamirian commented Dec 12, 2024

Provide a general summary of the issue here

When using useSelectableCollection with selectOnFocus set to true and with some selected keys, focusing the list for the first time changes the selection. It's easier to see it in action in the codesandbox linked below.

🤔 Expected Behavior?

When focusedKey is initialized upon first focus, the focused item should also be selected if selectOnFocus is true, but only if the focus is going to a key not already selected. i.e focusing a collection with initially selected items should not have the side effect of selection being changed.

😯 Current Behavior

Initial non-empty selection can change upon focus.

💁 Possible Solution

The bug is in the code that tries to initialize focusedKey upon the first focus:

if (manager.focusedKey == null) {
let navigateToFirstKey = (key: Key | undefined | null) => {
if (key != null) {
manager.setFocusedKey(key);
if (selectOnFocus) {
manager.replaceSelection(key);
}
}
};
// If the user hasn't yet interacted with the collection, there will be no focusedKey set.
// Attempt to detect whether the user is tabbing forward or backward into the collection
// and either focus the first or last item accordingly.
let relatedTarget = e.relatedTarget as Element;
if (relatedTarget && (e.currentTarget.compareDocumentPosition(relatedTarget) & Node.DOCUMENT_POSITION_FOLLOWING)) {
navigateToFirstKey(manager.lastSelectedKey ?? delegate.getLastKey?.());
} else {
navigateToFirstKey(manager.firstSelectedKey ?? delegate.getFirstKey?.());
}
} else if (!isVirtualized && scrollRef.current) {

If the key passed to navigateToFirstKey is either manager.lastSelectedKey or manager.firstSelectedKey selection should not be toggled, as the item that's about to get focused is already selected.

See #7513

🔦 Context

No response

🖥️ Steps to Reproduce

Self explanatory sandbox: https://codesandbox.io/p/sandbox/blissful-joliot-gysfg2

Version

@react-aria/[email protected]

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macOS

🧢 Your Company/Team

https://github.com/alirezamirian/jui

🕷 Tracking Issue

No response

alirezamirian added a commit to alirezamirian/react-spectrum that referenced this issue Dec 12, 2024
@alirezamirian alirezamirian changed the title Selection change when selectable list focuses 🐛 Selection change when selectable list focuses Dec 12, 2024
alirezamirian added a commit to alirezamirian/react-spectrum that referenced this issue Dec 13, 2024
@snowystinger
Copy link
Member

snowystinger commented Dec 17, 2024

Seems reasonable. I see Outlook's selection works like this, as does Mac Finder if I change windows and come back.
I don't quite know if I fully consider it a bug yet.

alirezamirian added a commit to alirezamirian/react-spectrum that referenced this issue Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants