perf: search transition smoother on slower devices #31354

hurali97 · 2023-11-15T11:36:27Z

Details

When user navigates back from Search Sidebar there’s a noticeable delay and lag in closing the Search Sidebar. It mainly arises from withNavigationFocus HOC as it re-renders the BaseOptionSelector tree including OptionsList . Adding a memo to OptionsList doesn’t help as it’s parent gets re-rendered due to the withNavigationFocus HOC.
Also, we notice that due to isFocused state value in ActionItemView the ReportActionsList re-renders which also adds the overhead to the rendering cycle as it recreates the instances of recordTimeToMeasureItemLayout and loadOlderChats which are passed as props to ReportActionsList .

We can remove the withNavigationFocus HOC and add navigation listeners(focus, blur) in BaseOptionSelector and memoize OptionsList to not re-render when state is set in it’s parent. We can avoid recreating instances of recordTimeToMeasureItemLayout and loadOlderChats by wrapping them in useCallbacks and memoizing ReportActionsList .

Before on 6x CPU throttle:

recording-before.mov

profile-ranked-before

profile-timeline-before

After on 6x CPU throttle:

Screen.Recording.2023-11-16.at.12.43.10.PM.mov

profile-ranked-after

profile-timeline-after

Fixed Issues

$ #33596
PROPOSAL: https://expensify.slack.com/archives/C05LX9D6E07/p1699514971219659

Tests

Manual Test:

Click on Search Icon and Search Sidebar should appear quickly
Verify that no errors appear in the JS console

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

android-recording.mov

Android: mWeb Chrome

android-web-recording.mov

iOS: Native

ios-recording.mov

iOS: mWeb Safari

ios-web-recording.mov

MacOS: Chrome / Safari

web-recording.mov

MacOS: Desktop

desktop-recording.mov

This memoizes relevant functions and values to not re-render LHNOptionsList and ReportActionsList when there's some update in react tree which is not relevant

melvin-bot · 2023-11-16T07:45:40Z

@Gonals Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

situchan · 2023-11-16T07:58:39Z

I can help if requires C+ review

Gonals · 2023-11-20T15:34:31Z

Seems like we have a number of conflicts here 😁

situchan · 2023-11-20T15:37:48Z

I think @hurali97 is OOO this week. Maybe move this PR forward next week.

… perf/search-transition-smoother-on-slower-devices

hurali97 · 2023-11-27T08:36:37Z

@situchan conflicts resolved 👍

Gonals · 2023-11-27T14:50:27Z

@situchan conflicts resolved 👍

I still see some

hurali97 · 2023-11-28T07:12:21Z

@situchan conflicts resolved 👍

I still see some

Yeah it might be that when I fixed the previous, some latest changes were merged outside my working hours. I will resolve those as well.

… perf/search-transition-smoother-on-slower-devices

hurali97 · 2023-11-28T08:58:44Z

@Gonals conflicts resolved !!

hurali97 · 2023-11-29T10:05:04Z

@situchan Are you able to review the PR? 🙂

situchan · 2023-11-29T11:15:44Z

yes reviewing today

… perf/search-transition-smoother-on-slower-devices

hurali97 · 2023-12-01T10:29:18Z

@situchan any updates on the review 🙂

situchan · 2023-12-01T11:13:20Z

reviewing now

src/pages/home/sidebar/SidebarLinks.js

src/pages/SearchPage.js

hurali97 · 2023-12-29T10:29:06Z

Coming from here. Waiting on to fix Reassure Tests.

In the mean time, I will keep resolving conflicts so that we don't have huge conflicts at the end, when Reassure tests are fixed and we are following up again on this PR. Thanks.

… perf/search-transition-smoother-on-slower-devices

hurali97 · 2024-01-08T11:14:29Z

The reassure tests failure is expected as we have moved the following piece of code from being executed in constructor to execute once the transition animations are finished. Which results in a state update but providing a better user experience.

mountiny

Thanks! @Gonals Could you please review this one soon? thanks!

mountiny · 2024-01-09T00:27:12Z

src/pages/home/report/ReportActionsList.js

@@ -497,4 +497,4 @@ ReportActionsList.propTypes = propTypes;
 ReportActionsList.defaultProps = defaultProps;
 ReportActionsList.displayName = 'ReportActionsList';

-export default compose(withWindowDimensions, withPersonalDetails(), withCurrentUserPersonalDetails)(ReportActionsList);
+export default compose(withWindowDimensions, withPersonalDetails(), withCurrentUserPersonalDetails)(memo(ReportActionsList));


How is this change related to the rest of the PR?

This goes hand in hand with the following:

we notice that due to isFocused state value in ActionItemView the ReportActionsList re-renders which also adds the overhead to the rendering cycle as it recreates the instances of recordTimeToMeasureItemLayout and loadOlderChats which are passed as props to ReportActionsList

… perf/search-transition-smoother-on-slower-devices

mountiny

Thanks!

melvin-bot · 2024-01-09T17:16:09Z

@Gonals looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

Gonals · 2024-01-09T17:16:45Z

As discussed in the PR, Performance tests failing here are not an emergency

OSBotify · 2024-01-09T17:20:21Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-01-10T06:20:29Z

🚀 Deployed to staging by https://github.com/Gonals in version: 1.4.24-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

…n-smoother-on-slower-devices" This reverts commit 96a8617, reversing changes made to a144af4.

[CP Staging] Revert "Merge pull request #31354 from hurali97/perf/search-transitio…

[CP Staging] Revert "Merge pull request #31354 from hurali97/perf/search-transitio… (cherry picked from commit 0ffca02)

mountiny · 2024-01-10T17:57:47Z

This was reverted as bunch of regressions have been found not related to the search page

OSBotify · 2024-01-11T08:52:09Z

🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.24-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2024-01-11T09:58:34Z

🚀 Deployed to staging by https://github.com/Gonals in version: 1.4.24-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	cancelled 🔪
🕸 web 🕸	success ✅

OSBotify · 2024-01-15T11:39:54Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.24-8 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

hurali97 added 6 commits November 14, 2023 17:50

perf: add memoization

5de34fb

This memoizes relevant functions and values to not re-render LHNOptionsList and ReportActionsList when there's some update in react tree which is not relevant

perf: add memoization

3ebfc50

perf: add navigation listeners and remove inline functions

bbb216d

refactor: use personalDetails from utils and add Interaction Manager

15147c6

fix: linting

15e62f9

refactor: focus text input

aab633b

hurali97 marked this pull request as ready for review November 16, 2023 07:45

hurali97 requested a review from a team as a code owner November 16, 2023 07:45

melvin-bot bot removed the request for review from a team November 16, 2023 07:45

melvin-bot bot requested a review from Gonals November 16, 2023 07:45

muttmuure mentioned this pull request Nov 24, 2023

[HIGH] Make the Search Sidebar transition relatively smooth on slower devices #31842

Closed

muttmuure requested a review from situchan November 24, 2023 16:37

Merge branch 'main' of https://github.com/hurali97/Expensify-App into…

70e5512

… perf/search-transition-smoother-on-slower-devices

hurali97 added 2 commits November 28, 2023 13:07

fix: skeleton being shown when typing

0e28330

Merge branch 'main' of https://github.com/hurali97/Expensify-App into…

5f66ef4

… perf/search-transition-smoother-on-slower-devices

hurali97 added 2 commits December 1, 2023 13:23

Merge branch 'main' of https://github.com/hurali97/Expensify-App into…

b2be763

… perf/search-transition-smoother-on-slower-devices

Merge branch 'main' of https://github.com/hurali97/Expensify-App into…

a494ec0

… perf/search-transition-smoother-on-slower-devices

situchan reviewed Dec 1, 2023

View reviewed changes

src/pages/home/sidebar/SidebarLinks.js Outdated Show resolved Hide resolved

src/pages/SearchPage.js Outdated Show resolved Hide resolved

ArekChr mentioned this pull request Jan 2, 2024

[NoQA] Fix: Large Arguments Error by Changing File Transmission Method #33831

Merged

54 tasks

Merge branch 'main' of https://github.com/hurali97/Expensify-App into…

ee753e4

… perf/search-transition-smoother-on-slower-devices

mountiny previously approved these changes Jan 9, 2024

View reviewed changes

Merge branch 'main' of https://github.com/hurali97/Expensify-App into…

f8605a3

… perf/search-transition-smoother-on-slower-devices

hurali97 dismissed mountiny’s stale review via f8605a3 January 9, 2024 07:30

mountiny approved these changes Jan 9, 2024

View reviewed changes

Gonals approved these changes Jan 9, 2024

View reviewed changes

Gonals merged commit 96a8617 into Expensify:main Jan 9, 2024
17 of 18 checks passed

melvin-bot bot added the Emergency label Jan 9, 2024

Gonals removed the Emergency label Jan 9, 2024

github-actions bot mentioned this pull request Jan 10, 2024

Deploy Checklist: New Expensify 2024-01-10 #34203

Closed

54 tasks

bernhardoj mentioned this pull request Jan 10, 2024

[$125] Request money - The 'Enter' key fails to select the email/workspace/phone when requesting money. #34239

Closed

6 tasks

dukenv0307 mentioned this pull request Jan 10, 2024

Expense - "To" field appears with delay in confirmation page #34241

Closed

6 tasks

mountiny added a commit that referenced this pull request Jan 10, 2024

Revert "Merge pull request #31354 from hurali97/perf/search-transitio…

10a4134

…n-smoother-on-slower-devices" This reverts commit 96a8617, reversing changes made to a144af4.

flodnv mentioned this pull request Jan 10, 2024

IOU - Category page renders slowly #34252

Closed

6 tasks

mountiny added a commit that referenced this pull request Jan 10, 2024

Merge pull request #34267 from Expensify/vit-revert31354

0ffca02

[CP Staging] Revert "Merge pull request #31354 from hurali97/perf/search-transitio…

OSBotify pushed a commit that referenced this pull request Jan 10, 2024

Merge pull request #34267 from Expensify/vit-revert31354

a722713

[CP Staging] Revert "Merge pull request #31354 from hurali97/perf/search-transitio… (cherry picked from commit 0ffca02)

melvin-bot bot mentioned this pull request Jan 11, 2024

[PAY 23/02] [Performance] Make the Search Sidebar transition relatively smooth on slower devices #33596

Closed

github-actions bot mentioned this pull request Jan 11, 2024

Deploy Checklist: New Expensify 2024-01-11 #34327

Closed

54 tasks

hurali97 mentioned this pull request Jan 12, 2024

perf: smooth search transition #34422

Merged

50 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: search transition smoother on slower devices #31354

perf: search transition smoother on slower devices #31354

hurali97 commented Nov 15, 2023 •

edited

Loading

melvin-bot bot commented Nov 16, 2023

situchan commented Nov 16, 2023

Gonals commented Nov 20, 2023

situchan commented Nov 20, 2023

hurali97 commented Nov 27, 2023

Gonals commented Nov 27, 2023

hurali97 commented Nov 28, 2023

hurali97 commented Nov 28, 2023

hurali97 commented Nov 29, 2023

situchan commented Nov 29, 2023

hurali97 commented Dec 1, 2023

situchan commented Dec 1, 2023

hurali97 commented Dec 29, 2023

hurali97 commented Jan 8, 2024

mountiny left a comment

mountiny Jan 9, 2024

hurali97 Jan 9, 2024

mountiny left a comment

melvin-bot bot commented Jan 9, 2024

Gonals commented Jan 9, 2024

OSBotify commented Jan 9, 2024

OSBotify commented Jan 10, 2024

mountiny commented Jan 10, 2024

OSBotify commented Jan 11, 2024

OSBotify commented Jan 11, 2024

OSBotify commented Jan 15, 2024

perf: search transition smoother on slower devices #31354

perf: search transition smoother on slower devices #31354

Conversation

hurali97 commented Nov 15, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Nov 16, 2023

situchan commented Nov 16, 2023

Gonals commented Nov 20, 2023

situchan commented Nov 20, 2023

hurali97 commented Nov 27, 2023

Gonals commented Nov 27, 2023

hurali97 commented Nov 28, 2023

hurali97 commented Nov 28, 2023

hurali97 commented Nov 29, 2023

situchan commented Nov 29, 2023

hurali97 commented Dec 1, 2023

situchan commented Dec 1, 2023

hurali97 commented Dec 29, 2023

hurali97 commented Jan 8, 2024

mountiny left a comment

Choose a reason for hiding this comment

mountiny Jan 9, 2024

Choose a reason for hiding this comment

hurali97 Jan 9, 2024

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

melvin-bot bot commented Jan 9, 2024

Gonals commented Jan 9, 2024

OSBotify commented Jan 9, 2024

OSBotify commented Jan 10, 2024

mountiny commented Jan 10, 2024

OSBotify commented Jan 11, 2024

OSBotify commented Jan 11, 2024

OSBotify commented Jan 15, 2024

hurali97 commented Nov 15, 2023 •

edited

Loading