-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Page List: Remove duplicate code #46620
Conversation
@@ -164,13 +113,7 @@ export default function PageListItemEdit( { context, attributes } ) { | |||
<ItemSubmenuIcon /> | |||
</button> | |||
) } | |||
<ul | |||
className={ classnames( 'submenu-container', { |
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.
I can't see why we need these extra classes so I'm just removing them.
Size Change: -218 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
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.
I tested this quite a bit. I ran into an issue with the submenu styling of nested page lists not appearing correctly in the editor, but I found the issue also appeared on trunk (#46696), so it shouldn't hold up this change.
What?
As suggested in #46587, we should reduce the duplicate code here.
This also removes some extra code which I believe was redundant.
Why?
Maintainability.
How?
Extract two functions to a utils file.
Testing Instructions
I have also removed some code which handled a couple of cases because I believe its covered by other code.
In the editor, select a navigation link which is deeply embedded in a submenu stack.
When you mouse off the link, the menu should remain open
(Note that this does not yet work for Page List items, this is a separate bug - Page List: Keep submenu open when child block is selected. #46621)
Create a page that has a navigation with a Page List block inside it
View the page in the frontend of your site
Resize your browser window to the point where the navigation collapses to a hamburger menu
Open the overlay using the hamburger menu
Check that the Page List items are all visible.
Testing Instructions for Keyboard
As above.