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: Try unifying submenu arrow positioning. #37003
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,7 +196,17 @@ const PageItems = memo( function PageItems( { | |
} ) } | ||
> | ||
{ hasChildren && context.openSubmenusOnClick ? ( | ||
<ItemSubmenuToggle title={ page.title?.rendered } /> | ||
<> | ||
<button | ||
className="wp-block-navigation-item__content wp-block-navigation-submenu__toggle" | ||
aria-expanded="false" | ||
> | ||
{ page.title?.rendered } | ||
</button> | ||
<span className="wp-block-page-list__submenu-icon wp-block-navigation__submenu-icon"> | ||
<ItemSubmenuIcon /> | ||
</span> | ||
</> | ||
) : ( | ||
<a | ||
className={ classnames( | ||
|
@@ -213,7 +223,14 @@ const PageItems = memo( function PageItems( { | |
{ hasChildren && ( | ||
<> | ||
{ ! context.openSubmenusOnClick && | ||
context.showSubmenuIcon && <ItemSubmenuToggle /> } | ||
context.showSubmenuIcon && ( | ||
<button | ||
className="wp-block-navigation-item__content wp-block-navigation-submenu__toggle wp-block-page-list__submenu-icon wp-block-navigation__submenu-icon" | ||
aria-expanded="false" | ||
> | ||
<ItemSubmenuIcon /> | ||
</button> | ||
) } | ||
<ul | ||
className={ classnames( 'submenu-container', { | ||
'wp-block-navigation__submenu-container': isNavigationChild, | ||
|
@@ -232,17 +249,3 @@ const PageItems = memo( function PageItems( { | |
); | ||
} ); | ||
} ); | ||
|
||
function ItemSubmenuToggle( { title } ) { | ||
return ( | ||
<button | ||
className="wp-block-navigation-item__content wp-block-navigation-submenu__toggle" | ||
aria-expanded="false" | ||
> | ||
{ title } | ||
<span className="wp-block-page-list__submenu-icon wp-block-navigation__submenu-icon"> | ||
<ItemSubmenuIcon /> | ||
</span> | ||
</button> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change also needs to be done for the page list front end, here. As it stands there's still a diff in the front end between open on hover and click with page list submenus: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! I wanted to note that I missed this because it appears I can't currently a navigation menu with only a page list to open submenus on click in the interface. As of #36826, we currently hide those options unless the menu It seems like we should change that to simply just show those options if the menu I was able to check the box in the code editor though, so I'll take a stab at addressing the markup issue now. |
||
); | ||
} |
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.
Well.