Skip to content

Conversation

@david-szabo97
Copy link
Member

Related: #25247

Description

Allow specifying custom back button click handler.

How has this been tested?

  • Run in terminal: yarn storybook:dev
  • Click Category
  • Click Custom back click handler
  • Click Go back to root

Screenshots

image

Types of changes

Enhancement

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@david-szabo97 david-szabo97 added the [Feature] Navigation Component A navigational waterfall component for hierarchy of items. label Sep 23, 2020
@david-szabo97
Copy link
Member Author

Back button will be shown if either parentMenu or onBackButtonClick prop is specified on NavigationMenu

I wonder which should take priority. Right now if both parentMenu and onClick are specified in the NavigationBackButton then the parentMenu takes priority so the button will act as a back button to the parent menu.

It doesn't make sense to have both props specified, but parentMenu might act as an information. For example, when I'm creating menus it would be great to see what's the parent menu of the menu I'm creating. But I'd like to have a custom back button click handler.

IMO, onClick should take precedence. Or we can utilise the event.preventDefault().

<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"
/>

@github-actions
Copy link

github-actions bot commented Sep 23, 2020

Size Change: +31 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/components/index.js 168 kB +31 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 8.61 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 129 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.6 kB 0 B
build/block-library/editor.css 8.6 kB 0 B
build/block-library/index.js 135 kB 0 B
build/block-library/style-rtl.css 7.65 kB 0 B
build/block-library/style.css 7.64 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.5 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 1.27 kB 0 B
build/data/index.js 8.42 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.25 kB 0 B
build/edit-post/style.css 6.24 kB 0 B
build/edit-site/index.js 20.4 kB 0 B
build/edit-site/style-rtl.css 3.78 kB 0 B
build/edit-site/style.css 3.78 kB 0 B
build/edit-widgets/index.js 18.7 kB 0 B
build/edit-widgets/style-rtl.css 3.03 kB 0 B
build/edit-widgets/style.css 3.03 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.5 kB 0 B
build/editor/style-rtl.css 3.83 kB 0 B
build/editor/style.css 3.82 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.55 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@mattwiebe mattwiebe left a 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

Copy link
Member

@vindl vindl left a 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.

@vindl
Copy link
Member

vindl commented Sep 25, 2020

I wonder which should take priority. Right now if both parentMenu and onClick are specified in the NavigationBackButton then the parentMenu takes priority so the button will act as a back button to the parent menu.
...
IMO, onClick should take precedence. Or we can utilise the event.preventDefault().

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 event.preventDefault() as you've pointed out.

@david-szabo97 david-szabo97 force-pushed the add/navigation-component-back-button-click branch from 9e7dc87 to 74f6977 Compare September 29, 2020 12:38
@david-szabo97
Copy link
Member Author

  • onBackButtonClick will be called with the Event object. This way we can use event.preventDefault() to prevent the default behavior (navigate back to parentMenu)
  • Rebased and added stories.

@david-szabo97 david-szabo97 force-pushed the add/navigation-component-back-button-click branch from 74f6977 to 7e0475e Compare September 30, 2020 08:14
title="You can't go back from here"
badge={ backButtonPreventedBadge }
/>
</NavigationMenu>
Copy link
Contributor

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.

@david-szabo97 david-szabo97 merged commit e83987d into master Oct 8, 2020
@david-szabo97 david-szabo97 deleted the add/navigation-component-back-button-click branch October 8, 2020 07:48
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Navigation Component A navigational waterfall component for hierarchy of items.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants