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

Fix closing quick outline view #904

Merged
merged 1 commit into from Jan 30, 2024

Conversation

travkin79
Copy link
Contributor

There are a few cases, where updating the selection / cursor position in an open editor might fail. In those cases the solution from PR #901 might not work and the user could only close the quick outline view with the ESC key (the dialog has no x Button). Instead of first updating the editor's selection and then closing the quick outline view, I suggest to change the order, i.e. first close the quick outline view and then update the editor's selection.

@rubenporras
Copy link
Contributor

can you rebase?

@rubenporras
Copy link
Contributor

@mickaelistria , any opinion on this PR?

@mickaelistria
Copy link
Contributor

I have not tried it but I'm afraid it would close the dialog to close is navigating it with the keyboard, ie hitting down until one finds the element they want, which is a common use case (at least to me).
I think it's OK to close the dialog in case of a double-click, but for other operations (simple click, keyboard navigation), the dialog should remain open.

@travkin79
Copy link
Contributor Author

Hi @mickaelistria,
I explicitly tested both cases, (a) navigating in the quick outline view to the desired element with the mouse (simple click on the desired element) and (b) using the keyboard's up and down buttons to select the element to navigate to. Both cases work with this version. You're welcome to try it yourself.

The dialog is only closed if the user selected a certain element and did hit enter or clicked on it (or hits the ESC key).

@mickaelistria
Copy link
Contributor

OK, thanks for confirming.

@mickaelistria mickaelistria merged commit d533c57 into eclipse:master Jan 30, 2024
2 checks passed
@travkin79 travkin79 deleted the patch-move-close branch January 30, 2024 16:00
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 this pull request may close these issues.

None yet

3 participants