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

feat: add dedicated error page #586

Merged
merged 5 commits into from Jan 23, 2023
Merged

feat: add dedicated error page #586

merged 5 commits into from Jan 23, 2023

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Jan 3, 2023

Before this commit, in case there was an error, a generic error page had been shown.
After this commit, a dedicated error page, including the navbar, the footer and with a link to create an issue on GitHub, will be displayed.

Whenever you browse the app, this page should never render in the first place, as all error are to be handled by the actual component. However, some errors slip through (e.g. with the date parsing bug in safari #584) and then, the user should be presented with a better view.

The page is really just the bare minimum. The error is unexpected and in most cases, cannot be resolved by the user without support.

In order for this to work properly, the dependency react-router-dom has been upgraded from v6.4.3 to v6.6.1.

How to test

In dev mode (when starting with npm run dev:start), a special route with name error-example is present.
Visit http://localhost:3000/error-example to view this page.

📸 Before

image

📸 After

image

@theborakompanioni theborakompanioni added the enhancement New feature or request label Jan 3, 2023
@theborakompanioni theborakompanioni self-assigned this Jan 3, 2023
@theborakompanioni theborakompanioni marked this pull request as ready for review January 3, 2023 21:43
@theborakompanioni theborakompanioni requested review from editwentyone and removed request for dergigi January 19, 2023 09:10
Copy link
Contributor

@dergigi dergigi 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 to me, tACK ✅

Suggesting some small changes.

src/i18n/locales/en/translation.json Outdated Show resolved Hide resolved
src/i18n/locales/en/translation.json Outdated Show resolved Hide resolved
src/i18n/locales/en/translation.json Outdated Show resolved Hide resolved
src/i18n/locales/en/translation.json Outdated Show resolved Hide resolved
src/components/ErrorPage.tsx Outdated Show resolved Hide resolved
src/components/ErrorPage.tsx Outdated Show resolved Hide resolved
@dergigi
Copy link
Contributor

dergigi commented Jan 23, 2023

Looks like this now:

Screenshot 2023-01-24 at 00-04-30 Jam for JoinMarket

@dergigi dergigi merged commit 42c9f5d into master Jan 23, 2023
@dergigi dergigi deleted the fix/error-page branch January 23, 2023 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants