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

Bug: Keyboard shortcuts aren't working properly #11800

Closed
yaira2 opened this issue Mar 22, 2023 · 6 comments · Fixed by #11875
Closed

Bug: Keyboard shortcuts aren't working properly #11800

yaira2 opened this issue Mar 22, 2023 · 6 comments · Fixed by #11875

Comments

@yaira2
Copy link
Member

yaira2 commented Mar 22, 2023

Description

Keyboard shortcuts like delete don't work when you first select an item, requiring users to press the keyboard shortcut a second time.

Steps To Reproduce

  1. Navigate between a couple of different folders
  2. Select an item and press delete
  3. See if the delete dialog shows
  4. Press delete again and see that the dialog shows this time
  5. Click on a blank space to deselect the file
  6. Select the file again and see that the issue is back

Requirements

  • Fix issue where shortcuts don't work

Files Version

v2.4.55

Windows Version

Windows 11

Log file

NA

@yaira2 yaira2 added the bug label Mar 22, 2023
@yaira2 yaira2 moved this to 🔖 Ready to build in Files task board Mar 22, 2023
@yaira2 yaira2 changed the title Bug: Selecting items via selection rectangle breaks delete shortcut Bug: Keyboard shortcuts don't work the first time you use them Mar 22, 2023
@yaira2 yaira2 changed the title Bug: Keyboard shortcuts don't work the first time you use them Bug: Keyboard shortcuts aren't working properly Mar 22, 2023
@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Mar 22, 2023

@yaira2 that's my fault (related to #11743 ). The underlying issue is that OnPreviewKeyUp is not always fired.
(Lines causing the issue are 259 -> 264 of MainPage.xaml.cs)
We have two ways to fix this, I'm not sure which one is the best:
- we can handle it as we do when renaming items (keyReleased = (GetKeyState((int)e.Key) & KEY_DOWN_MASK) == 0;
- we can filter which command we want to be executed only once (which I think is the best). We can achieve this as follows keyReleased = e.Key is not VirtualKey.T or VirtualKey.P or ...;

Ignore this for the moment, the bug may be related to something else. I've been testing it for an hour and I can't find a real pattern to reproduce it consistently. Even with your Steps to reproduce sometimes it works and sometimes it doesn't

The issue seems to be that currentModifiers sometimes is set even when no modifier is pressed. In my case this happens when I use Alt + Tab to switch window. Menu is set when you hit Alt but it isn't unset when you release it because you are on a different window.

@cinqmilleans I suggest evaluating Modifiers status (or at least Menu) just before we execute the command. We may use GetKeyState(int key) (User32.dll) to do that. What do you think?

@yaira2 yaira2 closed this as completed Mar 22, 2023
@github-project-automation github-project-automation bot moved this from 🔖 Ready to build to ✅ Done in Files task board Mar 22, 2023
@yaira2
Copy link
Member Author

yaira2 commented Mar 28, 2023

This is still an issue

@yaira2 yaira2 reopened this Mar 28, 2023
@github-project-automation github-project-automation bot moved this from ✅ Done to 🆕 New in Files task board Mar 28, 2023
@yaira2 yaira2 moved this from 🆕 New to 🔖 Ready to build in Files task board Mar 28, 2023
@ferrariofilippo
Copy link
Contributor

It might be caused by the method we use to get the status of a key. I remember I had some issues using that.

We may use GetKeyState(int key) (User32.dll) to do that.

This might fix it

Anyway, are there any steps to reproduce or does it happen casually?

@hishitetsu
Copy link
Member

Reproduce by the following steps:

  1. Select an item.
  2. Press Delete key.
  3. You will see a dialog. Press Cancel.
  4. Deselect the item.
  5. Select an item again.
  6. Press Delete key.
  7. The dialog will not appear this time.

@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Mar 28, 2023

The problem is that Delete opens a modal and because of that OnPreviewKeyUp is called on that modal and not on the MainPage. This issue should involve all the actions that open modals.
We might include a field in IAction which tells if it will open a modal.

Note. If you press esc to close the modal, everything will work

Related actions:

  • DeleteAction
  • DecompressArchiveAction
  • OpenSettingsAction

There are probably some more

@hishitetsu
Copy link
Member

I have come up with a workaround and will submit a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants