Added an Error Page #6856

mananjadhav · 2021-12-21T19:36:28Z

Details

Added a generic error page.

Fixed Issues

$ #6552

Tests

Tested the pages for different dimensions
Tested the component by forcing an error

QA Steps

Force an error (not sure how this will be on staging)
Ensure that the new error page is shown.

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

…on call

mananjadhav · 2021-12-21T19:45:14Z

@roryabraham @parasharrajat While most of the work is done, I need help with the following to close this. Can you please help?

Need to verify the design/colors, etc. if they're correct. Especially the fontsize of the title
I've seen other places we use blue for links but in the error page design, it is black. I've kept it blue for now, let me know if it has to be changed.
Need to validate the Spanish translations
I am not sure how do I refresh the user to the same URL. Do we have any reference code where we have done this?
How will the QA force an error to load this component? Need to update the QA steps accordingly.

parasharrajat · 2021-12-21T20:13:14Z

Need to verify the design/colors, etc. if they're correct. Especially the fontsize of the title
I've seen other places we use blue for links but in the error page design, it is black. I've kept it blue for now, let me know if it has to be changed.

cc: @Expensify/design

I am not sure how do I refresh the user to the same URL. Do we have any reference code where we have done this?

Web => location.reload.
Android => you may have to restart the main activity. Add a small native method which we can call here.

How will the QA force an error to load this component? Need to update the QA steps accordingly.

I think the best would be to mark it as No QA.

src/components/GenericErrorView/ErrorBodyText/index.js

src/components/GenericErrorView/index.js

parasharrajat · 2021-12-21T20:19:09Z

Android => you may have to restart the main activity. Add a small native method which we can call here.

may be utilized react-navigation for this. Reset the navigation state and redirect the user to the home Route.

mananjadhav · 2021-12-21T20:21:24Z

may be utilized react-navigation for this. Reset the navigation state and redirect the user to the home Route.

I guess the requirement is to reload on the same route and not home route. I'll check what are the options.

parasharrajat · 2021-12-21T20:28:29Z

Then you can redirect to the same route just reset the state.

mananjadhav · 2021-12-21T20:32:51Z

Then you can redirect to the same route just reset the state.

I've picked the current route by adding a function. Is that the right direction?

src/components/GenericErrorView/index.js

shawnborton · 2021-12-21T21:58:08Z

Looks pretty good to me, but looping in @megankelso since she worked on the designs for this.

roryabraham · 2021-12-22T14:25:13Z

Minor piece of feedback for @mananjadhav – when you address feedback, feel free to resolve the reviewer's comment threads to make it easier for other reviewers to scan the issue and pick out any threads with active discussion.

mananjadhav · 2021-12-22T15:38:43Z

Noted @roryabraham. I generally used to put a 👍 emoji on completion. Will resolve next time PR onwards.

…rror-page

mananjadhav · 2021-12-22T23:12:15Z

@roryabraham @parasharrajat I've tried finding a react-navigation solution but that doesn't seem to be working. I was wondering if we should even rely on react-navigation. I forced an error on ReportScreen and if I use the navigationRef.current, it throws null error.

Can we use https://www.npmjs.com/package/react-native-restart for the native platforms, https://www.npmjs.com/package/electron-reload for desktop and location.reload for web platforms?

roryabraham · 2021-12-22T23:41:02Z

I agree we shouldn't rely on react-navigation for the reload. Regarding your proposed alternate solution:

https://www.npmjs.com/package/react-native-restart
- I'm not sure – don't have time to research at the moment. Maybe bring it up in Slack to get some more opinions?
https://www.npmjs.com/package/electron-reload
- This seems like it's used for hot-reload for development purposes. Could we just use location.reload on desktop?
location.reload works for web 👍

mananjadhav · 2021-12-23T00:05:07Z

Could we just use location.reload on desktop?

Missed that. Yeah we can use it.

mananjadhav · 2021-12-23T00:47:14Z

I've asked in the slack channel, but still went ahead to test the lib. It seems to work fine. Will change if I get any feedback to not use lib,

…into feat/error-page

mananjadhav · 2022-01-12T20:25:00Z

@roryabraham PR updated. Left with two comments - punctuation utility and button size prop.

…rror-page # Conflicts: # src/styles/styles.js

roryabraham

Other than the one remaining thread I think this looks good.

OSBotify · 2022-01-14T10:03:32Z

✋ 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 · 2022-01-14T20:00:41Z

🚀 Deployed to staging by @roryabraham in version: 1.1.29-6 🚀

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

mvtglobally · 2022-01-15T02:53:13Z

@mananjadhav @roryabraham @shawnborton @parasharrajat
Any suggestion on how can we trigged this error to QA?

roryabraham · 2022-01-17T17:21:34Z

Ah, sorry this PR title should have been prefixed with [No QA]. Unless there's a bad "white screen" bug that's consistently reproducible on staging there's no way to QA this.

...unless... we could purposely introduce such a bug to QA this, then revert that PR 😈

parasharrajat · 2022-01-17T17:26:38Z

I have a suggestion. Just try to run the QA for this on any crash issue. I think there is one for room details.

roryabraham · 2022-01-17T20:18:01Z

I think there is one for room details.

Have a link?

parasharrajat · 2022-01-17T20:26:51Z

@roryabraham @mvtglobally #7262

roryabraham · 2022-01-17T20:32:56Z

Thanks @parasharrajat, that's perfect! @mvtglobally for QA steps:

Web/Desktop/mWeb

Follow the reproduction steps in New Room - "Uh-oh, something went wrong! error" after click on room details #7262
Make sure the UI looks correct.
Verify that it does not prompt you to open the page on web.

iOS/Android

Same as web/desktop/mWeb, except verify that it does suggest opening the app on web, and includes a link.
Click on the link, verify it takes you to the correct NewDot website (staging, if QA'ing on staging, production, if QA'ing on production).

mananjadhav · 2022-01-18T04:04:01Z

Thank you @parasharrajat @roryabraham for helping with this.

mvtglobally · 2022-01-18T12:50:57Z

@roryabraham @mananjadhav
Should sign out button fix the error?
Is this a separate issue or expected since we have a crash?

Upload.from.GitHub.for.iOS.MOV

mvtglobally · 2022-01-18T14:32:15Z

Also On IOS the page doesn't really display as app crashes. Expected? https://user-images.githubusercontent.com/43995119/149956996-295be13f-1109-499b-afba-8bb9c435411b.MOV

mananjadhav · 2022-01-18T16:15:02Z

I saw the video for iOS, it seems ti flashes for a second and then crashes. It shouldn't be related to the ErrorPage, but yeah let @roryabraham and @parasharrajat comment.

parasharrajat · 2022-01-18T16:42:03Z

It seems that the error was thrown out of the Render tree and yeah that won't be caught by the current implementation. Should that be part of this issue?

mvtglobally · 2022-01-18T19:23:43Z

@roryabraham Should we log a new issue or just leave it as is

roryabraham · 2022-01-18T19:29:28Z

@mvtglobally That's a separate issue, the scope of this PR was really just designing and implementing a generic error page. As long as the page displays correctly, I think other issues are out-of-scope. From what I can see in the videos, this looks to me like it's passing QA. I'm going to check it off on the checklist.

OSBotify · 2022-01-18T22:03:13Z

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

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

mananjadhav added 5 commits December 21, 2021 22:35

feat(error-page): First draft of the page with the content

8d800fe

feat(error-page): Additional styling and alignment

a9df026

feat(error-page): Added link styling and design adjustment

bacc606

feat(error-page): Fixed button size for small screen devices

83fc0e2

feat(error-page): Added alignment for mobile devices and fixed functi…

5bf449b

…on call

mananjadhav requested a review from a team as a code owner December 21, 2021 19:36

MelvinBot requested review from parasharrajat and roryabraham and removed request for a team December 21, 2021 19:36

parasharrajat suggested changes Dec 21, 2021

View reviewed changes

feat(error-page): Code cleanup

c123dd1

parasharrajat suggested changes Dec 21, 2021

View reviewed changes

src/components/GenericErrorView/index.js Outdated Show resolved Hide resolved

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

132adef

…rror-page

mananjadhav added 4 commits December 23, 2021 05:51

feat(error-page): Link fixes and removed unused code

719d3c2

feat(error-page): Added reload for web and desktop

68a0a3b

feat(error-page): Added app reload functionality

748719d

feat(error-page): Added comments

a025797

mananjadhav added 2 commits January 13, 2022 01:32

fix: Code cleanup

5101d2e

Merge branch 'feat/error-page' of https://github.com/mananjadhav/App …

9209388

…into feat/error-page

mananjadhav dismissed stale reviews from parasharrajat and shawnborton via 9209388 January 12, 2022 20:03

fix: Renamed func ref

964ee33

mananjadhav requested a review from roryabraham January 12, 2022 20:25

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

ae0f10d

…rror-page # Conflicts: # src/styles/styles.js

roryabraham approved these changes Jan 14, 2022

View reviewed changes

roryabraham mentioned this pull request Jan 14, 2022

[Discussion] No mutually exclusive boolean props #7220

Closed

roryabraham merged commit 991504f into Expensify:main Jan 14, 2022

OSBotify mentioned this pull request Jan 14, 2022

Deploy Checklist: New Expensify 2022-01-14 #7235

Closed

66 tasks

This was referenced Jan 20, 2022

[HOLD for payment 2022-02-09] Generic error page on desktop does not include a link to the website. #7316

Closed

fix: Added web link for desktop error page #7364

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added an Error Page #6856

Added an Error Page #6856

mananjadhav commented Dec 21, 2021 •

edited

Loading

mananjadhav commented Dec 21, 2021

parasharrajat commented Dec 21, 2021

parasharrajat commented Dec 21, 2021

mananjadhav commented Dec 21, 2021 •

edited

Loading

parasharrajat commented Dec 21, 2021

mananjadhav commented Dec 21, 2021

shawnborton commented Dec 21, 2021

roryabraham commented Dec 22, 2021

mananjadhav commented Dec 22, 2021 •

edited

Loading

mananjadhav commented Dec 22, 2021

roryabraham commented Dec 22, 2021

mananjadhav commented Dec 23, 2021

mananjadhav commented Dec 23, 2021

mananjadhav commented Jan 12, 2022

roryabraham left a comment

OSBotify commented Jan 14, 2022

OSBotify commented Jan 14, 2022

mvtglobally commented Jan 15, 2022

roryabraham commented Jan 17, 2022

parasharrajat commented Jan 17, 2022

roryabraham commented Jan 17, 2022

parasharrajat commented Jan 17, 2022

roryabraham commented Jan 17, 2022

mananjadhav commented Jan 18, 2022

mvtglobally commented Jan 18, 2022

mvtglobally commented Jan 18, 2022

mananjadhav commented Jan 18, 2022

parasharrajat commented Jan 18, 2022

mvtglobally commented Jan 18, 2022

roryabraham commented Jan 18, 2022

OSBotify commented Jan 18, 2022

Added an Error Page #6856

Added an Error Page #6856

Conversation

mananjadhav commented Dec 21, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mananjadhav commented Dec 21, 2021

parasharrajat commented Dec 21, 2021

parasharrajat commented Dec 21, 2021

mananjadhav commented Dec 21, 2021 • edited Loading

parasharrajat commented Dec 21, 2021

mananjadhav commented Dec 21, 2021

shawnborton commented Dec 21, 2021

roryabraham commented Dec 22, 2021

mananjadhav commented Dec 22, 2021 • edited Loading

mananjadhav commented Dec 22, 2021

roryabraham commented Dec 22, 2021

mananjadhav commented Dec 23, 2021

mananjadhav commented Dec 23, 2021

mananjadhav commented Jan 12, 2022

roryabraham left a comment

Choose a reason for hiding this comment

OSBotify commented Jan 14, 2022

OSBotify commented Jan 14, 2022

mvtglobally commented Jan 15, 2022

roryabraham commented Jan 17, 2022

parasharrajat commented Jan 17, 2022

roryabraham commented Jan 17, 2022

parasharrajat commented Jan 17, 2022

roryabraham commented Jan 17, 2022

Web/Desktop/mWeb

iOS/Android

mananjadhav commented Jan 18, 2022

mvtglobally commented Jan 18, 2022

mvtglobally commented Jan 18, 2022

mananjadhav commented Jan 18, 2022

parasharrajat commented Jan 18, 2022

mvtglobally commented Jan 18, 2022

roryabraham commented Jan 18, 2022

OSBotify commented Jan 18, 2022

mananjadhav commented Dec 21, 2021 •

edited

Loading

mananjadhav commented Dec 21, 2021 •

edited

Loading

mananjadhav commented Dec 22, 2021 •

edited

Loading