Always scroll to item if it's not visible on keyPress, even if it's the current index.
Details
- Reviewers
ngraham elvisangelaccio - Group Reviewers
Dolphin - Commits
- R318:b7db272af2f2: Scroll to item if it's not visible on keyPress
Select the last item, scroll up to make it not visible > press the End key > Dolphin now scrolls to that item.
Diff Detail
- Repository
- R318 Dolphin
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 10684 Build 10702: arc lint + arc unit
@elvisangelaccio does this look good? And if so, should we land it on the stable branch?
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.
if (m_selectionManager->currentItem() != index) { ... m_selectionManager->setCurrentItem(index); ... } if (index < m_view->firstVisibleIndex() || index > m_view->lastVisibleIndex()) { m_view->scrollToItem(index); }
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.
See https://phabricator.kde.org/source/dolphin/browse/master/src/kitemviews/kitemlistselectionmanager.cpp$51
Before it is not visible, that setCurrentItem(index) function was called once before (must be called), and there is no change while we scroll up and press End again, so we don't need to call it again.
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.
This commit broke the PgUp and PgDn keys for me. They select, but no longer scroll. It also stops scrolling when using the up arrow key before reaching the top.
Sorry, I sent a new patch to fix this: https://phabricator.kde.org/D21573
Please review it.