Review Request: Make it possible to clear the selection using Esc; make it easier to cancel keyboard searches

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104709/
-----------------------------------------------------------

(Updated April 24, 2012, 7:22 p.m.)


Review request for Dolphin.


Changes
-------

OK, I did what you suggested. The unit tests still pass, which shows that I should probably add some unit tests for the currentChanged() signal ;-)

Funnily enough, the change you suggested fixes another bug that I was not aware of before: when deleting the current item, the view does not underline the new current item.

Note that I used "emit currentChanged(m_currentItem, -1);" for the second signal emission, with a hardcoded previous value of -1. It only matters that the value is invalid, so I think that adding another variable to store that value might not be worth the effort (moreover, we know that we always use -1 for invalid values).

Any other comments? When I commit this, I will split the patch into two commits (first "Esc-press" handling and then "remove current item" handling) to make it easier to match the changes with changelog entries and bug reports.


Description
-------

In Dolphin 2.x, it is currently not possible to clear the selection using the keyboard. In Dolphin 1.x, pressing Esc cleared the selection. This patch restores that behaviour.

Moreover, I think it makes sense to cancel the current keyboard search when pressing Esc (some users noticed that it is currently not possible to cancel a keyboard search, see https://bugs.kde.org/show_bug.cgi?id=297458).

Finally, when I had added the function KItemListKeyboardSearchManager::cancelSearch(), I thought I could just make it a slot and invoke it when the current item is removed, which fixes the problem that the keyboard search is not reset when the view's URL is changed.


This addresses bugs 297458, 297488 and 298742.
    http://bugs.kde.org/show_bug.cgi?id=297458
    http://bugs.kde.org/show_bug.cgi?id=297488
    http://bugs.kde.org/show_bug.cgi?id=298742


Diffs (updated)
-----

  dolphin/src/kitemviews/kitemlistcontroller.cpp 79a6ecf 
  dolphin/src/kitemviews/kitemlistkeyboardsearchmanager.cpp 592605a 
  dolphin/src/kitemviews/kitemlistkeyboardsearchmanager_p.h d6a6686 
  dolphin/src/kitemviews/kitemlistselectionmanager.cpp 79c3370 

Diff: http://git.reviewboard.kde.org/r/104709/diff/


Testing
-------

Works for me.


Thanks,

Frank Reininghaus

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20120424/8e514093/attachment.htm>