D23716: When the selection is deselected, restart the keyboard search from the beginning

Elvis Angelaccio noreply at phabricator.kde.org
Sun Oct 6 12:24:42 BST 2019


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


  Patch seems to work fine, but the lack of commit message makes it hard to understand where the actual bugfix is. Please simplify the patch as mentioned inline.

INLINE COMMENTS

> kitemlistselectionmanager.cpp:181
> +    // setSelected(index, count);
> +    // but emitting once only selectionChanged
> +    const KItemSet previous = selectedItems();

Is `selectionChanged()` being emitted twice part of the bug? Or is this an unrelated refactoring that could go to another commit?

> kitemlistkeyboardsearchmanager.cpp:37-42
> +bool KItemListKeyboardSearchManager::shouldClearSearchIfInputTimeReached()
>  {
>      const bool keyboardTimeWasValid = m_keyboardInputTime.isValid();
>      const qint64 keyboardInputTimeElapsed = m_keyboardInputTime.restart();
> -    if (keyboardInputTimeElapsed > m_timeout || !keyboardTimeWasValid) {
> +    return (keyboardInputTimeElapsed > m_timeout) || !keyboardTimeWasValid;
> +}

This is an unrelated refactoring that should go in its own commit. Feel free to commit this single change without review.

REPOSITORY
  R318 Dolphin

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

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


More information about the kfm-devel mailing list