Repository navigation
NO-1497: Improve table modal scroll performance - #400
Closed
vivekjyani wants to merge 1 commit into
Closed
vivekjyani wants to merge 1 commit into
vivekjyani wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The table modal grid scrolled sluggishly with large row counts. Profiling showed every row was being materialized on load (no virtualization), and the SwiftUI body re-evaluated on every scroll frame because the sticky header and left row-number column were kept in sync through
@State-driven offsets.What changed
Single file:
Sources/JoyfillUI/View/Fields/TableView/TableModalView.swiftLazyVStackvirtualization by removing.fixedSize(horizontal: false, vertical: true)from the grid stack — it forced the stack to its full ideal height, eagerly building every row. Now only visible rows are built.ViewOffsetKeypreference +@State offset) with UIKitcontentOffsetKVO. A smallTableScrollSynchandle +TableGridScrollReader/TableScrollCapturemirror the grid's scroll offset onto the left row-number column (y) and column header (x) on the render thread — off the SwiftUI update path.leftCornerHeader,columnsHeader, and aTableRowNumbersColumnview; collapsed the duplicated iOS 15/16 branches.Decisions
@State-driven offset. Driving the header/left column from the grid'scontentOffsetin UIKit keeps the sync on the render thread, so steady-state scrolling no longer re-evaluates the modal body each frame.TableScrollSyncis a plain class held in@State(notObservableObject) so mutating the captured scroll views never invalidates the body..scrollDisabled(true)on iOS 16+ (programmaticcontentOffsetstill works); drag-blocking gesture fallback on iOS 15.Media
Public API / docs
No public API change — all new types (
TableScrollSync,TableGridScrollReader,TableScrollCapture,TableRowNumbersColumn) are internal/private. No docs update needed.Tests
Full unit suite passes (2334/2334). Scroll smoothness verified manually on-device. No new unit tests — this is a view-layer change not covered by the unit harness; existing table UI tests exercise the accessibility identifiers, all of which are preserved.
Reviewer notes
Behavior is intended to be identical to before apart from the scroll performance improvement. All accessibility identifiers, selection, filter/sort, pencil→edit-sheet, and
scrollTobehavior are unchanged.