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

feat: shift-click to unselect #451

Merged
merged 7 commits into from
Jan 13, 2025
Merged

feat: shift-click to unselect #451

merged 7 commits into from
Jan 13, 2025

Conversation

Vtec234
Copy link
Member

@Vtec234 Vtec234 commented May 23, 2024

A simple(istic) implementation of an easier way to unselect hypotheses or types in a goal: shift-clicking anywhere in the info display that "does nothing" (i.e., doesn't select something else or instigate a popup) unselects all items. This might still be a bit annoying, since a shift-misclick will clear everything, but better than nothing. (I am open to suggestions for alternatives.)

Implements context menu entries to (un)select subexpressions in the tactic state, in particular to unselect all of them at once.

CC @PatrickMassot

Screen.Recording.2024-05-22.at.11.25.19.PM.mov

@mhuisi
Copy link
Collaborator

mhuisi commented May 23, 2024

Unrelated, but it would also be nice to have a context menu entry for this eventually.

@PatrickMassot
Copy link
Contributor

This is a tricky question. The misclick issue seems pretty serious. Adding a button only for this seems really cluttering. Maybe Marc’s context menu suggestion is a good compromise. It’s not super discoverable but the current proposal is worse from this point of view.

Copy link
Collaborator

@mhuisi mhuisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if you're fine with the UX.

@Vtec234
Copy link
Member Author

Vtec234 commented Jan 12, 2025

Following up on the discussion, I implemented the alternative proposal: instead of shift-clicking on the background, unselecting everything is now a context menu option. We can also (un)select specific subexpressions using the context menu.

A remaining usability issue is that when you do this, the type popups still show up (if you have them enabled, but that is the default). Maybe they should be hidden on certain contextmenu events?

Screen.Recording.2025-01-12.at.5.25.06.PM.mov

Copy link
Collaborator

@mhuisi mhuisi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main two usability issues I've found when testing this PR are that for some reason, when selecting or unselecting a term using the context menu, the term becomes text-selected, and the aforementioned popup. Neither of these are major, so feel free to proceed.

vscode-lean4/package.json Show resolved Hide resolved
lean4-infoview-api/src/infoviewApi.ts Show resolved Hide resolved
lean4-infoview/src/infoview/goalLocation.tsx Outdated Show resolved Hide resolved
vscode-lean4/package.json Outdated Show resolved Hide resolved
vscode-lean4/package.json Outdated Show resolved Hide resolved
vscode-lean4/package.json Show resolved Hide resolved
lean4-infoview/src/infoview/info.tsx Show resolved Hide resolved
@Vtec234
Copy link
Member Author

Vtec234 commented Jan 13, 2025

(The auto-selection issue is due to #312). I fixed both usability issues by having the tooltip hide on contextmenu, and removing the auto-selection in the new clickedContextMenu handler. More testing is definitely needed, but merging in order to make a pre-release.

@Vtec234 Vtec234 merged commit 0df0cb2 into master Jan 13, 2025
4 checks passed
@Vtec234 Vtec234 deleted the shift-unclick branch January 13, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants