rework of keyboard widgets focus
AbandonedPublic

Authored by gengisdave on Dec 30 2019, 1:56 PM.

Details

Reviewers
yurchor
abika
asensi
Group Reviewers
Krusader
Summary

In Krusader you can move between widgets both with two configurable shortcuts "Focus up/down" (default : CTRL+SHIFT+UP/DOWN) and with an hardcoded CTRL+UP/DOWN, but neither can move through all the widgets.

This patch drops the hardcoded version in favor of the configurable one letting all widgets to be focused in sequence, all the unneeded code is removed.

The documentation is changed with the new shortcuts.

FIXED: [ 414831 ] cant focus embedded terminal with ctrl+down
BUG: 414831

Test Plan

With every or part of the widgets active, CTRL+SHIFT+UP/DOWN (or anything else configured as Focus up/down), must move between the widgets in the following order (from top to bottom):

  • Location Bar (edit mode on focus in, navigate mode on focus out)
  • File View
  • Sidebar (it is considered always beneath the file view, unregarding its real position)
  • Embedded emulator
  • Command line

Moreover, CTRL+UP/DOWN should not change widgets focus

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
gengisdave created this revision.Dec 30 2019, 1:56 PM
Restricted Application added a project: Documentation. · View Herald TranscriptDec 30 2019, 1:56 PM
Restricted Application added a subscriber: kde-doc-english. · View Herald Transcript
gengisdave requested review of this revision.Dec 30 2019, 1:56 PM

Thanks.

It would be nice to move shortcuts in keyboard-commands.docbook (~line 947) from Ctrl to Ctrl-Shift section ( ~line 1218) or make those sections coherent with this patch.

gengisdave updated this revision to Diff 72383.Dec 30 2019, 2:51 PM

Updated docbook as stated by @yurchor.

yurchor accepted this revision as: yurchor.Dec 30 2019, 2:58 PM
This revision is now accepted and ready to land.Dec 30 2019, 2:58 PM
asensi added a subscriber: asensi.Jan 15 2020, 10:49 PM

The base of the idea is good, although after applying changes... Krusader users are not able to keep using the handy Ctrl+ and Ctrl+ to move focus even if they redefine the Move Focus Up and Move Focus Down keys :-?

abika accepted this revision as: abika.Jan 27 2020, 5:46 PM
abika added a subscriber: abika.

Good improvement in my opinion!
My test results:

  • The CTRL+SHIFT+UP/DOWN shortcuts work perfectly. Only when unfocusing the navigation bar a second time, the edit mode stays on. It might be a widget problem, but i consider it minor and it happened also before this patch.
  • Then i set CTRL+UP/DOWN as alternative shortcuts for the same actions to see if this can be used for backwards compatibility: This does not work when trying to unfocusing the navigation bar, the embedded terminal or the command line. All these widgets are capturing the key press and the shortcut action is not activated.

So we have to consider that CTRL+UP/DOWN cannot be used anymore as before and users may complain about it. However, this did also not work for the embedded terminal (see bug report) and for the sidebar before this patch.

So we have to consider that CTRL+UP/DOWN cannot be used anymore as before and users may complain about it. However, this did
also not work for the embedded terminal (see bug report) and for the sidebar before this patch.

Well, without applying the current proposal, in the tests that I have just performed, if you need to use the embedded terminal emulator: it's handy to press e.g. Ctrl+Alt+E to show it, and then press Ctrl+Up and Ctrl+Down for going to the embedded terminal and back.

I've seen that a problem appears if the so-called "Command Line" is also shown, then Ctrl+Up and Ctrl+Down don't work as before. It's not usual to see that, as that Command Line does not offer the services that the embedded terminal and, at least in my case, the embedded terminal is preferred (because e.g. the results of the executed commands are shown in the same Krusader window and stay there for next operations).

The good thing would be that Ctrl+Up and Ctrl+Down were allowed to e.g. keep on leading to the embedded terminal and back.

nmel added a subscriber: nmel.May 28 2020, 7:51 AM

@gengisdave , I don't see this merged. Forgotten?

asensi requested changes to this revision.May 28 2020, 8:49 AM

Ctrl+Up and Ctrl+Down would stop working 😔 and IIRC Davide was trying to solve that.

This revision now requires changes to proceed.May 28 2020, 8:49 AM
nmel added a comment.May 30 2020, 6:06 AM

Thanks Toni, I missed that.

@gengisdave , could you rebase this on current master and send PR from your fork in gitlab? Maybe someone (like myself) could help you to fix the remaining issue.

gengisdave abandoned this revision.May 31 2020, 12:58 PM

Moving to Gitlab