Avoid emitting twice twice selectionChanged when keyboard changes the selection
Needs ReviewPublic

Authored by meven on Wed, Oct 9, 8:16 AM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
Summary

Also take into account searchFromNextItem in KItemListController::slotChangeCurrentItem

Diff Detail

Repository
R318 Dolphin
Branch
arcpatch-D24505
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17669
Build 17687: arc lint + arc unit
meven created this revision.Wed, Oct 9, 8:16 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptWed, Oct 9, 8:16 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
meven requested review of this revision.Wed, Oct 9, 8:16 AM
elvisangelaccio requested changes to this revision.Sun, Oct 13, 2:08 PM

I still don't understand if we are fixing an issue or if this is just a removal of a redundant signal. AFAICS we are only removing a selectionChanged() instance called by clearSelection() in "search mode", but this signal is also emitted in many other places (e.g. is emitted 3 times if I change selection with the mouse).

Please explain the rationale for this patch in the commit message.

src/kitemviews/kitemlistcontroller.cpp
480

Can you explain the reason for this change?

src/kitemviews/kitemlistselectionmanager.cpp
178–181

Please move this info in the apidox for replaceSelection() in the header file.

This revision now requires changes to proceed.Sun, Oct 13, 2:08 PM
meven updated this revision to Diff 67876.Mon, Oct 14, 6:25 AM
meven marked an inline comment as done.

Move comment to header

meven marked an inline comment as done.Mon, Oct 14, 6:25 AM
meven added inline comments.
src/kitemviews/kitemlistcontroller.cpp
480

This function way bugged :
The two branches of if (searchFromNextItem) both looked for the next keyboard with indexForKeyboardSearch(text, currentIndex (the first one with just a +1 modulo).
But when searchFromNextItem is false, we are supposed to start to look for the next indexKeyboard from the start of the list 0, not from the currentIndex.