[$250] [Xero] Tapping "Other integrations" in accounting page slight flashing / janky animation

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.81-1 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @shawnborton Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1718032595682989

Action Performed:

Have a QBO connection

1. Open app
2. Go to workspace
3. Go to accounting
4. Tap " Other integrations"

   Expected Result:

   There should be smooth animation

   Actual Result:

   There is a slight flashing/janky animation

   Workaround:

   unknown

   Platforms:

   Which of our officially supported platforms is this issue occurring on?

   • [ ] Android: Native
   • [ ] Android: mWeb Chrome
   • [x] iOS: Native
   • [ ] iOS: mWeb Safari
   • [ ] MacOS: Chrome / Safari
   • [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/38435837/11478888-3152-4781-8904-cc48f40cbad2

https://github.com/Expensify/App/assets/38435837/596bb9bc-8801-4bdc-8b56-9a2d69ce75a6

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01894cdf15ac4a5e92
• Upwork Job ID: 1800299476976101068
• Last Price Increase: 2024-06-17
• Automatic offers:
• Pujan92 | Reviewer | 102770646
• truph01 | Contributor | 102770647

Issue Owner

Current Issue Owner: @Pujan92

[melvin-bot[bot]]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[trjExpensify]

CC: @rushatgabhane @mananjadhav @hungvu193 seen this before?

[rushatgabhane]

@trjExpensify haven't seen before but i can reproduce this janky-ness

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01894cdf15ac4a5e92

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)

[trjExpensify]

Let's send it external then!

[truph01]

Proposal

Please re-state the problem that we are trying to solve in this issue.

There is a slight flashing/janky animation of the collapse component

What is the root cause of that problem?

The bug is in react-native-collapsible

There's a condition mismatch between https://github.com/oblador/react-native-collapsible/blob/2bd7f8d50c2f3d876119bc65da8e7681c38563ad/Collapsible.js#L188 and https://github.com/oblador/react-native-collapsible/blob/2bd7f8d50c2f3d876119bc65da8e7681c38563ad/Collapsible.js#L194.

The hasKnownHeight is used to render the parent view with fixed height or not. So when hasKnownHeight is false initially when collapsed becomes false, there will be no style here. At the same time, the measuring doesn't start yet, so the measuring here is false, so the content becomes rendered.

Right after that the measuring becomes true and the content becomes hidden, then after measuring completed, the content becomes visible again.

So the root cause is that although the hasKnownHeight is false (the parent view is not ready to be shown yet), we're still showing the content (because measuring didn't start).

What changes do you think we should make in order to solve the problem?

Make the condition between https://github.com/oblador/react-native-collapsible/blob/2bd7f8d50c2f3d876119bc65da8e7681c38563ad/Collapsible.js#L188 and https://github.com/oblador/react-native-collapsible/blob/2bd7f8d50c2f3d876119bc65da8e7681c38563ad/Collapsible.js#L194 consistent.

If hasKnownHeight is false (the parent view is not ready to be shown yet), the content should also not show.

This condition can be updated to:

if (!hasKnownHeight) {

The !hasKnownHeight condition is inclusive of the existing (measuring) condition because if measuring is true, hasKnownHeight will be false and !hasKnownHeight will also be true. So this will not cause any problem.

To test this locally, please modify the same condition in node_modules/react-native-collapsible/Collapsible.js

What alternative solutions did you explore? (Optional)

Update the logic here https://github.com/oblador/react-native-collapsible/blob/2bd7f8d50c2f3d876119bc65da8e7681c38563ad/Collapsible.js#L189-L192 so if hasKnownHeight is false, the parent view will have invisible style

const style = hasKnownHeight ? {
      overflow: 'hidden',
      height: height,
    } : {
      overflow: 'hidden',
      height: 0,
    };

Or just

const style = {
      overflow: 'hidden',
      height: height,
    };

As when hasKnownHeight is false, the height will also be 0

[TheGithubDev]

Proposal

Please re-state the problem that we are trying to solve in this issue:

The problem is that when a user navigates to "Other integrations" in the accounting section of the workspace, the animation is not smooth. Instead, there is a slight flashing animation.

What is the root cause of that problem?

The root cause of the janky animation could be due to:

1. Inefficient Rendering: Inefficient rendering or re-rendering of components causing the animation to lag.
2. Heavy Components: Heavy components or operations being loaded synchronously, which blocks the main thread and causes animation to stutter.
3. CSS/Style Issues: Improper use of CSS or style properties that interfere with smooth animations.

What changes do you think we should make in order to solve the problem? To address this issue, we need to:

1. Optimize Rendering:
   • Ensure that components are rendered efficiently, possibly using React.memo or PureComponent to avoid unnecessary re-renders.
2. Asynchronous Loading:
   • Load heavy components or data asynchronously to prevent blocking the main thread during animations.
3. CSS/Style Improvements:
   • Review and optimize CSS or style properties used for animations to ensure smooth transitions.

Example of changes:

1. Optimize Rendering:

   • Use React.memo or PureComponent to prevent unnecessary re-renders.

     const OtherIntegrations = React.memo(() => {
     return (
        <View>
            {/* Other integration components */}
        </View>
     );
     });

2. Asynchronous Loading:

   • Load heavy components or data asynchronously.

     useEffect(() => {
     const loadHeavyComponents = async () => {
        await loadData();
     };
     
     loadHeavyComponents();
     }, []);

3. CSS/Style Improvements:

   • Ensure CSS animations are optimized and using properties that can be hardware accelerated, like transform and opacity.

     .integration-animation {
     transition: transform 0.3s ease-in-out, opacity 0.3s ease-in-out;
     }

Alternative solutions ?

1. Performance Profiling: Use performance profiling tools to identify and address specific bottlenecks causing the animation to jank.
2. Virtualization: Implement virtualization for lists or heavy components to load only visible items, reducing the rendering load.
3. Animation Libraries: Utilize animation libraries like Lottie or React Native Reanimated to handle complex animations more efficiently.

Please note that the above explanation outlines only the technical approach I plan to take. If my proposal gets accepted, I will submit the finalized solution, as mentioned.

Thanks!

[lschurr]

@Pujan92 can you take a look at the proposals here?

[Pujan92]

As @truph01's mentioned in their proposal the bug seems to be in react-native-collapsible. A similar issue https://github.com/oblador/react-native-collapsible/issues/473 is raised in the same repo.

I agree with @truph01's RCA where due to those conditions the content gets rendered before the parent View style has been assigned. I like their alternative solution to provide overflow: 'hidden' in each cases for the parent to avoid overflowing the content. I believe hasKnownHeight part can be omitted. For this we need to raise a PR in react-native-collapsible and I think the reviewer will help there in case hasKnownHeight is required.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[lschurr]

@neil-marcellini - can you take a quick look at this one?

[neil-marcellini]

As @truph01's mentioned in their proposal the bug seems to be in react-native-collapsible. A similar issue oblador/react-native-collapsible#473 is raised in the same repo.

I agree with @truph01's RCA where due to those conditions the content gets rendered before the parent View style has been assigned. I like their alternative solution to provide overflow: 'hidden' in each cases for the parent to avoid overflowing the content. I believe hasKnownHeight part can be omitted. For this we need to raise a PR in react-native-collapsible and I think the reviewer will help there in case hasKnownHeight is required.

🎀👀🎀 C+ reviewed

Sounds good, hiring @truph01

[melvin-bot[bot]]

📣 @Pujan92 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @truph01 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[truph01]

@Pujan92 @neil-marcellini With this issue, should I create a pull request (PR) to fix the library directly or create a patch package?

[Pujan92]

I prefer to create PR in the lib but since long the repo isn't been updated, so we can proceed with the patch package option. WDYT @neil-marcellini ?

[rushatgabhane]

@Pujan92 i think it's best that we discuss on opensource channel. We have precedents for this

[Pujan92]

Thanks @rushatgabhane, I will check and raise it on the open-source channel.

[Pujan92]

@truph01 Based on the discussion here, let's raise a PR in the library repo and we try to inform the maintainer to review it.

[lschurr]

Any update on this one @truph01?

[truph01]

@lschurr I created PR in that lib https://github.com/oblador/react-native-collapsible/pull/475

[Pujan92]

Thanks @truph01. On Twitter, they have responded to me and I have shared the PR link. They will check the PR after their vacation. Let's wait for some days.

[Pujan92]

Waiting for a lib PR to be reviewed.

[melvin-bot[bot]]

@Pujan92 @neil-marcellini @lschurr @truph01 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[truph01]

@Pujan92 Do you know when the lib's author can review the PR?

[Pujan92]

I sent a bump message yesterday, we will ask in the slack thread if we don't get a response untill Monday.