Summary: | Pressing Arrow keys behaves incorrectly in quicksearch | ||
---|---|---|---|
Product: | [Applications] krusader | Reporter: | S-trace |
Component: | general | Assignee: | Nikita Melnichenko <nikita+kde> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | alex.bikadorov, nikita+kde, toni.asensi |
Priority: | NOR | ||
Version: | 2.6.0 | ||
Target Milestone: | --- | ||
Platform: | Ubuntu | ||
OS: | Linux | ||
Latest Commit: | https://commits.kde.org/krusader/8d08925dac3f340a43476b9034e5072952a59fe6 | Version Fixed In: | |
Attachments: | revert patch |
Description
S-trace
2017-10-26 17:23:00 UTC
Created attachment 108621 [details]
revert patch
I confirm the regression. Thanks to S-trace for finding a commit. As a quick fix, I created a revert patch, resolved conflict on top of master and checked that it's back to normal. Please find the patch attached.
Alex, what was a motivation for the original commit 210ca94?
About issue 1: This is intended. While typing in the quicksearch field this is the widget which has the input focus. I would claim this is normal behaviour expected from any other text field. Because it has the focus all keyboard input it can handle is handled. And <arrow_left> and <arrow_right> are useful keys here because they can move the cursor. Other users (at least myself) would like to keep this behaviour. And you can still type <Escape> to get the focus back to the panel. Keyboard-only usage is possible without any problems. [And there are other potential conflicts, e.g. <CTRL+arrow_left>. It would be a mess to handle all these in a special way - for whatever the user expects it to be.] About issue 2: This is indeed a bug. I will look into it. >Alex, what was a motivation for the original commit 210ca94? It is referenced in the commit message -> Bug 375639. Git commit b569e558c72102886d7bd268b74ae59a0e5f0776 by Alexander Bikadorov. Committed on 29/10/2017 at 20:29. Pushed by abikadorov into branch 'master'. Panel: Handle key events by search bar if it is open and text field is focused And some code cleaning again. FIXED: [ 386217 ] Fix moving cursor with up/down to item not matching in quicksearch M +6 -4 krusader/Panel/PanelView/krinterbriefview.cpp M +34 -19 krusader/Panel/PanelView/krview.cpp M +12 -2 krusader/Panel/PanelView/krview.h M +7 -1 krusader/Panel/krsearchbar.cpp M +23 -6 krusader/Panel/krsearchbar.h https://commits.kde.org/krusader/b569e558c72102886d7bd268b74ae59a0e5f0776 Issue 2 fixed. Please test. (In reply to Alex Bikadorov from comment #4) > Issue 2 fixed. Please test. Works fine. But about Issue1: Of course, moving cursor with arrow keys is very useful. But it's confusing when same key do different things in very similar usage patterns: A: 1. Type a text for quick search 2. Matching directory was selected in the panel 3. Press right - nothing happened B: 1. Type a text for quick search 2. Matching directory was selected in the panel 3. Press up to select another matching directory 4. Press right - current directory changed to the directory selected in panel C: 1. Press up or down to select any directory 2. Press right - current directory changed to the directory selected in panel Maybe it will be useful to add a check for this edge case? I see several variants: 1. Check if right arrow was pressed when cursor already at the end of text in quicksearch and previous pressed key was not the left or right arrow - if it is, change directory to the selected (fixes a possible problem when user moves cursor using long press of arrow keys and just wanted to move it to end of typed in quicksearch field text). 2. Check if right arrow was pressed when cursor already at the end of text in quicksearch - if it is, change directory to the selected (very simple to implement, i think) 3. Check if any cursor movement by arrows was performed in this quicksearch session, if no - change directory to highlighted on right arrow press. Thank you. (In reply to S-trace from comment #5) I agree with S-trace that the current behavior is slightly inconsistent. As a developer, I totally understand Alex's concern about text field behavior expectation, however from user point of view my daily usage pattern has been disrupted. For example, whenever I want to navigate to the dir data/projects/wdirs/krusader I simply press d > p r > w > k r > After the change I have to press d Esc > p r Esc > w Esc > k r Esc > As you see, the number of keystrokes has significantly increased. If I accidentally mistyped, I simply use Backspace and no problem. Arrow navigation is one of the most powerful features in Krusader. I hope we can keep it this way. I also encourage the devs to consider different variants as S-trace suggested. Personally, I'm for #3 as it seems like it's reflecting user's intentions - for quick navigation simply press right arrow and go, however if the user pressed left arrow at least once, he/she wants to edit the search expression and accidental right arrow in the end won't spoil his/her efforts. It's also simple to implement with 2 states - initial A and text editing B. Start with A, switch to B whenever left arrow is pressed. Alex Bikadorov wrote: > This is intended. While typing in the quicksearch field this is the widget > which has the input focus. I would claim this is normal behaviour expected > from any other text field. [...] I expect the same behaviour, too. Nikita Melnichenko wrote: > whenever I want to navigate to the dir data/projects/wdirs/krusader I > simply press > d > p r > w > k r > > After the change I have to press > d Esc > p r Esc > w Esc > k r Esc > > [...] If it can be useful: in those cases I press d Enter p r Enter w Enter k r Enter Perhaps it works in your case :-? (In reply to Toni Asensi Esteve from comment #7) > If it can be useful: in those cases I press > > d Enter p r Enter w Enter k r Enter > > Perhaps it works in your case :-? Toni, thanks for your suggestion. IMO, pressing Enter is insecure as you can unintentionally run some executable. Right arrow protects you from that. What is the status of this ticket? If I understand the comments correctly, it is unresolved? No response, changing status. (In reply to Christoph Feck from comment #10) > No response, changing status. Hi Christoph, IMO, it's more like CONFIRMED (as we all agree the product behaves much differently than it was for years before the change) or WONTFIX status - depends on the Krew decision. I hope they'll consider one of the S-trace's suggestions as they seem to be reasonable trade-offs. Thanks! BTW, I don't mind to work on a change to bring the proposal from comment #6 to life, however we must agree on the behavior spec first. I need a confidence that my patch won't be declined because of differences in views on UI. > however we must agree on the behavior spec first. Mmm... Alex Bikadorov explained the reasons of the behavior in: https://bugs.kde.org/show_bug.cgi?id=386217#c2 Well, it's not like i (or anybody else) has veto right. Under the assumption it is only this one case -just typed something in quick search and pressing right arrow key- and you even provide the patch yourself, I can live with it. I'm only concerned about more complicated code and inconsistent user-experience, including another bug report in the future criticising this change again. And I only have little time for this project anymore. But if you wanna work on it, go ahead. What do you think, Toni? I think the same, Alex, things would get more complicated. Also, it should be also taken into account the "brief view" (Alt+Shift+B) of Krusader, where the right arrow key is normally used to lead you to other files. There is a space for a 4th checkbox in the Settings dialog (Panel -> General -> Search Bar) right under "Case sensitive" - I can add a setting to control the behavior. Toni, is it ok with you? S-trace, Alex, Toni - please share your opinion on the best option. If get what Alex said in comment #14, he's for option #3. I'm for option #3 too. Yes, option #3. A new checkbox is also great. Got it. I'm pretty packed these days, however I scheduled to work on this on Jan 27th. In case anyone decides to pick this up before, please let me know. The easiest part, i.e. code changes, is done. I also fixed another bug related to a search bar in my branch. Now I need to figure out how to create a pull request... bear with me. Git commit 8d08925dac3f340a43476b9034e5072952a59fe6 by Nikita Melnichenko. Committed on 28/01/2018 at 06:19. Pushed by melnichenko into branch 'master'. Panel: quick navigation with right arrow when search bar is used FIXED: [ 386217 ] Pressing Arrow keys behaves incorrectly in quicksearch Fixes a usability regression introduced in 210ca94. Checking if cursor is in the end of the line is needed for the case a mouse is used to position the cursor. M +21 -1 krusader/Panel/krsearchbar.cpp M +3 -1 krusader/Panel/krsearchbar.h https://commits.kde.org/krusader/8d08925dac3f340a43476b9034e5072952a59fe6 |