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.

Sign up for GitHub

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

Jump to bottom

Fix displaying error messages on unsupported layers #553

Merged
merged 1 commit into from
Jun 19, 2023
Merged

Fix displaying error messages on unsupported layers #553

merged 1 commit into from
Jun 19, 2023

Conversation

csillag
Copy link
Contributor

@csillag csillag commented Jun 19, 2023

Before:

image

After:

image

@github-actions
Copy link

github-actions bot commented Jun 19, 2023 •
edited
Loading

Deployed to Cloudflare Pages

Latest commit: 1d441c17f3195e2fa79d344f58de9012a06f4f05
Status:✅ Deploy successful!
Preview URL: https://9dd10c80.oasis-explorer.pages.dev

@csillag csillag force-pushed the csillag/fix-layer-error-display branch from 92209bf to 3f59f2c Compare June 19, 2023 10:32
@csillag csillag requested a review from buberdds June 19, 2023 10:33
@buberdds
Copy link
Contributor

when we render layout with unsupported layer, do we need to hide network/ParaTime picker?

@csillag
Copy link
Contributor Author

csillag commented Jun 19, 2023 •
edited
Loading

when we render layout with unsupported layer, do we need to hide network/ParaTime picker?

In it's current form, yes we should hide it, because otherwise it might give us further errors.
However, showing it could be the easiest way back to a valid URL.... but in order to be able to show it, we need to teach it to handle launching with an invalid (or disabled) layer. I don't want to add code changes to it now, since there is another pending PR.

So for now I will just hide it. User can still click on the Oasis logo and start overf.

@csillag csillag force-pushed the csillag/fix-layer-error-display branch from 3f59f2c to 0b8b15a Compare June 19, 2023 11:49
@csillag csillag marked this pull request as draft June 19, 2023 11:49
@csillag csillag marked this pull request as ready for review June 19, 2023 11:50
@csillag
Copy link
Contributor Author

csillag commented Jun 19, 2023

Updated the code accordingly.

@csillag csillag force-pushed the csillag/fix-layer-error-display branch from 0b8b15a to 8b0e5e6 Compare June 19, 2023 11:53
@csillag
Improve handling of routing errors on invalid layers
1d441c1 
 - Don't throw more errors while already trying to display error
 - Use the most specific error message possible
 - Teach useScopeParam() to also return info about validity
 - Hide network selector and freshness info on invalid scopes
@csillag csillag force-pushed the csillag/fix-layer-error-display branch from 8b0e5e6 to 1d441c1 Compare June 19, 2023 13:01
@csillag csillag enabled auto-merge June 19, 2023 13:25
@csillag csillag merged commit 61319b5 into master Jun 19, 2023
@csillag csillag deleted the csillag/fix-layer-error-display branch June 19, 2023 13:31
lukaw3d
lukaw3d reviewed Jun 20, 2023
View reviewed changes
src/app/hooks/useScopeParam.ts
scope.valid = false
if (!error)
throw new Error(
'You must either specify both network and layer or none of them. You can not have one but not the other.',
Copy link
Member

Choose a reason for hiding this comment

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

Why are we encoding this here? Isn't it a consequence of routes, not scope param, so add a 404 route instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukaw3d lukaw3d lukaw3d left review comments

@buberdds buberdds buberdds approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@csillag @buberdds @lukaw3d