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

Implement options for Ctrl/Shift middle click on tabs #1140

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

emvaized
Copy link
Contributor

Default is discard for Ctrl + middle click, and duplicate for Shift + middle click. Setting option to none will revert to the current behavior (serve as a regular middle click)

@@ -187,6 +187,33 @@ function onMouseDown(e: MouseEvent): void {
else if (e.button === 1) {
e.preventDefault()
Mouse.blockWheel()

if (e.ctrlKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't really following the same style as the existing code, where instead of returning, it makes sure that everything is an else if.

That would mean removing the return statements below, and then the existing lines 202 - 204 would become } else if ... {

Same deal on existing lines 215-217.

Copy link
Owner

Choose a reason for hiding this comment

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

Fair point, I'll update it after merge.

Copy link
Owner

@mbnuqw mbnuqw Jul 28, 2023

Choose a reason for hiding this comment

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

Hm, no, not fair point :), it's simpler to use return statement to get the 'fallback' logic (with 'none' values)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fallback to 'none' is exactly why I made it in return style. Some people may rely on Ctrl+Middle click to behave just as regular middle click (for whatever reason).

I was also thinking about writing it in a bit more complex way, so that actions like "duplicate" and "discard" would work with middle click drag selection— but this way it wouldn't work with "Edit title" action (and I really wanted to have it there), so I guess maybe next time. It's going to be too many checks to handle all possible actions this way.

@mbnuqw mbnuqw merged commit bd7784a into mbnuqw:v5 Jul 28, 2023
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.

3 participants