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

Add splash screen related logs #6476

Merged
merged 4 commits into from
Dec 2, 2021

Conversation

kidroca
Copy link
Contributor

@kidroca kidroca commented Nov 25, 2021

Details

Fixed Issues

$ #6358

Tests

Verify there are no regressions caused by the refactor

App launch logged out

  1. Start in a logged out state
  2. Login
  3. You should see the splash screen briefly
  4. After the app is loaded you should see the app content

App launch logged in

  1. Start the app logged in
  2. You should see the splash screen briefly
  3. After the app is loaded you should see the app content

If you monitor app logs you should be able to see this message logged about 30 sec after start

[info] [BootSplash] Splash screen status - {"status":"hidden"}

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

image

Desktop

image

iOS

image

Android

image

@kidroca kidroca marked this pull request as ready for review November 25, 2021 13:26
@kidroca kidroca requested a review from a team as a code owner November 25, 2021 13:26
@MelvinBot MelvinBot removed the request for review from a team November 25, 2021 13:26
Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple minor comments + suggestions. Looks good overall though

src/Expensify.js Outdated Show resolved Hide resolved
src/Expensify.js Outdated Show resolved Hide resolved
BootSplash.getVisibilityStatus()
.then((status) => {
Log.info('[BootSplash] Splash screen status', false, {status});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: Extra line

@kidroca
Copy link
Contributor Author

kidroca commented Nov 30, 2021

Ready for another review

Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor comments

src/Expensify.js Outdated Show resolved Hide resolved
src/Expensify.js Outdated Show resolved Hide resolved
@kidroca
Copy link
Contributor Author

kidroca commented Dec 1, 2021

Ready for another review

Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks good

@NikkiWines NikkiWines merged commit f92580d into Expensify:main Dec 2, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Dec 2, 2021

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

@kidroca kidroca deleted the kidroca/add-splash-related-logs branch December 3, 2021 18:12
@OSBotify
Copy link
Contributor

OSBotify commented Dec 7, 2021

🚀 Deployed to staging by @NikkiWines in version: 1.1.17-8 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Dec 8, 2021

🚀 Deployed to production by @roryabraham in version: 1.1.18-3 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants