Skip to content

Conversation

@SainathPoojary
Copy link
Contributor

@SainathPoojary SainathPoojary commented Dec 16, 2024

Part of: #67813

What?

Update Navigation Block to Implement ToolsPanel and ToolsPanelItem Components for Display Panel; Adjust Styles for Migration Compatibility.

Screenshots

Before After
image image

@SainathPoojary SainathPoojary marked this pull request as ready for review December 23, 2024 07:10
@github-actions
Copy link

github-actions bot commented Dec 23, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: SainathPoojary <sainathpoojary@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano
Copy link
Contributor

t-hamano commented May 31, 2025

Thanks for the PR!

This UI is quite different from other blocks' UI, so it definitely needs some custom styling 😅

It's mostly good, but here are some suggestions for improving the code quality.

We can apply a class name directly to the Notice component:

{ submenuAccessibilityNotice && (
	<Notice
		className="wp-block-navigation__submenu-accessibility-notice"
		spokenMessage={ null }
		status="warning"
		isDismissible={ false }
	>
		{ submenuAccessibilityNotice }
	</Notice>
) }

The classname with .components- prefix should be avoided as much as possible. We may be able to avoid this by using a stack-based approach, like this:

{ overlayMenuPreview && (
	<VStack
		id={ overlayMenuPreviewId }
		spacing={ 4 }
		style={ {
			gridColumn: 'span 2',
		} }
	>
		<OverlayMenuPreview
			setAttributes={ setAttributes }
			hasIcon={ hasIcon }
			icon={ icon }
			hidden={ ! overlayMenuPreview }
		/>
	</VStack>
) }

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@t-hamano t-hamano merged commit c1ba20f into WordPress:trunk Jun 3, 2025
60 checks passed
@github-actions github-actions bot added this to the Gutenberg 21.0 milestone Jun 3, 2025
chriszarate pushed a commit to chriszarate/gutenberg that referenced this pull request Jul 1, 2025
* Navigation: Refactor display panel to use ToolsPanel

* Navigation: Add ToolsPanelItem in overlay-menu-preview

* Navigation: Improve Layout and Styling for Overlay Menu and Submenus

* Navigation: Refactor overlay menu preview controls

Co-authored-by: SainathPoojary <sainathpoojary@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor "Display" panel of Navigation block to use ToolsPanel instead of PanelBody

3 participants