D24505: Avoid emitting twice twice selectionChanged when keyboard changes the selection

Elvis Angelaccio noreply at phabricator.kde.org
Sun Oct 13 15:08:47 BST 2019


elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  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.

INLINE COMMENTS

> kitemlistcontroller.cpp:480
>      } else {
> -        index = m_model->indexForKeyboardSearch(text, currentIndex);
> +        index = m_model->indexForKeyboardSearch(text, 0);
>      }

Can you explain the reason for this change?

> kitemlistselectionmanager.cpp:178-181
> +    // Equivalent to:
> +    // clearSelection();
> +    // setSelected(index, count);
> +    // but emitting once only selectionChanged

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

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D24505

To: meven, elvisangelaccio, #dolphin
Cc: kfm-devel, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191013/b4b106d1/attachment.htm>


More information about the kfm-devel mailing list