Skip to content

Commit b12a086

Browse files
committed
Implemented logic to retain and release cells. Used the same for focussed cells
1 parent 46db750 commit b12a086

File tree

6 files changed

+88
-5
lines changed

6 files changed

+88
-5
lines changed

Diff for: Proton/Sources/Swift/Table/TableCell.swift

+25
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ public class TableCell {
4949

5050
public var onEditorInitialized: ((TableCell, EditorView) -> Void)?
5151

52+
@MainActor
53+
private var retainCount = 0
54+
/// Returns `true` if the cell has one or more retain invocations.
55+
@MainActor
56+
public var isRetained: Bool {
57+
retainCount > 0
58+
}
59+
5260
/// Additional attributes that can be stored on Cell to identify various aspects like Header, Numbered etc.
5361
public var additionalAttributes: [String: Any] = [:]
5462

@@ -184,6 +192,22 @@ public class TableCell {
184192
contentView?.removeFocus()
185193
}
186194

195+
/// Retains the cell to prevent it from getting recycled when viewport changes and cell is scrolled off-screen
196+
/// Cell with focus is automatically retained and released.
197+
/// - Note: A cell may be retained as many time as needed but needs to have a corresponding release for every retain.
198+
/// Failing to release would mean that the cell will never participate in virtualization and will always be kept alive even when off-screen.
199+
/// - Important: It is responsibility of consumer to ensure that the cells are correctly released after being retained.
200+
@MainActor
201+
public func retain() {
202+
retainCount += 1
203+
}
204+
205+
/// Releases a retained cell. Calling this function on a non-retained cell is a no-op.
206+
@MainActor
207+
public func release() {
208+
retainCount = max(0, retainCount - 1)
209+
}
210+
187211
func hideEditor() {
188212
contentView?.hideEditor()
189213
}
@@ -192,6 +216,7 @@ public class TableCell {
192216
contentView?.showEditor()
193217
}
194218

219+
195220
func performWithoutChangingFirstResponder(_ closure: () -> Void) {
196221
editor?.disableFirstResponder()
197222
closure()

Diff for: Proton/Sources/Swift/Table/TableView.swift

+23-5
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,12 @@ public class TableView: UIView {
412412
removeScrollObserver()
413413
}
414414

415+
var retainedCells = Set<TableCell>()
416+
415417
var cellsInViewport: [TableCell] = [] {
416418
didSet {
419+
reclaimReleasedCells()
420+
417421
guard oldValue != cellsInViewport else { return }
418422

419423
let oldCells = Set(oldValue)
@@ -422,16 +426,19 @@ public class TableView: UIView {
422426
let toReclaim = oldCells.subtracting(newCells)
423427

424428
toReclaim.forEach { [weak self] cell in
425-
// Ignore reclaiming the cell if it has focus
426-
if cell.containsFirstResponder == false {
429+
// Ignore reclaiming the cell if these are retained
430+
// Cell having focus is always retained and released on lost focus
431+
if cell.isRetained == false {
427432
self?.repository.enqueue(cell: cell)
433+
} else {
434+
self?.retainedCells.insert(cell)
428435
}
429436
}
430437

431438
toGenerate.forEach { [weak self] cell in
432-
// Ignore generating the cell if it has focus. The focussed cell is not reclaimed, hence need not be regenerated.
433-
// In absence of this check, there may be cases where the focussed cell gets duplicated
434-
if cell.containsFirstResponder == false {
439+
// Ignore generating the cell if it is already retained. The retained cell is not reclaimed, hence need not be regenerated.
440+
// In absence of this check, there may be cases where the retained cell gets duplicated
441+
if cell.isRetained == false {
435442
self?.repository.dequeue(for: cell)
436443
}
437444
}
@@ -489,6 +496,15 @@ public class TableView: UIView {
489496
.intersects(adjustedViewport) }
490497
}
491498

499+
private func reclaimReleasedCells() {
500+
retainedCells.forEach {
501+
if $0.isRetained == false {
502+
self.repository.enqueue(cell: $0)
503+
retainedCells.remove($0)
504+
}
505+
}
506+
}
507+
492508
func cellBelow(_ cell: TableCell) -> TableCell? {
493509
guard let row = cell.rowSpan.max(),
494510
let column = cell.columnSpan.min() else {
@@ -854,11 +870,13 @@ extension TableView: TableContentViewDelegate {
854870
}
855871

856872
func tableContentView(_ tableContentView: TableContentView, didReceiveFocusAt range: NSRange, in cell: TableCell) {
873+
cell.retain()
857874
resetColumnResizingHandles(selectedCell: cell)
858875
delegate?.tableView(self, didReceiveFocusAt: range, in: cell)
859876
}
860877

861878
func tableContentView(_ tableContentView: TableContentView, didLoseFocusFrom range: NSRange, in cell: TableCell) {
879+
cell.release()
862880
removeSelectionBorders()
863881
delegate?.tableView(self, didLoseFocusFrom: range, in: cell)
864882
}

Diff for: Proton/Tests/Table/TableViewAttachmentSnapshotTests.swift

+40
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,46 @@ class TableViewAttachmentSnapshotTests: SnapshotTestCase {
10941094
assertSnapshot(matching: viewController.view, as: .image, record: recordMode)
10951095
}
10961096

1097+
func FLAKY_testRecyclesRetainedCell() {
1098+
var viewport = CGRect(x: 0, y: 100, width: 350, height: 200)
1099+
delegate.viewport = viewport
1100+
1101+
Utility.drawRect(rect: viewport, color: .red, in: editor)
1102+
1103+
let attachment = AttachmentGenerator.makeTableViewAttachment(id: 1, numRows: 20, numColumns: 5)
1104+
attachment.view.delegate = delegate
1105+
editor.replaceCharacters(in: .zero, with: "Some text in editor")
1106+
editor.insertAttachment(in: editor.textEndRange, attachment: attachment)
1107+
editor.replaceCharacters(in: editor.textEndRange, with: "Text after grid")
1108+
1109+
XCTAssertEqual(attachment.view.containerAttachment, attachment)
1110+
1111+
viewController.render(size: CGSize(width: 400, height: 700))
1112+
assertSnapshot(matching: viewController.view, as: .image, record: recordMode)
1113+
1114+
let cell10 = attachment.view.cellAt(rowIndex: 1, columnIndex: 0)
1115+
cell10?.editor?.setFocus()
1116+
1117+
viewport = CGRect(x: 0, y: 300, width: 350, height: 200)
1118+
delegate.viewport = viewport
1119+
attachment.view.scrollViewDidScroll(editor.scrollView)
1120+
1121+
Utility.drawRect(rect: viewport, color: .red, in: editor)
1122+
viewController.render(size: CGSize(width: 400, height: 700))
1123+
assertSnapshot(matching: viewController.view, as: .image, record: recordMode)
1124+
1125+
let cell40 = attachment.view.cellAt(rowIndex: 4, columnIndex: 0)
1126+
cell40?.editor?.setFocus()
1127+
1128+
viewport = CGRect(x: 0, y: 320, width: 350, height: 200)
1129+
delegate.viewport = viewport
1130+
attachment.view.scrollViewDidScroll(editor.scrollView)
1131+
1132+
Utility.drawRect(rect: viewport, color: .red, in: editor)
1133+
viewController.render(size: CGSize(width: 400, height: 700))
1134+
assertSnapshot(matching: viewController.view, as: .image, record: recordMode)
1135+
}
1136+
10971137
func FLAKY_testPreventsRecyclingNestedEditorFocussedCell() {
10981138
var viewport = CGRect(x: 0, y: 100, width: 350, height: 200)
10991139
delegate.viewport = viewport
Loading
Loading
Loading

0 commit comments

Comments
 (0)