flipperdevices / qFlipper

qFlipper — desktop application for updating Flipper Zero firmware via PC
https://update.flipperzero.one
GNU General Public License v3.0
1.06k stars 142 forks source link

Adding extra keyboard navigation #135

Closed gncnpk closed 1 year ago

gncnpk commented 1 year ago

I added a Keys.onPressed callback that deletes files with no confirmations when pressing the Delete key.

BoBeR182 commented 1 year ago

Shift + Delete for no confirmation. Delete should show a confirmation window.

gncnpk commented 1 year ago

Shift + Delete for no confirmation. Delete should show a confirmation window.

I'll add that in!

gsurkov commented 1 year ago

Although this feature seems useful, it has its problems. In order for it to work, we need to implement a reliable keyboard navigation for the file manager, otherwise the following questions will surely be asked by the users:

There also is an annoying problem with keyboard focus in Qml, which has to be solved to ensure that the file manager will always receive the keyboard input.

Right now, I have to close this PR, but if you are willing to address the above mentioned problems (namely, implementing a proper, if even very basic keyboard navigation), then please re-open it and I'll gladly accept it.

gncnpk commented 1 year ago

Hey @gsurkov I definitely want to help fix the problems you listed above and I appreciate your feedback, so I'll take a look today and see what I can implement! Thanks :)

gncnpk commented 1 year ago

I have implemented key events for entering and exiting directories using the Enter and Backspace key, I also set the GridView's current index to 0 and use the highlighted file as a cursor when navigating thru the directories/files. Also I can't reopen the PR so if someone can do that, that would be great!

gsurkov commented 1 year ago

@Smooklu thank you for your effort! I have re-opened the PR for you. The changes you have made are a big improvement. However, it doesn't work quite right yet.

For example:

  1. Open qFlipper
  2. Switch to the File manager tab.
  3. Use the keyboard navigation right away. Instead of changing the selection, the arrow <- and -> keys will change tabs. (despite the first element already being highlighted).
  4. Clicking onto a file icon (but not just inside the file view) enables the expected behaviour.

I think the user would expect the ability to use the keyboard right away, without clicking anywhere after selecting the file manager tab.

gncnpk commented 1 year ago

Hey @gsurkov, I'm gonna try to figure out how to get KeyboardFocus when the File Manager is viewed, so hopefully I get that figured out today :)

gncnpk commented 1 year ago

Fixes https://github.com/flipperdevices/qFlipper/issues/112

gncnpk commented 1 year ago

I added the code block

if(visible) {
            fileView.forceActiveFocus();
        }

to the onVisibleChanged section in FileManager.qml

So now when the File Manager becomes visible, it automatically grabs focus, let me know if you experience any other issues or have any other implementations I should add :)

gsurkov commented 1 year ago

Good job keeping on improving the feature! Speaking of additional implementations, this seem to go down a rabbit hole. :)

  1. When a "Delete file?" dialog appears, the user would expect to be able to use <Tab> or another common keyboard sequence to select the answer, then press <Enter> or <Esc> to confirm/cancel their selection.
  2. After closing the dialog (one way or another) the file manager loses focus again and does not react to keyboard input.
  3. It would be a nice touch to have a shortcut for uploading/downloading files (say, <Ctrl/Cmd-U> to upload a file and <Ctrl/Cmd-D> to download a file/directory).
gncnpk commented 1 year ago

So I've added a few more keyboard combos and added Ctrl + Tab to switch between options on the Confirmation Popup/Dialog and Return selects the option that is highlighted.

Ctrl-E -> Rename file/directory
Ctrl-N -> Create new directory
Ctrl-D -> Save/download file
Ctrl-L -> Upload file

I still must work on returning activeFocus to the file manager when the dialog/popup closes and I also must figure out on how to stop the Return keypress from entering a directory that is being created because it will spit the user back to the screen with Internal Flash and SD Card.

gncnpk commented 1 year ago

Okay, those two issues I was facing have been fixed now, please let me know if you see any other issues!

gncnpk commented 1 year ago

Added Ctrl-G for getting new files/directories or refreshing.

gsurkov commented 1 year ago

It looks like the feature is finally usable!

There are a couple of minor problems, namely:

  1. <Ctrl+Tab> instead of simply <Tab> to switch the dialog buttons. I believe I know why you chose that (Qt uses <Tab> internally to switch between UI elements), but it feels rather weird. Windows' explorer supports that shortcut, however (but not other OSes and file managers I have tested), so that feels at least partially justified. If you wish to try and make it work with the <Tab> key, please go ahead, otherwise I'll merge it as is.
  2. Accidentally pressing <Tab> in the file manager makes it lose focus. This is not a bug per se. A well-behaved application with keyboard support must give the user a visual feedback on the selected UI element and a reliable way to cycle between them by pressing <Tab> repeatedly. It's not obviously the case here (qFlipper wasn't designed with keyboard navigation in mind), so it feels more like a bug, thus it would be best to prevent the focus loss altogether. As with the previous point, fixing it is optional (I can merge it as is). Refer to this documentation for more info.
gncnpk commented 1 year ago

Yeah I noticed this with a few key combos, Rename was initially going to be Ctrl-R and Upload was going to be Ctrl-U but they seem to be binded already? I'll try to get Tab working since it does feel a little weird.

gsurkov commented 1 year ago

<Ctrl-R> and <Ctrl-U> are the existing (historical) shortcuts in qFlipper that force a check for updates (firmware and the application respectively). Not sure if anyone is using them, the only place they are mentioned is the top menu in MacOS.

gncnpk commented 1 year ago

I'll leave the bindings alone incase they are being used :)

gncnpk commented 1 year ago

It looks like the feature is finally usable!

There are a couple of minor problems, namely:

  1. <Ctrl+Tab> instead of simply <Tab> to switch the dialog buttons. I believe I know why you chose that (Qt uses <Tab> internally to switch between UI elements), but it feels rather weird. Windows' explorer supports that shortcut, however (but not other OSes and file managers I have tested), so that feels at least partially justified. If you wish to try and make it work with the <Tab> key, please go ahead, otherwise I'll merge it as is.
  2. Accidentally pressing <Tab> in the file manager makes it lose focus. This is not a bug per se. A well-behaved application with keyboard support must give the user a visual feedback on the selected UI element and a reliable way to cycle between them by pressing <Tab> repeatedly. It's not obviously the case here (qFlipper wasn't designed with keyboard navigation in mind), so it feels more like a bug, thus it would be best to prevent the focus loss altogether. As with the previous point, fixing it is optional (I can merge it as is). Refer to this documentation for more info.

Thank you for that documentation! That was the key to making Tab work perfectly! Everything should be all good now.

gsurkov commented 1 year ago

Awesome! I've tested it and everything seems to work as expected. Thanks a lot for your help!