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

Investigate focus change and selection update onModelRootChanged #1194

Closed
planger opened this issue Dec 13, 2023 · 1 comment · Fixed by eclipse-glsp/glsp-client#313
Closed

Investigate focus change and selection update onModelRootChanged #1194

planger opened this issue Dec 13, 2023 · 1 comment · Fixed by eclipse-glsp/glsp-client#313
Assignees
@martin-fleck-at
Labels
bug Something isn't working
Projects
GLSP kanban

Comments

@planger
Copy link
Member

planger commented Dec 13, 2023

See #1186 (reply in thread) for the origin of this issue.

We should investigate:

  1. Why do we update the selection onModelRootChanged to an empty set? Imho this should be reconsidered if that's necessary / useful. See SelectionService.modelRootChanged?
  2. Why does the update of the selection impact / steel the focus as reported in the discussion linked above?
@planger planger added the bug Something isn't working label Dec 13, 2023
@planger planger added this to New in GLSP kanban via automation Dec 13, 2023
@planger planger assigned planger and martin-fleck-at and unassigned planger Jan 11, 2024
@planger planger moved this from New to To do in GLSP kanban Jan 11, 2024
martin-fleck-at added a commit to eclipse-glsp/glsp-client that referenced this issue Jan 17, 2024
@martin-fleck-at
GLSP-1194: Do not send out selection changes if only the root changed
0f46b7c 
When we receive a model update and the selection state (ids of selected
elements) stays the same, we should not send out a selection changed
event. Code interested in model updates can listen to root changes
manually. This also prevents Theia from permanently updating the
selection if only the root element was changed.

Fixes eclipse-glsp/glsp#1194
martin-fleck-at added a commit to eclipse-glsp/glsp-client that referenced this issue Jan 18, 2024
@martin-fleck-at
GLSP-1194: Do not send out selection changes if only the root changed
aa3c9eb 
When we receive a model update and the selection state (ids of selected
elements) stays the same, we should not send out a selection changed
event. Code interested in model updates can listen to root changes
manually. This also prevents Theia from permanently updating the
selection if only the root element was changed.

Fixes eclipse-glsp/glsp#1194
@martin-fleck-at martin-fleck-at mentioned this issue Jan 18, 2024
Merged
@martin-fleck-at
Copy link
Contributor

I opened a PR that no longer sends a selection update when we only change the root but all the selected elements stay the same. I think this was a remnant of some older code that we no longer need. This also prevents the Theia-GLSP-Selection-Forwarder from being triggered all the time. We still do the onModelRootChanged -> updateSelection as we want to re-calculate the selection based on the new root to see if the selection can still be applied. (the empty sets just means no change and we use the previous state). As for the focus change I can no longer reproduce this, changing the selection in Theia does not change the active widget or steal focus as far as I can tell.

martin-fleck-at added a commit to eclipse-glsp/glsp-client that referenced this issue Jan 18, 2024
@martin-fleck-at
GLSP-1194: Do not send out selection changes if only the root changed
c05e103 
When we receive a model update and the selection state (ids of selected
elements) stays the same, we should not send out a selection changed
event. Code interested in model updates can listen to root changes
manually. This also prevents Theia from permanently updating the
selection if only the root element was changed.

Fixes eclipse-glsp/glsp#1194
martin-fleck-at added a commit to eclipse-glsp/glsp-client that referenced this issue Jan 18, 2024
@martin-fleck-at
GLSP-1194: Do not send out selection changes if only the root changed
8edd5de 
When we receive a model update and the selection state (ids of selected
elements) stays the same, we should not send out a selection changed
event. Code interested in model updates can listen to root changes
manually. This also prevents Theia from permanently updating the
selection if only the root element was changed.

Fixes eclipse-glsp/glsp#1194
martin-fleck-at added a commit to eclipse-glsp/glsp-client that referenced this issue Jan 19, 2024
@martin-fleck-at
GLSP-1194: Do not send out selection changes if only the root changed (…
87e78dc 
…#313)

When we receive a model update and the selection state (ids of selected
elements) stays the same, we should not send out a selection changed
event. Code interested in model updates can listen to root changes
manually. This also prevents Theia from permanently updating the
selection if only the root element was changed.

Fixes eclipse-glsp/glsp#1194
GLSP kanban automation moved this from To do to Done Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@martin-fleck-at martin-fleck-at

Labels
bug Something isn't working
Projects
No open projects
GLSP kanban
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GLSP-1194: Do not send out selection changes if only the root changed eclipse-glsp/glsp-client
2 participants
@planger @martin-fleck-at