Skip to content
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

[$250] [New feature] Add loading indicator when ReconnectApp is running #46611

Open
slafortune opened this issue Jul 31, 2024 · 107 comments
Open
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Monthly KSv2 NewFeature Something to build that is a new item. Reviewing Has a PR in review

Comments

@slafortune
Copy link
Contributor

slafortune commented Jul 31, 2024

Original post -

Version Number: v1.4.77-11
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers): NA
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/350373
Issue reported by: @flodnv
Slack conversation: Original proposal here
: https://expensify.slack.com/archives/C01GTK53T8Q/p1699972596342559

Sometimes, the app is loading something for a little while; as an end user, you have no clue what's happening. Then, when loading finishes, sometimes several seconds later, all of a sudden, new data pops up and takes you by surprise, resulting in a very unpleasant UX that almost feels broken.

Coming from this we've landed on showing the thin bar loader at the top of the screen when ReconnectApp is running.

image

@dubielzyk-expensify created a CodePen and it's the animation called Rubber Band or #loading1 in code.

My code isn't the best so I'd urge us to optimize it rather than copy paste 😅

Much discussion took place here

Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021836556152855644897
  • Upwork Job ID: 1836556152855644897
  • Last Price Increase: 2024-09-19
  • Automatic offers:
    • shubham1206agra | Contributor | 104442782
@slafortune slafortune added Daily KSv2 NewFeature Something to build that is a new item. labels Jul 31, 2024
Copy link

melvin-bot bot commented Jul 31, 2024

Triggered auto assignment to @jliexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jul 31, 2024
Copy link

melvin-bot bot commented Jul 31, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Jul 31, 2024

Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify (NewFeature)

@getusha
Copy link
Contributor

getusha commented Jul 31, 2024

@slafortune could you assign me here too? thanks :)

@jliexpensify
Copy link
Contributor

Assigned you @getusha

@dubielzyk-expensify
Copy link
Contributor

Excited about this one. Let me know when there's something to look and test 😄

@flodnv flodnv added the External Added to denote the issue can be worked on by a contributor label Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

@flodnv flodnv added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

Current assignee @getusha is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 1, 2024
@flodnv flodnv changed the title [New feature] Formalize loading UX patterns [New feature] Add loading indicator when ReconnectApp is running Aug 1, 2024
@nkdengineer
Copy link
Contributor

nkdengineer commented Aug 1, 2024

Edited by proposal-police: This proposal was edited at 2024-08-06 15:13:28 UTC.

Proposal

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

Add loading indicator when ReconnectApp is running

What is the root cause of that problem?

This is a new feature

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

The test branch is here: https://github.com/nkdengineer/App/tree/fix/46611

About the animation: animation: 1.5s loading-bar .3s cubic-bezier(0.65, 0, 0.35, 1) infinite;

@keyframes loading-bar {
  0% {
    left: 0;
    width: 0;
  }
  50% {
    left: 0;
    width: 100%;
  }
  100% {
    left: 100%;
    width: 0;
  }
}
  • Follow the animation in codePen, we will use animation style for left and width.

  • infinite can be used as withRepeat, loading-bar can be used as withSequence with three values of left and width

  • Delay .3s with withDelay

  • cubic-bezier(0.65, 0, 0.35, 1) can be used as Easing.bezier(0.65, 0, 0.35, 1)

  1. We can create a ProgressBar component showing a loading indicator as the design above. Here is an example of this. We can refactor the logic and variables in the PR phrase. The implement detail can see in the test branch above

  2. When ReconnectApp is called, we have isLoadingReportData in Onyx to know when it's running and it's complete. So we can use this variable to show the ProgressBar while ReconnectApp API is running.

  • For ReportScreen get isLoadingReportData using useOnyx and show the ProgressBar below the header view if the screen is the small screen
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
{shouldUseNarrowLayout && <ProgressBar shouldShow={isLoadingReportData}/>}

{headerView}

  • For LHN, we will show the ProgressBar below the TopBar component if the screen is small screen
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
{<ProgressBar shouldShow={isLoadingReportData}/>}

<TopBar
breadcrumbLabel={translate('common.inbox')}
activeWorkspaceID={activeWorkspaceID}
/>

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2024-09-05.at.20.16.57.mov
Screen.Recording.2024-09-05.at.20.17.37.mov

@ShridharGoel
Copy link
Contributor

ShridharGoel commented Aug 1, 2024

Proposal

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

Add loading indicator when ReconnectApp is running.

What is the root cause of that problem?

New feature.

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

Add isLoadingApp in TopBar:

const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);

Then, add the animation component at the bottom:

{isLoadingApp && <LoadingAnimation />}

Example code for loading animation:

import React, { useEffect, useRef } from 'react';
import { Animated, View, StyleSheet, Dimensions } from 'react-native';

const { width } = Dimensions.get('window');

const LoadingAnimation= () => {
    const greenAnimation = useRef(new Animated.Value(0)).current;

    useEffect(() => {
        const animate = () => {
            greenAnimation.setValue(0);

            Animated.sequence([
                // Animation from 0 to width
                Animated.timing(greenAnimation, {
                    toValue: width,
                    duration: 2000,
                    useNativeDriver: false,
                }),
                // Animation from width to 0
                Animated.timing(greenAnimation, {
                    toValue: 0,
                    duration: 2000,
                    useNativeDriver: false,
                }),
            ]).start(() => animate());
        };

        animate();
    }, [greenAnimation]);

    const greenWidth = greenAnimation.interpolate({
        inputRange: [0, width],
        outputRange: [width, 0],
    });

    return (
        <View style={styles.container}>
            <Animated.View style={[styles.loadingBar, { width: greenWidth }]} />
        </View>
    );
};

const styles = StyleSheet.create({
    container: {
        left: 0,
        width: '100%',
        height: 4,
        backgroundColor: 'transparent',
    },
    loadingBar: {
        position: 'absolute',
        height: '100%',
        backgroundColor: 'green',
    },
});

export default LoadingAnimation;

We can improve the animation.

Screen.Recording.2024-08-01.at.2.19.53.PM.mov
Screen.Recording.2024-08-01.at.2.27.54.PM.mov

Note that the exact animation can be created during the PR time.

@ShridharGoel
Copy link
Contributor

Proposal

Updated to include videos.

@melvin-bot melvin-bot bot added the Overdue label Aug 5, 2024
@dubielzyk-expensify
Copy link
Contributor

Not overdue

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2024
@getusha
Copy link
Contributor

getusha commented Aug 6, 2024

The demos attached by both @ShridharGoel & @nkdengineer look way off to me...
Could you guys refer to the codepen provided by @dubielzyk-expensify https://codepen.io/dubielzyk/full/ExBVxVG?

@nkdengineer
Copy link
Contributor

@getusha I updated my proposal with the result as the expected design from codepen and also updated the test branch.

@nkdengineer
Copy link
Contributor

I modified my proposal with the expected.

@melvin-bot melvin-bot bot added the Overdue label Aug 8, 2024
@melvin-bot melvin-bot bot added the Weekly KSv2 label Nov 7, 2024
@melvin-bot melvin-bot bot changed the title [$250] [New feature] Add loading indicator when ReconnectApp is running [HOLD for payment 2024-11-14] [$250] [New feature] Add loading indicator when ReconnectApp is running Nov 7, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 7, 2024
Copy link

melvin-bot bot commented Nov 7, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Nov 7, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.58-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-14. 🎊

For reference, here are some details about the assignees on this issue:

  • @getusha requires payment through NewDot Manual Requests
  • @nkdengineer requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented Nov 7, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@getusha] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@srikarparsi
Copy link
Contributor

To leave a quick update, the initial PR caused multiple regressions so a new PR is being worked on.

@flodnv flodnv changed the title [HOLD for payment 2024-11-14] [$250] [New feature] Add loading indicator when ReconnectApp is running [$250] [New feature] Add loading indicator when ReconnectApp is running Nov 11, 2024
@flodnv flodnv added Daily KSv2 and removed Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production labels Nov 11, 2024
@srikarparsi
Copy link
Contributor

@nkdengineer do you have an ETA on when the new PR will be ready?

@melvin-bot melvin-bot bot removed the Overdue label Nov 12, 2024
@nkdengineer
Copy link
Contributor

I'm investigating to avoid the previous regression, will give an update today or tomorrow.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 12, 2024
@nkdengineer
Copy link
Contributor

@getusha @srikarparsi we have a open follow PR here

@muttmuure muttmuure moved this from CRITICAL to HIGH in [#whatsnext] #quality Nov 25, 2024
@melvin-bot melvin-bot bot removed the Weekly KSv2 label Dec 6, 2024
Copy link

melvin-bot bot commented Dec 6, 2024

This issue has not been updated in over 15 days. @jliexpensify, @srikarparsi, @getusha, @dubielzyk-expensify, @nkdengineer eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@melvin-bot melvin-bot bot added the Monthly KSv2 label Dec 6, 2024
@muttmuure
Copy link
Contributor

How close are we with this one? Can we get it out before EoW?

@jliexpensify
Copy link
Contributor

@srikarparsi @getusha @muttmuure - just a heads up that I am OOO from 19th - 29th December. If this needs payment, please feel free to reassign, otherwise I'll review and sort out when I'm back.

@srikarparsi
Copy link
Contributor

How close are we with this one? Can we get it out before EoW?

Hey @muttmuure, we're waiting on deciding if we want the background of the loading indicator to be 1px or 2px tall and we'll make the necessary changes. I think towards the beginning of next week is probably a good estimate to get this merged.

@adhorodyski
Copy link
Contributor

adhorodyski commented Jan 3, 2025

Hey everyone, I just stumbled upon this thread because I wanted to propose the ~same thing for a more native UX.

One question, how does this loading indicator play together with the skeletons on LHN? Ideally, we could get rid of them with this feature introduced. LTM if this should be moved to Slack.

@srikarparsi
Copy link
Contributor

Yeah, slack would be perfect. This was the thread this was originally discussed so that should be a good place :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Monthly KSv2 NewFeature Something to build that is a new item. Reviewing Has a PR in review
Projects
Status: Release 2.5: SuiteWorld (Sept 9th)
Development

No branches or pull requests