Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Error page improvements. #1259

Merged
merged 14 commits into from
Aug 26, 2021
Merged

Error page improvements. #1259

merged 14 commits into from
Aug 26, 2021

Conversation

alongoni
Copy link
Contributor

@alongoni alongoni commented Aug 13, 2021

Summary

Work in progress
Closes #1255

Improvements to the layout and styles of the Error page.

  • Add CowSwap logo on header
  • Add cow meme
  • Add labels on GH query.
  • Mobile fixes.
  • Delete forced error. IMPORTANT

image

To Test

Open the page "Cow Game" to throw an error so we can see the app crashing.

@alongoni alongoni added Enhancement New feature or request Protofire Handled by Protofire development team labels Aug 13, 2021
@alongoni alongoni self-assigned this Aug 13, 2021
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@@ -70,6 +87,9 @@ export default class ErrorBoundary extends React.Component<unknown, ErrorBoundar
const encodedBody = encodeURIComponent(issueBody(error))
return (
<FallbackWrapper>
<HeaderWrapper>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anxolin As you anticipated, I have some problems with the header links. Maybe because the header is inside the ErrorBoundary.
Can we try to fix that or maybe we can show only the CowSwap logo to go back to the home.

Copy link
Contributor

@anxolin anxolin Aug 19, 2021

Choose a reason for hiding this comment

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

I would simplify as u suggest. An alternative header, with just a Logo suffices.

The less logic we add there, the less chances we break it. So I would add only HTML Markup, not logics that can fail

@alongoni alongoni marked this pull request as ready for review August 19, 2021 20:48
@anxolin anxolin self-requested a review August 19, 2021 21:10
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

HAha, nice meme :)

Looks good to me. I like the header with just the logo. 👌

uberniting, when you reduce the window, there's some points where the title size feels way smaller proportionally to the other text. It looks okeish

💥 IMPORTANT: Don't forget to remove that throw Error before merging.

@elena-zh
Copy link

Hey @alongoni , great changes!
However, some minor issues from my side:

  1. When I open the GitHub link, I see double URL + no labels in the 'Labels' section
    double url, no labels
  2. May be we could add a right padding in the Error log section?
    add padding
  3. It seems to me, that center-aligned page header will look a bit better that lef-aligned on reduces screens. WDYT?
    image
    new row

I mean, something like this on screens with less than 1140 px width
image

  1. May be we could remove the cow and an exclamation sign from the beginning of the message? I think, it is excessive one as we have another cow in the error area. Moreover, it is hardly visible on iPhone device in the light mode.. WDYT?
    image

Thanks =)

@elena-zh
Copy link

Hey @alongoni ,
cool changes!
However, this is how the github url looks like (a loooong horrible link):
https://github.com/gnosis/cowswap/issues/new?assignees=&labels=%F0%9F%90%9E%20Bug,%F0%9F%94%A5%20Critical&body=%23%23%20URL%0A%20%20%0Ahttps%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2F%23%2Fplay%0A%0A%0A%23%23%20Error%0A%0A%60%60%60%0AError%3A%20This%20cow%20is%20dropping%20some%20serious%20milk%20%F0%9F%A5%9B!!%0A%60%60%60%0A%0A%23%23%20Stacktrace%0A%0A%60%60%60%0AError%3A%20This%20cow%20is%20dropping%20some%20serious%20milk%20%F0%9F%A5%9B!!%0A%20%20%20%20at%20VI%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2Fmain.1e1b0156.chunk.js%3A1%3A584822)%0A%20%20%20%20at%20ro%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2473428)%0A%20%20%20%20at%20Js%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2525886)%0A%20%20%20%20at%20Su%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2513060)%0A%20%20%20%20at%20Eu%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2512988)%0A%20%20%20%20at%20Tu%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2512851)%0A%20%20%20%20at%20mu%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2509817)%0A%20%20%20%20at%20https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2459196%0A%20%20%20%20at%20t.unstable_runWithPriority%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2536615)%0A%20%20%20%20at%20zi%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2458973)%0A%20%20%20%20at%20Gi%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2459141)%0A%20%20%20%20at%20Ji%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2459076)%0A%20%20%20%20at%20je%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2530425)%0A%20%20%20%20at%20Qt%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A2437700)%0A%20%20%20%20at%20HTMLDivElement.r%20(https%3A%2F%2Fpr1259--gpswapui.review.gnosisdev.com%2Fstatic%2Fjs%2F4.b49540c3.chunk.js%3A2%3A4219541)%0A%60%60%60%0A%0A%23%23%20Device%20data%0A%0A%60%60%60json%0A%7B%0A%20%20%22ua%22%3A%20%22Mozilla%2F5.0%20(Windows%20NT%2010.0%3B%20Win64%3B%20x64)%20AppleWebKit%2F537.36%20(KHTML%2C%20like%20Gecko)%20Chrome%2F92.0.4515.131%20Safari%2F537.36%22%2C%0A%20%20%22browser%22%3A%20%7B%0A%20%20%20%20%22name%22%3A%20%22Chrome%22%2C%0A%20%20%20%20%22version%22%3A%20%2292.0.4515.131%22%2C%0A%20%20%20%20%22major%22%3A%20%2292%22%0A%20%20%7D%2C%0A%20%20%22engine%22%3A%20%7B%0A%20%20%20%20%22name%22%3A%20%22Blink%22%2C%0A%20%20%20%20%22version%22%3A%20%2292.0.4515.131%22%0A%20%20%7D%2C%0A%20%20%22os%22%3A%20%7B%0A%20%20%20%20%22name%22%3A%20%22Windows%22%2C%0A%20%20%20%20%22version%22%3A%20%2210%22%0A%20%20%7D%2C%0A%20%20%22device%22%3A%20%7B%7D%2C%0A%20%20%22cpu%22%3A%20%7B%0A%20%20%20%20%22architecture%22%3A%20%22amd64%22%0A%20%20%7D%0A%7D%0A%60%60%60%0A%0A&title=Crash%20report%3A%20%60Error%3A%20This%20cow%20is%20dropping%20some%20serious%20milk%20%F0%9F%A5%9B!!%60

Also, still there is no padding in the error log section from the right. And it was removed from the left: as a result, section value is not fully visible in an iPhone device (Android looks good):
image

Could you please take a look at it?

@henrypalacios
Copy link
Contributor

henrypalacios commented Aug 23, 2021

@alongoni and @elena-zh, the URL is long because a predefined body is being added to the issue through query string.
Selection_318

@alongoni
Copy link
Contributor Author

alongoni commented Aug 23, 2021

Also, still there is no padding in the error log section from the right. And it was removed from the left: as a result, section value is not fully visible in an iPhone device (Android looks good):
image

Could you please take a look at it?

hey @elena-zh. Can you mention your iOS and browser version? It seems like is a CSS problem on iOS with the white-space: pre property. Thanks!
https://developer.mozilla.org/en-US/docs/Web/CSS/white-space
https://caniuse.com/mdn-css_properties_white-space_pre-wrap

@elena-zh
Copy link

elena-zh commented Aug 24, 2021

@alongoni , I tested on iPhone 12 mini with 14.6 iOS in Chrome browser. In Safari the issue is reproducible either.

<CodeBlockWrapper>
<code>
<TYPE.main fontSize={10} color={'text1'}>
{error.stack}
<StyledParagraph>{error.stack}</StyledParagraph>
</TYPE.main>
Copy link
Contributor Author

@alongoni alongoni Aug 24, 2021

Choose a reason for hiding this comment

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

After a review along with @matextrem, we came to the conclusion that it is not a CSS problem.
It is related how the error is interpreted and rendered on iOS (Safari and Chrome). The complete message is not stored directly in the error, as it happens in other platforms and devices.

For example on Safari we get:

GI@https://pr1259--gpswapui.review.gnosisdev.com/static/js/main.1856fc78.chunk.js:1:585424 ro@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2473681 Js@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2526140 Su@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2513314 Eu@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2513242 Tu@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2513105 mu@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2510071 mu@[native code] https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2459449 https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2536868 Gi@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2459395 Ji@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2459330 fu@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2507440 Oo@https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:2479007 Oo@[native code] https://pr1259--gpswapui.review.gnosisdev.com/static/js/4.262e7757.chunk.js:2:281441 promiseReactionJob@[native code]

image

Instead of:

Error: This cow is dropping some serious milk 🥛!!
    at CowGamePage (http://localhost:3000/static/js/main.chunk.js:43233:11)
    at renderWithHooks (http://localhost:3000/static/js/vendors~main.chunk.js:236075:22)
    at mountIndeterminateComponent (http://localhost:3000/static/js/vendors~main.chunk.js:238837:17)
    at beginWork (http://localhost:3000/static/js/vendors~main.chunk.js:240036:20)
    at HTMLUnknownElement.callCallback (http://localhost:3000/static/js/vendors~main.chunk.js:225025:18)
    at Object.invokeGuardedCallbackDev (http://localhost:3000/static/js/vendors~main.chunk.js:225074:20)
    at invokeGuardedCallback (http://localhost:3000/static/js/vendors~main.chunk.js:225134:35)
    at beginWork$1 (http://localhost:3000/static/js/vendors~main.chunk.js:244876:11)
    at performUnitOfWork (http://localhost:3000/static/js/vendors~main.chunk.js:243712:16)
    at workLoopSync (http://localhost:3000/static/js/vendors~main.chunk.js:243649:9)
    at renderRootSync (http://localhost:3000/static/js/vendors~main.chunk.js:243615:11)
    at performSyncWorkOnRoot (http://localhost:3000/static/js/vendors~main.chunk.js:243232:22)
    at http://localhost:3000/static/js/vendors~main.chunk.js:232485:30
    at unstable_runWithPriority (http://localhost:3000/static/js/vendors~main.chunk.js:307490:16)
    at runWithPriority$1 (http://localhost:3000/static/js/vendors~main.chunk.js:232431:14)
    at flushSyncCallbackQueueImpl (http://localhost:3000/static/js/vendors~main.chunk.js:232480:13)
    at flushSyncCallbackQueue (http://localhost:3000/static/js/vendors~main.chunk.js:232468:7)
    at discreteUpdates$1 (http://localhost:3000/static/js/vendors~main.chunk.js:243360:13)
    at discreteUpdates (http://localhost:3000/static/js/vendors~main.chunk.js:224835:16)
    at dispatchDiscreteEvent (http://localhost:3000/static/js/vendors~main.chunk.js:227017:7)

@alongoni alongoni added the Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds label Aug 26, 2021
@mergify mergify bot merged commit ee2dbea into develop Aug 26, 2021
@alongoni alongoni deleted the issue-1255-styles-hard-error branch August 26, 2021 15:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds Enhancement New feature or request Protofire Handled by Protofire development team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nicer styles of the hard error page
5 participants