-
Notifications
You must be signed in to change notification settings - Fork 603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor BalanceCoinRow into FastBalanceCoinRow and FastCoinIcon #3313
Conversation
* react-native-video-cache+2.0.5.patch * Fast icon * symbol * ChainBadge
…t-balance-coin-row
…t-balance-coin-row
src/components/animations/ButtonPressAnimation/ButtonPressAnimation.android.js
Outdated
Show resolved
Hide resolved
src/components/asset-list/RecyclerAssetList2/FastComponents/FastBalanceCoinRow.tsx
Show resolved
Hide resolved
src/components/asset-list/RecyclerAssetList2/FastComponents/FastBalanceCoinRow.tsx
Show resolved
Hide resolved
src/components/asset-list/RecyclerAssetList2/FastComponents/FastBalanceCoinRow.tsx
Show resolved
Hide resolved
src/components/asset-list/RecyclerAssetList2/FastComponents/FastCoinBadge.tsx
Outdated
Show resolved
Hide resolved
src/components/asset-list/RecyclerAssetList2/FastComponents/FastCoinBadge.tsx
Outdated
Show resolved
Hide resolved
src/components/asset-list/RecyclerAssetList2/FastComponents/FastCoinBadge.tsx
Outdated
Show resolved
Hide resolved
….com/rainbow-me/rainbow into @terrysahaidak/fast-balance-coin-row
thanks, @tchayen, should be fixed now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
built android and ios, looking good
/preview |
/preview |
Uh I am sorry Terry. I was looking at one of my branches and then closed browser tab and accidentally made your branch draft instead of mine 😬 |
…t-balance-coin-row
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hide / unhide & pin / unpin logic seems broken. I let Terry know via DM!
/preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! No app breaking behavior. Great performance improvements.
Fixes RNBW-3350
What changed (plus any additional context for devs)
Refactored BalanceCoinRow into its own family of components - Fast components - FastBalanceCoinRow. Fast components are meant to be used in Lists, especially in Recyclable lists.
Also here I made some optimizations to hooks and selectors that build the wallet screen asset list. Now it's called only when it receives new data. But also it's created on the WalletScreen only and we pass data to the next screens so we don't call the same hook 3 times in different places.
Another change - I removed stagging for the assets list since we don't need it anymore. The render of the asset list items is fast enough.
What "FastComponent" means
The "Fast" part means that the component is as fast to render/update as it can be. We are trying to use as less of dynamic and heavy stuff as possible. Because of that, we don't use the Design System for layout except for Text.
So in order to make a Fast Component, you should make sure that:
Before doing any refactoring - measure the mount and update time with
RenderProfiler
.Also, I noticed that refactoring old stuff that is used in many places and "reusable" is not worth it and it's easier to just create a brand new component instead.
We're using RenderProfiler to measure mount and update time. Since it's RecyclerViewList the mount - is when we show a full list of assets, the update is when we scroll since it's passing new props instead of remounting the items.
All the tests were done on Samsung A12 2021 in Release but with the Profiling version of RN (a tiny bit slower).
Before results:
After results:
PoW (screenshots / screen recordings)
Before:
REC_20220518_094024_2.mp4
After:
REC_20220518_091440_3.mp4
Dev checklist for QA: what to test
Final checklist