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

Open the context menu on Alt-Return #133

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mkhl
Copy link

@mkhl mkhl commented Aug 29, 2023

Also on Alt-Space but that's usually handled by Mutter.
Also on Alt-Number, and additionally handle Ctrl-Number.

Closes #126

Currently the number keys are handled by the window: pressing one will focus the corresponding tile and releasing the key will activate that tile.
Return and Space are handled by the tile itself.

This change moves responsibility for handling key-released events of number keys from the window to the tiles. The key-pressed event focuses the tile so it can handle the subsequent key-released event. The tiles don't know their corresponding number but since the tile is focused it can assume that the number key was meant for it.

Also on Alt-Space but that's usually handled by Mutter.
Also on Alt-Number, and additionally handle Ctrl-Number.

Closes sonnyp#126

Currently the number keys are handled by the window:
pressing one will focus the corresponding tile
and releasing the key will activate that tile.
Return and Space are handled by the tile itself.

This change moves responsibility for handling
key-released events of number keys
from the window to the tiles.
The key-pressed event focuses the tile
so it can handle the subsequent key-released event.
The tiles don't know their corresponding number
but since the tile is focused it can assume
that the number key was meant for it.
Copy link
Owner

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Thanks! I have merged main into this branch, make sure to pull.

Please do not handle Alt+Space - as you said it's already taken.

Also revert the changes, 1/2/3/4 worked fine, no reason to change it.

The following are good to add in this PR

  • Alt+Return opens the contextual menu for the selected entry
  • Alt+Number to open the context menu for a numbered entry

Also do add your name to about.js contributos and update the keyboard shortcut window please

Comment on lines -96 to -100
eventController.connect("key-released", (self, keyval) => {
const button = getButtonForKeyval(keyval);
button?.activate();
return !!button;
});
Copy link
Owner

Choose a reason for hiding this comment

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

Your version doesn't do the same thing as this.

This highlights the entry before opening the browsing showing the user which browser is gonna open before Juncton closes. It's subtitle but you should be able to tell the difference.

Please revert

} else {
open(!(modifier_state & Gdk.ModifierType.CONTROL_MASK));
}
controller_key.set_state(Gtk.EventSequenceState.CLAIMED);
Copy link
Owner

Choose a reason for hiding this comment

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

TypeError: controller_key.set_state is not a function

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.

Feature Request: Keyboard Shortcuts for Desktop Actions
2 participants