Skip to content

Commit 6a52d99

Browse files
committed
Generalize the math in vertical panel scrolling
The changes introduced in #453 were designed for McBopomofo's use case where the candidate index is only moved by one or by a page. The issue found at openvanilla/openvanilla#119 showed that the math needed to be generalized. How tested: manually running through the scenarios mentioned in #453 to make sure that the UI invariants are not changed.
1 parent 1a999c8 commit 6a52d99

1 file changed

Lines changed: 23 additions & 20 deletions

File tree

Packages/CandidateUI/Sources/CandidateUI/VerticalCandidateController.swift

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -390,33 +390,36 @@ public class VerticalCandidateController: CandidateController {
390390
guard let delegate = delegate else {
391391
return
392392
}
393-
let selectedRow = tableView.selectedRow
394-
let labelCount = keyLabels.count
395-
let itemCount = delegate.candidateCountForController(self)
396-
397393
if newValue == UInt.max {
398394
tableView.deselectAll(self)
399395
return
400396
}
401397

402-
if itemCount > labelCount {
403-
var rowToScroll = Int(newValue)
398+
// The cast is safe: at this point, newValue cannot be UInt.max.
399+
let newRowIndex = Int(newValue)
404400

405-
if selectedRow != -1 && itemCount > 0 {
406-
let firstVisibleRow = tableView.row(at: scrollView.documentVisibleRect.origin)
407-
// If it's not single row movement, trigger forward page switching.
408-
if newValue > selectedRow && (Int(newValue) - selectedRow) > 1 {
409-
let lastVisibleRow = firstVisibleRow + labelCount - 1
410-
rowToScroll = min(lastVisibleRow + labelCount, Int(itemCount) - 1)
411-
}
412-
// If it's not single row movement, trigger backward page switching.
413-
if newValue < selectedRow && (selectedRow - Int(newValue)) > 1 {
414-
rowToScroll = max(0, firstVisibleRow - labelCount)
415-
}
416-
}
401+
let labelCount = keyLabels.count
402+
let itemCount = delegate.candidateCountForController(self)
417403

418-
if rowToScroll < Int.max {
419-
tableView.scrollRowToVisible(rowToScroll)
404+
if itemCount > labelCount && labelCount > 0 {
405+
let selectedRow = tableView.selectedRow
406+
let pageIndex = newRowIndex / labelCount
407+
408+
if selectedRow == -1 || abs(newRowIndex - selectedRow) == 1 {
409+
// Simply scroll to the row if never selected or if it's moving by one.
410+
tableView.scrollRowToVisible(newRowIndex)
411+
} else {
412+
if newRowIndex > selectedRow {
413+
// Moving forward: scroll to the last row of the page that contains newRowIndex.
414+
let scrollTo = min((pageIndex + 1) * labelCount, Int(itemCount)) - 1
415+
tableView.scrollRowToVisible(scrollTo)
416+
} else if newRowIndex < selectedRow {
417+
// Moving backward: scroll to the first row of the page that contains newRowIndex.
418+
let scrollTo = pageIndex * labelCount
419+
tableView.scrollRowToVisible(scrollTo)
420+
} else {
421+
// No change in the selected index; do nothing.
422+
}
420423
}
421424
}
422425
tableView.selectRowIndexes(

0 commit comments

Comments
 (0)