Fix flow's hotkey not working correctly after changing in settings #565

jjw24 · 2021-07-09T12:07:27Z

Description
After changing the hotkey that triggers flow's query window, the query window that get triggered by new hotkey can not be used- unable to move up and down on the result list, unable to move the cursor on the query test box, the query text is not select if you have this option enabled, and query window's position is not in the same place as previous.

Changes

consolidated duplicate hotkey behaviour code into one class HotkeyMapper
added focus to custom query hotkey preview so can be used when previewing
behaviour should be the same for when a hot key/custom query hotkey is changed and when these hotkeys are loaded during startup

How to reproduce
Change hotkey. Trigger the query window, then unable to use it with the issues described above.

Tested with this PR:

changed hotkey and made sure new hotkey works afterwards
changed hotkey and check all 'Last Query Style' options are working afterwards
changed hotkey and set ignore hotkey in full screen mode
able to trigger Custom Query Hotkeys and query window position is correct

Close #166

taooceros · 2021-07-11T07:15:05Z

~~Could you share which part fixes the bug? It is a bit confuse this commit includes quite a lot refactor.~~

No problem I get the idea of this fix.

Flow.Launcher/Helper/HotKeyMapper.cs

taooceros · 2021-07-11T07:48:12Z

Flow.Launcher/Helper/HotKeyMapper.cs

+                    if (ShouldIgnoreHotkeys()) return;
+                    mainViewModel.MainWindowVisibility = Visibility.Visible;
+                    mainViewModel.ChangeQueryText(hotkey.ActionKeyword);


~~Could you also add mainViewModel.LastQuerySelected = false; to here because I do have encountered it is selected sometime.~~

That doesn't fix the issue. I will handle that once you have finished all the change you want. It will only happen when the current query text is the same as the expected plugin keyword text, which won't trigger the onTextChanged event.

Flow.Launcher/Helper/HotKeyMapper.cs

jjw24 · 2021-07-11T08:52:08Z

Will do those changes tomorrow, haven't got around to finishing this at all this weekend.

taooceros · 2021-07-12T10:54:50Z

By the way, do you think it would be better to change the use of the property LastQuerySelected. I think the usage is quite confusing.

Flow.Launcher/Helper/HotKeyMapper.cs

jjw24 · 2021-07-14T11:13:12Z

By the way, do you think it would be better to change the use of the property LastQuerySelected. I think the usage is quite confusing.

how is it confusing? i use it all the time, it makes sure the previous query is highlighted so i can start typing after triggered the window.

jjw24 · 2021-07-14T11:14:19Z

@taooceros ready to go, please review 😄

taooceros · 2021-07-14T14:20:05Z

By the way, do you think it would be better to change the use of the property LastQuerySelected. I think the usage is quite confusing.

how is it confusing? i use it all the time, it makes sure the previous query is highlighted so i can start typing after triggered the window.

It is confusing because when we don't want to select last query, we need to mark it as true to fake a selected. When we want the model to select last time query, we mark it false. Maybe a property like (Should)SelectLastQuery may be clearer.

taooceros

Do you think we shall change the select query property?

taooceros · 2021-07-14T14:24:20Z

Flow.Launcher/Helper/HotKeyMapper.cs

+            catch
+            {
+            }


We can remove the catch

cant remove it, catch is needed to go to return false at the bottom, you can just use catch but you duplicate code for no benefit, so may be just leave it?

taooceros · 2021-07-14T14:25:04Z

Flow.Launcher/HotkeyControl.xaml.cs

+            if (handler != null)
+                handler(this, EventArgs.Empty);


I like this way handler?.Invoke(this, EventArgs.Empty), but either is fine.

taooceros · 2021-07-14T14:29:08Z

Flow.Launcher/Languages/en.xaml

    <system:String x:Key="enable">Enable</system:String>
    <system:String x:Key="disable">Disable</system:String>
-    <system:String x:Key="actionKeywords">Action keyword:</system:String>
+    <system:String x:Key="actionKeywords">Query</system:String>


Is this reference to this? I think Query may be a bit confusing here.

good catch, i didnt realise that is hooked up to Action keyword field

jjw24 · 2021-07-15T10:37:18Z

By the way, do you think it would be better to change the use of the property LastQuerySelected. I think the usage is quite confusing.

how is it confusing? i use it all the time, it makes sure the previous query is highlighted so i can start typing after triggered the window.

It is confusing because when we don't want to select last query, we need to mark it as true to fake a selected. When we want the model to select last time query, we mark it false. Maybe a property like (Should)SelectLastQuery may be clearer.

oh ok, you mean the code not the actual functionality

jjw24 · 2021-07-15T11:20:57Z

@taooceros just done refactor based on my understanding of how the last query modes should work, have a look and let me know if it's correct #568

jjw24 · 2021-07-17T11:31:17Z

@taooceros i think this pr is ready to go, let's get this in and do the last query mode refactor seperately so we can release 1.8.0 since there is no functionality change for the refactor

taooceros · 2021-07-17T13:20:21Z

@taooceros i think this pr is ready to go, let's get this in and do the last query mode refactor seperately so we can release 1.8.0 since there is no functionality change for the refactor

Oh sorry, I was not at home these few days so haven't got through the whole pr again.
Sure. Let's keep them after 1.8.0.

jjw24 · 2021-07-17T13:43:49Z

@taooceros i think this pr is ready to go, let's get this in and do the last query mode refactor seperately so we can release 1.8.0 since there is no functionality change for the refactor

Oh sorry, I was not at home these few days so haven't got through the whole pr again.
Sure. Let's keep them after 1.8.0.

All good no stress, I was just doing a fyi

move and centralise the hotkey management calls

48fb0dc

jjw24 added the bug Something isn't working label Jul 9, 2021

jjw24 added this to the 1.8.0 milestone Jul 9, 2021

jjw24 self-assigned this Jul 9, 2021

jjw24 marked this pull request as draft July 9, 2021 12:07

taooceros requested changes Jul 11, 2021

View reviewed changes

jjw24 added 3 commits July 12, 2021 21:46

update display text to be more aligned with functionality

dd9cb2f

update method call to short-circuit logical operators

04d959c

add last query mode update to SetCustomPluginHotkey

b96a02b

taooceros reviewed Jul 13, 2021

View reviewed changes

Flow.Launcher/Helper/HotKeyMapper.cs Outdated Show resolved Hide resolved

jjw24 added 4 commits July 13, 2021 21:53

refactor- point CustomQueryHotkey to HotkeyMapper

8c31039

remove obsolete behaviour

86804bc

move CheckHotkeyAvailability to HotkeyMapper

20f849d

add focus for custom query hotkey preview

bcb17a0

jjw24 marked this pull request as ready for review July 14, 2021 11:09

jjw24 enabled auto-merge July 14, 2021 11:09

Merge branch 'dev' into fix_hotkey_change

689c42d

taooceros added the review in progress Indicates that a review is in progress for this PR label Jul 14, 2021

taooceros reviewed Jul 14, 2021

View reviewed changes

jjw24 added 3 commits July 15, 2021 20:06

update per review

33b34f2

add colon to Priority text for consistency

b96cc7a

fix hotkey unavailable text

4ba8296

jjw24 mentioned this pull request Jul 17, 2021

Release 1.8.0 | Plugin 2.0.0 #363

Merged

taooceros removed the review in progress Indicates that a review is in progress for this PR label Jul 17, 2021

taooceros approved these changes Jul 17, 2021

View reviewed changes

jjw24 merged commit 8d23f98 into dev Jul 17, 2021

jjw24 deleted the fix_hotkey_change branch July 17, 2021 13:23

taooceros mentioned this pull request Jul 18, 2021

fix Custom Plugin Hotkey not deselect text #570

Merged

taooceros mentioned this pull request Oct 18, 2021

Fix empty query #749

Merged

Uh oh!

Fix flow's hotkey not working correctly after changing in settings #565

Fix flow's hotkey not working correctly after changing in settings #565

Uh oh!

Conversation

jjw24 commented Jul 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taooceros commented Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

taooceros Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jjw24 commented Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taooceros commented Jul 12, 2021

Uh oh!

Uh oh!

jjw24 commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jjw24 commented Jul 14, 2021

Uh oh!

taooceros commented Jul 14, 2021

Uh oh!

taooceros left a comment

Choose a reason for hiding this comment

Uh oh!

taooceros Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

jjw24 Jul 15, 2021

Choose a reason for hiding this comment

Uh oh!

taooceros Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

taooceros Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

jjw24 Jul 15, 2021

Choose a reason for hiding this comment

Uh oh!

jjw24 commented Jul 15, 2021

Uh oh!

jjw24 commented Jul 15, 2021

Uh oh!

jjw24 commented Jul 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taooceros commented Jul 17, 2021

Uh oh!

jjw24 commented Jul 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jjw24 commented Jul 9, 2021 •

edited

Loading

taooceros commented Jul 11, 2021 •

edited

Loading

taooceros Jul 11, 2021 •

edited

Loading

jjw24 commented Jul 11, 2021 •

edited

Loading

jjw24 commented Jul 14, 2021 •

edited

Loading

jjw24 commented Jul 17, 2021 •

edited

Loading