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

[HOLD for payment 2022-11-24] [$500] Implement a SplashScreen on web #12234

Closed
roryabraham opened this issue Oct 28, 2022 · 42 comments
Closed

[HOLD for payment 2022-11-24] [$500] Implement a SplashScreen on web #12234

roryabraham opened this issue Oct 28, 2022 · 42 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Oct 28, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open a new incognito window
  2. Open the developer console, network tab
  3. Throttle your network connection to Slow 3G
  4. Go to NewDot web (production or staging)

Expected Result:

For the ~55 seconds while the app bundle is downloading, you should see a splash screen while the site is loading.

Actual Result:

For the ~55 seconds while the app bundle is downloading, you just see a blank white screen.

Platform:

We already have this on iOS and Android, just needed on:

  • Web
  • Mobile Web

Issue reported by: @roryabraham
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666947230935069

View all open jobs on GitHub

@roryabraham roryabraham added External Added to denote the issue can be worked on by a contributor Engineering Daily KSv2 NewFeature Something to build that is a new item. labels Oct 28, 2022
@roryabraham roryabraham self-assigned this Oct 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2022

Triggered auto assignment to @davidcardoza (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2022

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

@melvin-bot melvin-bot bot changed the title Implement a SplashScreen on web and desktop [$250] Implement a SplashScreen on web and desktop Oct 28, 2022
@roryabraham
Copy link
Contributor Author

On narrow screens (i.e: mWeb) – the splash screen should exactly match the native splash screen on iOS and Android

@roryabraham roryabraham changed the title [$250] Implement a SplashScreen on web and desktop [$250] Implement a SplashScreen on web Oct 28, 2022
@vladamx
Copy link
Contributor

vladamx commented Oct 28, 2022

Proposal

The idea is to show the splash as soon as possible and then wait for React to mount the root element with content which will replace the splash screen with app content.

In the alternative solution, the idea is similar but we are controlling the display of the splash by making sure React is done mounting and minTime has passed.

Solution 1:

image

Solution 2:

Controlling the minimum splash time

image

Additionally, Webpack needs to be configured to serve the logo like so:

image

If we don't want this feature on desktop we could detect if we are running in Electron by inspecting window instance

@roryabraham
Copy link
Contributor Author

Awesome, solution 2 looks good to me, though I think if we could use an svg instead of a png that would be ideal 👍🏼

If we don't want this feature on desktop we could detect if we are running in Electron by inspecting window instance

No need, we can have this work the same on web/desktop.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Oct 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2022

📣 @vladamx You have been assigned to this job by @roryabraham!
Please apply to this job in Upwork 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 📖

@vladamx
Copy link
Contributor

vladamx commented Oct 28, 2022

PR 31.10.2022

@roryabraham I think upwork job link is missing here?

@rushatgabhane
Copy link
Member

I think upwork job link is missing here

cc @davidcardoza

@davidcardoza
Copy link
Contributor

Sorry I have been out of the office. Job is posted - https://www.upwork.com/jobs/~0190eba7fee32bdedd

@vladamx
Copy link
Contributor

vladamx commented Nov 1, 2022

@davidcardoza I think you've mistakenly put 20 000$ instead of 250$ in the job description?

@parasharrajat
Copy link
Member

Oh lol, Lucky mistake...

@vladamx
Copy link
Contributor

vladamx commented Nov 1, 2022

PR is up

@OleksiiBieliaievDev

This comment was marked as duplicate.

@melvin-bot melvin-bot bot removed the Overdue label Nov 14, 2022
@roryabraham roryabraham added Weekly KSv2 and removed Daily KSv2 labels Nov 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 16, 2022

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@aimane-chnaif
Copy link
Contributor

Please make sure that the solution works in release build.
#12334 causes regression - #12760 in release mode.

@roryabraham
Copy link
Contributor Author

@aimane-chnaif is correct that the solution for this caused a regression in staging.

@rushatgabhane
Copy link
Member

Thanks for quickly fixing the regression @vladamx

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 17, 2022
@melvin-bot melvin-bot bot changed the title [$500] Implement a SplashScreen on web [HOLD for payment 2022-11-24] [$500] Implement a SplashScreen on web Nov 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 17, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.28-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 2022-11-24. 🎊

@davidcardoza
Copy link
Contributor

I will begin processing payment on the 24th if no regressions come up.

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Nov 23, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 28, 2022

@davidcardoza, @vladamx, @rushatgabhane, @roryabraham Huh... This is 4 days overdue. Who can take care of this?

@roryabraham
Copy link
Contributor Author

Bump for payment @davidcardoza

@melvin-bot melvin-bot bot removed the Overdue label Nov 28, 2022
@davidcardoza
Copy link
Contributor

payment to @vladamx is processed.

@rushatgabhane
Copy link
Member

rushatgabhane commented Nov 29, 2022

@davidcardoza could you please process payment for C+ too

thank you!

@rushatgabhane
Copy link
Member

@roryabraham can you please reopen so that it's on @davidcardoza's list

@davidcardoza
Copy link
Contributor

@rushatgabhane an offer was sent to you via upwork.

@davidcardoza
Copy link
Contributor

@rushatgabhane moving forward please add C+ reviewed in issues and PRs for my team to locate the Contributor+ to pay.

@rushatgabhane
Copy link
Member

@davidcardoza okayy, accepted

@davidcardoza
Copy link
Contributor

Paid, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

7 participants