feat(ui): enhance not-found page #1379
Conversation
There was a problem hiding this comment.
I mimicked the layout from the DisabledUser page, for consistency.
|
looks wise - much better! |
| useEffect(() => { | ||
| if (isFullyAuthenticated) { | ||
| return; | ||
| } | ||
|
|
||
| navigate(ROUTE_LOGIN); |
There was a problem hiding this comment.
This works and make sense but was curious as to that vs
useEffect(() => {
if (!isFullyAuthenticated) {
navigate(ROUTE_LOGIN);
}
}, [isFullyAuthenticated, navigate]);
is there a benefit or consideration in having the positive conditional return ?
There was a problem hiding this comment.
This referred to as the "exit early" pattern. There are typically many benefits to which objectively improve code quality. You can read about it here. However, in this case, there is not much reason to pick this over your option than habit and consistency.
There was a problem hiding this comment.
Nice! Thanks for that article, read and bookmarked :)
cmd/ui/src/views/NotFound.tsx
Outdated
| <Grid container spacing={4} justifyContent='center'> | ||
| <Grid item xs={12}> | ||
| <Alert severity='warning'> | ||
| <AlertTitle>404 - Page not found</AlertTitle> | ||
| There is no page associated with this route. Please contact your system administrator for | ||
| assistance. | ||
| </Alert> | ||
| </Grid> | ||
| <Grid item xs={12}> | ||
| <Box mt={2}> | ||
| <Button | ||
| => { | ||
| navigate(ROUTE_EXPLORE); | ||
| }} | ||
| data-testid='page-not-found-go-to-explore' | ||
| size='large' | ||
| type='button' | ||
| className='w-full'> | ||
| Go to Explore | ||
| </Button> | ||
| </Box> | ||
| </Grid> | ||
| </Grid> |
There was a problem hiding this comment.
( What we spoke about yesterday but figured I would add here just in case ) Not a showstopper, but could we maybe consider changing Box and Grid components to just the html tags with TW? We are trying to get some MUI components out of the codebase as we go.
|
Works and looks great! Good idea on mimicking the existing one! just a couple of comments and a curiosity question. |
5eb092b to
7aebe6e
Compare
Description
Update NotFound page to clearly state 404 status and to redirect to login if unauthenticated.
Motivation and Context
Resolves BED-5326
How Has This Been Tested?
Manually tested. Visited bad routes while authenticated and again while unauthenticated.
ex. http://bloodhound.localhost/ui/not-a-route
Screenshots:
Before
After
Types of changes
Checklist: