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

AddToSelection doesn't seem to use "mustSetFocus" #765

Closed
tmgarcia opened this issue Nov 22, 2023 · 4 comments
Closed

AddToSelection doesn't seem to use "mustSetFocus" #765

tmgarcia opened this issue Nov 22, 2023 · 4 comments

Comments

@tmgarcia
Copy link

The docs for addToSelection (here) mentions that mustSetFocus is an optional 2nd arg that defaults to true, similar to what the single selectNode function does.

I might be misunderstanding what "setting focus" means in this context, but I've been assuming that it means expanding collapsed parent nodes to make the selected node visible, since that's what selectNode does.
However, using addToSelection doesn't seem to have that same behavior, even if I explicitly pass 'mustSetFocus' as true.

Example jsfiddle here:
https://jsfiddle.net/fj37ckye/
Clicking the "selectNode" button expands Node A to make the selected "Node C" visible.
Clicking "addToSelection" though does not expand either Node D or Node G to make the selected nodes E or H visible.

This isn't a huge issue as I've been able to manually open nodes down the chain myself to work around it, but I wanted to at least find out whether this is a bug or just me misunderstanding intended behavior.

@mbraak
Copy link
Owner

mbraak commented Nov 23, 2023

The option mustSetFocus doesn't mean that it collapses the parent elements. It sets the active element as described here: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus

The method selectNode always collapses the parent nodes, and addToSelection doesn't. I'll update addToSelection so it will do the same as selectNode.

@mbraak
Copy link
Owner

mbraak commented Nov 23, 2023

Work in progress: #766

@mbraak
Copy link
Owner

mbraak commented Nov 26, 2023

This is fixed in version 1.8.0

@mbraak mbraak closed this as completed Nov 26, 2023
@tmgarcia
Copy link
Author

Oh my gosh thank you so much for turning that around so quickly!
And thank you for the clarification, I don't know why I didn't realize "focus" just meant standard html focus, but that absolutely makes sense.

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

No branches or pull requests

2 participants