-
Notifications
You must be signed in to change notification settings - Fork 37
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: [110] add error pages #149
Conversation
WalkthroughThe changes improve error handling in the application by establishing dedicated routes for 403 and 404 error pages. New components, constants, and styles enhance user communication during errors, providing clear messages and navigation options. This update ensures users receive guidance when facing access issues or missing pages, significantly enhancing overall usability. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
frontend/src/scenes/errors/Error.jsx (1)
9-15
: Consider moving button styles to a CSS module.Using a CSS module for styles can improve maintainability and consistency across components.
- const errorButtonStyle = { - borderRadius: '8px', - marginTop: '58px', - fontSize: '13px', - lineHeight: '24px', - padding: '5px 27px' - } + const errorButtonStyle = styles.errorButton;Add the following to
Error.module.scss
:.errorButton { border-radius: 8px; margin-top: 58px; font-size: 13px; line-height: 24px; padding: 5px 27px; }
b824922
to
83a5e4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good . Could you please add a test using vitest
It is under test folder frontend\src\tests
I added test cases |
Is there anything expected from me for this PR? |
Yes please delete jest library and use vitest as I requested in previous comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, mark all conversation resolved when you done
How is it going? |
I got errors with toHaveStyle when I was doing the issue, so I added the setupTest file. I'll see if there's a different way. |
Fixed your comments @uparkalau |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do git pull from develop and it is good to go
IT looks good. Accept new changes from the develop branch and you can merge it. |
18e9b8b
to
66bd998
Compare
I updated branch. Can you check again? |
|
Yes, you can merge it. |
Issues #110
When I looked at the project, I saw this as an entry-level issue and wanted to do it.
I think page not found(404) and no authorization(403) pages should be separate. They mean different things and the url names should be different. That's why I made it as two different pages. I separated the texts in the design.