Key handling bug and a QoL improvement
ClosedPublic

Authored by rade on Mar 20 2018, 6:36 PM.

Details

Summary

Added code to filter both return and and escape keys. Before this sometimes they would be added to the search string. I.e opening the menu and then pressing return, and then typing 'ca' would cause the return string to become '\rca' . With no indication to the user of the inital '\r'. This might only happen if no item is highlighted though. Still filtering it is the right choice.

I also added a quality of life improvement, in the form of enabling left arrow to close the menu. Before if you were three submenus deep and wanted to exit you would have to press left arrow twice to close the submenus, and then esc to close the main menu. Now you can just hit left arrow three times.

Test Plan

Ensure no regressions.

See if there are any other keys we want to filter such as tab.

Consider what to do when the user enters non printable character, how should they be displayed in the text field?

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
rade requested review of this revision.Mar 20 2018, 6:36 PM
rade created this revision.
rade updated this revision to Diff 30037.Mar 20 2018, 6:58 PM

fixed minor error

martinkostolny accepted this revision.Mar 21 2018, 12:50 AM

It seems to work nicely. Thanks!

Consider what to do when the user enters non printable character

I'm not sure about the tab key and non-printable characters. I'd probably filter them all out but I may be wrong about this.

This revision is now accepted and ready to land.Mar 21 2018, 12:50 AM
nmel accepted this revision.Mar 21 2018, 4:48 AM

Looks good. I can submit this to the branch.

NOTE: For this and other patches. If you want your name on the commit, please create a commit with a proper message, git format-patch it and attach the patch here. If you don't care much, I'll leave what arc tool has created for me from the code review and put your name in the message.
krusader/BookMan/krbookmarkhandler.cpp
536

Extra empty line - please remove.

rade updated this revision to Diff 30118.Mar 21 2018, 12:08 PM

removed the extra newline.

rade marked an inline comment as done.Mar 21 2018, 12:08 PM
rade added a comment.EditedMar 21 2018, 12:12 PM

It seems to work nicely. Thanks!

Consider what to do when the user enters non printable character

I'm not sure about the tab key and non-printable characters. I'd probably filter them all out but I may be wrong about this.

I agree that would be the right thing to do. My original thinking for choosing a permissive model was that I didn't want to accidentally filter out valid chinese/japanese/etc characters. And if the user hits tab or another unexpected character then that's their fault, and they should erase it manually or start over. However adding all the keys we _know_ we don't want to search for like tab etc to the filtering if statement is a good idea.

adding all the keys we _know_ we don't want to search for like tab etc to the filtering if statement is a good idea

Agreed. This can also be done later so the diff can be pushed to the branch.

This revision was automatically updated to reflect the committed changes.