perf: virtualize bookmark & syllabus lists (#219)#558
Open
eleven-smg wants to merge 1 commit into
Open
Conversation
Convert BookmarkList to the shared VirtualList and MobileSyllabus to a SectionList so course/quiz lists stay smooth with 1000+ items. Add virtualization tests and document the tuning in code comments. Also bump jest-expo to a version that actually exists (54.0.16) so installs work.
|
@eleven-smg Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Kindly resolve conflict |
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
2 participants
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.
What this does
Closes #219.
Converts the remaining long lists from
ScrollViewto virtualized lists so they stay smooth with 1000+ items:VirtualList(the tunedFlatListwrapper) instead of mapping every bookmark inside aScrollView. Bookmark rows are a fixed height, so I passitemHeightto enablegetItemLayoutfor O(1) scroll-to-index.SectionListinstead of nested.map()s inside aScrollView. Collapsed sections pass an emptydataarray so their lesson rows unmount while the section header stays visible. Lesson rows have variable height (wrapping titles + Resume/Bookmarked badges), sogetItemLayoutis intentionally left off here.MobileSearchalready used aFlatList, so it didn't need changes.The windowing settings (
windowSize,maxToRenderPerBatchin the 10-15 range,updateCellsBatchingPeriod,removeClippedSubviews) are documented in code comments in both files.Tests
Added
src/components/mobile/__tests__/list-virtualization.perf.test.tsx, following the same style as the existingcarousel.test.tsx: under Jest the RN list components are host stubs, so the tests assert each list is virtualized and configured correctly rather than counting rendered rows.Notes
npm installwas failing becausejest-expowas pinned to~54.0.19, which was never published (latest 54.x is 54.0.16). I bumped it to~54.0.16so the project installs again.jest@30vs the environmentjest-expo@54pulls in) -carousel.test.tsxfails the same way before my changes. Looks like a pre-existing toolchain issue; happy to open a separate issue or help fix it if useful.