-
Notifications
You must be signed in to change notification settings - Fork 8
Add admin not found page #3870
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
base: next
Are you sure you want to change the base?
Add admin not found page #3870
Conversation
@manuelblum I intend to change MasterMenuRoutes to always redirect to the first match if a route doesn't exist here. This would conflict with your implementation. Should I revert my change? Do we need a not found page? |
@johnnyomair from an UX perspective, i would prefer having a Not Found page, instead of an redirect to a first route match. I also thing we discussed this with UX already. Maybe should rediscuss the desired behaviour one more time with ux at least we have now both solutions implemented. |
@@ -33,18 +34,24 @@ export function useRoutePropsFromMasterMenuData(items: MasterMenuData): RoutePro | |||
|
|||
export interface MasterMenuRoutesProps { | |||
menu: MasterMenuData; | |||
notFoundPage?: ReactNode; |
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.
What do you think about using fallback
instead? This could also be used to display an access denied page.
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.
what do you mean with fallback? Just renaming the prop?
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.
Yes
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.
We already have https://github.com/vivid-planet/comet/blob/next/packages/admin/cms-admin/src/common/header/about/CometDigitalExperienceLogo.tsx, isn't this the same?
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.
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.
@manuelblum could we change the existing CometDigitalExperience component so support both logos? I think it could be confusing if we have two components with a similar name.
Description
Previously, when navigating to a non-existent admin route or a page the user doesn’t have permission to access, the app would render a blank white screen. This PR improves the user experience by introducing a fallback route that displays a “Not Found” page instead.
Additionally, projects can now provide a custom “Not Found” page by setting the
notFoundPage
prop on theMasterMenuRoutes
component.Screenshots/screencasts
Further information