D20152: Scroll to item if it's not visible on keyPress

[Elvis Angelaccio]

elvisangelaccio added a comment.


  In D20152#446323 <https://phabricator.kde.org/D20152#446323>, @trmdi wrote:
  
  > In D20152#446262 <https://phabricator.kde.org/D20152#446262>, @elvisangelaccio wrote:
  >
  > > In D20152#445778 <https://phabricator.kde.org/D20152#445778>, @trmdi wrote:
  > >
  > > > In D20152#445537 <https://phabricator.kde.org/D20152#445537>, @elvisangelaccio wrote:
  > > >
  > > > > We always call `KItemListSelectionManager::setCurrentItem()` before calling `scrollToItem()`, so we should probably do it also here.
  > > >
  > > >
  > > > I don't think it is needed because it was set in the above code block <https://phabricator.kde.org/source/dolphin/browse/master/src/kitemviews/kitemlistcontroller.cpp$432>.
  > >
  > >
  > > Only if `m_selectionManager->currentItem() != index`, but this won't be true when the item is not visible. Which is why you want to move the `scrollToItem()` call outside this if() block in the first place, isn't it?
  >
  >
  > When `m_selectionManager->currentItem() == index`, the `setCurrentItem(index)` will do nothing.
  
  
  Fair enough. But anyway I found a bigger problem: with this patch we will scroll unconditionally, even when pressing unrelated keys such as `Key_Escape` or `Key_Menu`.

REPOSITORY
  R318 Dolphin

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

To: trmdi, #dolphin, ngraham, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, alexde, 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/20190409/a21247c1/attachment.htm>