ParthJadhav / Verve

Verve is a lightweight and blazingly fast launcher for accessing and opening applications, files and documents. ⚡
GNU Affero General Public License v3.0
658 stars 25 forks source link

fix: shortcuts with `Option` key #31

Closed Ty3uK closed 1 year ago

Ty3uK commented 1 year ago

Related to #11.

event.key returns transformed char when option key is pressed.

For example, when user is trying to record shortcut Option+Shift+G, event.key returns Option+Shift+˝.

event.code returns KeyG instead. So getting substring with offset 3 fixes this issue.

This commit fixes another bug: when user records shortcut that already registered, window will hide. Adding unregister before start recording fixes this issue.

ParthJadhav commented 1 year ago

Ohh, I see. That's alright. We will resolve it in different PR. This doesn't seem to be that important as of now

ParthJadhav commented 1 year ago

Do you think this PR is ready to merge?

Ty3uK commented 1 year ago

Do you think this PR is ready to merge?

Give me another five minutes to check production release 🫡

Ty3uK commented 1 year ago

Got another bug when trying to set Option+Shift+Space shortcut

Ty3uK commented 1 year ago

Also, quick question: do I need to squash commits or you prefer to preserve full history of changes?

ParthJadhav commented 1 year ago

Also, quick question: do I need to squash commits or you prefer to preserve full history of changes?

Oh dw, I'll directly do Squash & Merge

ParthJadhav commented 1 year ago

TBH, The whole shortcut recorder function is messy & hard to understand.

Ty3uK commented 1 year ago

TBH, The whole shortcut recorder function is messy & hard to understand.

You can create issue and assign it to me, if you want. I can do some refactoring 🙂

ParthJadhav commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Ty3uK commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Maybe we can do some researching for global shortcuts from Rust itself? I think Tauri do this under the hood, but maybe some other library can do this better...

ParthJadhav commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Maybe we can do some researching for global shortcuts from Rust itself? I think Tauri do this under the hood, but maybe some other library can do this better...

Oh no, Everything works as expected. But the fact that we unregister the key before recording the shortcut is causing the problem.

What do you think?

ParthJadhav commented 1 year ago

You can create issue and assign it to me, if you want. I can do some refactoring 🙂

I think we can do it at later time when we find to be working on any issue related to the shortcut function... What are you opinion on it?

Ty3uK commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Maybe we can do some researching for global shortcuts from Rust itself? I think Tauri do this under the hood, but maybe some other library can do this better...

Oh no, Everything works as expected. But the fact that we unregister the key before recording the shortcut is causing the problem.

What do you think?

I've tried to comment unregister and it does nothing 🥲

image
Ty3uK commented 1 year ago

You can create issue and assign it to me, if you want. I can do some refactoring 🙂

I think we can do it at later time when we find to be working on any issue related to the shortcut function... What are you opinion on it?

Great catch! I think the same way 🙂

ParthJadhav commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Maybe we can do some researching for global shortcuts from Rust itself? I think Tauri do this under the hood, but maybe some other library can do this better...

Oh no, Everything works as expected. But the fact that we unregister the key before recording the shortcut is causing the problem. What do you think?

I've tried to comment unregister and it does nothing 🥲

image

If you just unbind the unregister line you would notice that the pre-existing shortcut still works after you try to set a shortcut which already exist.

Eg: if you try option + command + space without unregister then it would now listen for the default shortcut which was CMD + SHIFT + G... But with Unregister it wouldn't listen for anything..

ParthJadhav commented 1 year ago

Do you use Discord by any chance ? It would be easier to communicate there IMO 😄

Ty3uK commented 1 year ago

Do you use Discord by any chance ? It would be easier to communicate there IMO 😄

Yes 🙂 Ty3uK#0057

Ty3uK commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Maybe we can do some researching for global shortcuts from Rust itself? I think Tauri do this under the hood, but maybe some other library can do this better...

Oh no, Everything works as expected. But the fact that we unregister the key before recording the shortcut is causing the problem. What do you think?

I've tried to comment unregister and it does nothing 🥲

image

If you just unbind the unregister line you would notice that the pre-existing shortcut still works after you try to set a shortcut which already exist.

Eg: if you try option + command + space without unregister then it would now listen for the default shortcut which was CMD + SHIFT + G... But with Unregister it wouldn't listen for anything..

Got it. So maybe I just drop this line and we put this in another issue?

Also, we can do some refactoring just for resolving that kind of issues 🙂

ParthJadhav commented 1 year ago

I've tested it and everything works as expected... Except if anything else launches while setting the shortcut then we can only open the window by clicking the menu item

Maybe we can do some researching for global shortcuts from Rust itself? I think Tauri do this under the hood, but maybe some other library can do this better...

Oh no, Everything works as expected. But the fact that we unregister the key before recording the shortcut is causing the problem. What do you think?

I've tried to comment unregister and it does nothing 🥲

image

If you just unbind the unregister line you would notice that the pre-existing shortcut still works after you try to set a shortcut which already exist. Eg: if you try option + command + space without unregister then it would now listen for the default shortcut which was CMD + SHIFT + G... But with Unregister it wouldn't listen for anything..

Got it. So maybe I just drop this line and we put this in another issue?

Also, we can do some refactoring just for resolving that kind of issues 🙂

Yep, lets do that 🙌

ParthJadhav commented 1 year ago

Awesome, Let's merge it 🚀