Add virtualization in province contacts page #134
Conversation
|
❌ Deploy Preview for wargabantuwarga failed. 🔨 Explore the source changes: d1e5a9e 🔍 Inspect the deploy log: https://app.netlify.com/sites/wargabantuwarga/deploys/610f9d971c94f60007828f9a |
suliskh
force-pushed
the
fix/provinces-virtualization
branch
from
57339d1
to
482696e
Compare
suliskh
changed the title
|
could it be possible to implement |
components/contact-list.tsx
Outdated
| position: "absolute", | ||
| top: 0, | ||
| left: 0, | ||
| width: "100%", |
There was a problem hiding this comment.
| width: "100%", | |
| height: `${rowVirtualizer.totalSize}px`, | |
| width: "100%", |
As @andriawan mentioned in his comment, have you tried doing this before?
There was a problem hiding this comment.
Yes, I've tried that, Mas, but unfortunately, it does not solve the main problem. Instead, I use totalSize to create an inner container of the list.
My actual concern is that we ended up having two vertical scrolls which I think is a bad scrolling experience, particularly on a mobile viewport. Are you okay with that behaviour, Mas? Would you mind confirming that scrolling experience yourself via the deploy preview, Mas @zainfathoni? :sungkem:
Double.Scroll.mp4
Double.Scroll.Mobile.mp4
There was a problem hiding this comment.
Yes, I am aware of it. In my opinion, before we need to merge this, we need to simplify the Search and Filter UI, because it takes too much space.
I think we can use something like this to squeeze the filtering & sorting UI into a smaller space, what do you think, @resir014?
Upload.from.GitHub.for.iOS.MOV
If we can squeeze the filtering & sorting UI into a smaller space, we can make the height of the page absolute, so no more scrolling on the page itself.
There was a problem hiding this comment.
Yes, agree @zainfathoni.
I'm not sure about the double scroll though. It feels a little too janky. Unless we can simplify the filter to make the virtualised list fit the whole screen we should hold off on merging this.
There was a problem hiding this comment.
@zainfathoni Let's create a separate issue for the simplified filter. I can work on this later tonight.
Co-authored-by: Zain Fathoni <zain.fathoni@gmail.com>
…gabantuwarga.com into fix/provinces-virtualization
|
Added the |
zainfathoni
added
enhancement
|
@resir014 what if we implement the virtualisation like how Netlify provides the double scroll experience? Upload.from.GitHub.for.iOS.MOV |
…to fix/provinces-virtualization
Codecov Report
@@ Coverage Diff @@
## main #134 +/- ##
==========================================
+ Coverage 76.28% 78.81% +2.53%
==========================================
Files 110 110
Lines 1265 1270 +5
Branches 416 416
==========================================
+ Hits 965 1001 +36
+ Misses 294 263 -31
Partials 6 6
Continue to review full report at Codecov.
|
Closes #125
Description
Add virtualizations of the province contacts list page
Use
react-virtuallib to provide virtualization. I chosereact-virtualinstead ofreact-windowbecause it has a smaller bundle size and its headless mechanism that allowing us to have more control of the rendered markup.How has this been tested?
Live test on a real mobile device (Android 11)
20210720_142810.1.mp4
6x CPU slowdown profiling using Chrome DevTools
Overall performance testing using https://web.dev/measure
Current Tasks
Caveat and Discussion
Both
react-windowandreact-virtualrely on a fixed height of the container to make the virtualization possible. In this PR, I set the list container height to be100vh. As you can see from the above video demo, those result in the page having a double vertical scrollbar and unexpected scrolling experience which unfortunately I can't fix. Despite resulting in a better perf, IMO it doesn't worth the UX tradeoff.