-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Navigation component: Add back button click handler #25556
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
Navigation component: Add back button click handler #25556
Conversation
|
Back button will be shown if either I wonder which should take priority. Right now if both It doesn't make sense to have both props specified, but IMO, <NavigationMenu
menu="custom-back-click"
title="Custom back button click handler"
onBackButtonClick={ ( e ) => {
e.preventDefault(); // This will prevent the default onClick handler which navigates to the parentMenu
setActiveMenu( 'root' );
} }
backButtonLabel="Go back to root"
/> |
|
Size Change: +31 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
mattwiebe
left a 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.
Tests well, code looks good
vindl
left a 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.
Overall this looks good but needs rebase after latest Storybook reorganization from @Copons.
I also think that onClick should take precedence but not completely override it, since we might want to execute some side effects in it, and then continue with back navigation as usual (e.g. let's say someone wants to track stats for back button clicks in their component or similar). On the other hand, if someone wants a complete override of the behavior, they can optionally use |
9e7dc87 to
74f6977
Compare
|
74f6977 to
7e0475e
Compare
| title="You can't go back from here" | ||
| badge={ backButtonPreventedBadge } | ||
| /> | ||
| </NavigationMenu> |
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.
Ahaha this example is EVIL!
I like it.
Related: #25247
Description
Allow specifying custom back button click handler.
How has this been tested?
yarn storybook:devScreenshots
Types of changes
Enhancement
Checklist: