Skip to content
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 Screen: Page List block styling is inconsistent with navigation link styling #29101

Closed
Tracked by #29102
talldan opened this issue Feb 18, 2021 · 13 comments · Fixed by #29932
Closed
Tracked by #29102
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Block] Page List Affects the Page List Block CSS Styling Related to editor and front end styles, CSS-specific issues. [Status] In Progress Tracking issues with work in progress

Comments

@talldan
Copy link
Contributor

talldan commented Feb 18, 2021

Description

The page list block and the navigation screen re-design were both merged at around the same time.

The re-design change the way navigation links appear on the navigation screen—they no longer use a flyout menu for submenus, instead using an accordian style expandable/collapsible submenu.

The page list still renders submenus as flyout menus though, it needs to be brought in line with the way navigation links look.

Step-by-step reproduction instructions

  1. Create some pages for your site, and create a hierarchy using the parent page option
  2. Visit the navigation screen and create a menu
  3. Use the add all pages option on the placeholder
  4. View the submenus
  5. Add some navigation links to the block including creating a submenu.
  6. Compare the way navigation link submenus look to page list submenus

Expected behaviour

Page list submenus look like navigation link submenus

Actual behaviour

They look different

Screenshots or screen recording (optional)

Screenshot 2021-02-18 at 1 53 18 pm

@talldan talldan added [Feature] Navigation Screen [Block] Page List Affects the Page List Block CSS Styling Related to editor and front end styles, CSS-specific issues. labels Feb 18, 2021
@jasmussen
Copy link
Contributor

Potentially related: #28575

@talldan
Copy link
Contributor Author

talldan commented Mar 5, 2021

I think @shaunandrews mentioned something somewhere (I can't find it now). Potentially the issue here is that the vertical Navigation Block should show submenus in the accordion style that the navigation editor implements. Page List's styles could then follow suit and things would work as expected.

So it might be a case of updating the navigation block.

@talldan talldan added the [Block] Navigation Affects the Navigation Block label Mar 5, 2021
@jasmussen
Copy link
Contributor

jasmussen commented Mar 5, 2021

I'd suggest that flyouts and accordions could benefit both vertical and horizontal versions of the navigation block. I've seen horizontal menus that pushed content downwards, intentionally, when a submenu was opened, and I could see a table-of-contents like vertical menu where submenus are best shown in flyouts.

That would still make it a block-related thing, to your point.

@jasmussen
Copy link
Contributor

I noticed I wasn't able to insert the "Page List" block at all on this screen, and that there's a horizontal scrollbar:

Screenshot 2021-03-17 at 09 39 33

The search field also appears missing. Was that intentional?

@jasmussen
Copy link
Contributor

I can't reproduce the original issue reported here, @talldan — submenus do not appear as flyouts in the page list at all:

Screenshot 2021-03-17 at 09 43 30

However I did notice that #29869 improves things a fair bit on this page. Specifically, submenus collapse as expected:

Screenshot 2021-03-17 at 09 47 06

@jasmussen
Copy link
Contributor

I also added #28575 to the tracking issue. The above two screenshots illustrate the difference in markup, and making that markup be the same will substantially simplify the CSS.

@jasmussen
Copy link
Contributor

I'm tracking down this issue, and found out that the style.scss file which is responsible for handling basic hover styles for submenu items is not loaded in the separate navigation editor screen. Is this intentional?

I'd be happy to take a look at writing some CSS specific for just the navigation screen to handle this expand/collapsing, perhaps even to an extent on the Page List block. But I want to be sure the missing style.scss file is intentional, before I start duplicating/tweaking the content. A review of #29869 would also be a great help.

@jasmussen
Copy link
Contributor

Looks like #28874 answers my question above: style.scss files should be loaded in the navigation editor, but aren't being at the moment.

@talldan
Copy link
Contributor Author

talldan commented Mar 17, 2021

I noticed I wasn't able to insert the "Page List" block at all on this screen

@jasmussen Yeah, this is intentional. It isn't an allowed block unless the theme has opted in to support blocks in menus (the option detailed in #24503).

I don't think there's a way to support it that's backwards compatible with the way menus work in core, so I don't think it can be included in the screen by default.

More blocks can be enabled by adding the theme opt in add_theme_support( 'block-nav-menus' ); somewhere locally. At the moment the block can also still be added via the navigation block's placeholder (which seems like a bug).

and that there's a horizontal scrollbar.

Hmm, that looks new. Most likely a bug with the quick inserter as this uses the same component as the other editors.

The search field also appears missing. Was that intentional?

I don't think it shows unless there are more than six blocks. That's implicit behavior of the quick inserter. I'm not sure why you have Post Link twice in your screenshot, though.

Looks like #28874 answers my question above: style.scss files should be loaded in the navigation editor, but aren't being at the moment.

Only for FSE themes, but it can still be tested perfectly fine in Twenty Twenty One.

I can't reproduce the original issue reported here

I can still reproduce it:
Screenshot 2021-03-17 at 7 07 44 pm

I think it'd be best to remove the custom nav block (accordion) styles from the navigation editor and make them part of the block as a first step - I made this issue #29747.

I'd be happy to try that at some point

@talldan
Copy link
Contributor Author

talldan commented Mar 17, 2021

Also wondering why your screenshots looks so different? Is that with a particular theme?

@jasmussen
Copy link
Contributor

I don't think there's a way to support it that's backwards compatible with the way menus work in core, so I don't think it can be included in the screen by default.

Thank you for clarifying. Just noting that I was able to insert it if it's the first block I insert. I just click the "Add all pages". That explains the blue links — that and #28575.

Hmm, that looks new. Most likely a bug with the quick inserter as this uses the same component as the other editors.

I can take a look.

I don't think it shows unless there are more than six blocks. That's implicit behavior of the quick inserter. I'm not sure why you have Post Link twice in your screenshot, though.

Makes sense if the Page List has been omitted. I'll be sure to check if that's the cause of the hoz scrollbar.

I'm not sure why you have Post Link twice in your screenshot, though.

I noticed the icon uses an asterisk, which suggests it may be a custom post type. I just so happen to have one such defined for the theme I'm running, like so:

// Create a custom post type.
function myplugin_custom_post_type() {
	register_post_type( 'podcast',
		array(
			'labels' => array(
				'name' => __( 'Podcasts' ),
				'singular_name' => __( 'Podcast' )
			),
			'public' => true,
			'has_archive' => true,
			'rewrite' => array('slug' => 'podcast'),
			'show_in_rest' => true,
			'supports' => [
				'title', 'editor', 'custom-fields', // Custom fields must be supported for meta handling to function.
			],
		)
	);
}
add_action( 'init', 'myplugin_custom_post_type' );

// Assign a post template to our custom post type
function myplugin_register_template() {
	$post_type_object = get_post_type_object( 'podcast' );
	$post_type_object->template = array(
		array( 'core/heading', array( 'level' => 3, 'content' => 'Description', ) ),
		array( 'core/paragraph', array( 'placeholder' => 'Add description', ) ),
		array( 'core/heading', array( 'level' => 3, 'content' => 'Heading', ) ),
		array( 'core/paragraph', array( 'placeholder' => 'Add show notes', ) ),
	);

	// Lock the template from being rearranged or items deleted.
	$post_type_object->template_lock = 'all'; // Can be "all" or "insert", the latter allows moving blocks around.
}
add_action( 'init', 'myplugin_register_template' );

I'm not sure why the label calls it "post" — can you glean from the code above? Also CC: @gwwar as I recall she worked on that recently.

Only for FSE themes, but it can still be tested perfectly fine in Twenty Twenty One.

Yep, I'll take a look.

I think it'd be best to remove the custom nav block (accordion) styles from the navigation editor and make them part of the block as a first step - I made this issue #29747.

That would help us avoid code written just for the screen, makes sense. I'd love to pair with you on that if you can use my help. I'll take a look at the page list background now though.

Also wondering why your screenshots looks so different? Is that with a particular theme?

Most of the time I test running "Empty Theme" from https://github.com/WordPress/theme-experiments, which is a super barebones FSE theme. That makes it easier for m to know whether the theme influences feature styles or not.

@jasmussen
Copy link
Contributor

PR to fix the horizontal scrollbar here: #29930

@gwwar
Copy link
Contributor

gwwar commented Mar 17, 2021

I'm not sure why the label calls it "post"

As requested, I added a new post label for this. For i18n it's best not to build strings using parts of translated strings. See discussion in #29095 (comment)

To see the right labels, we can edit the plugin and add item_link and item_link_description to the taxonomy or post-type definition.

109557321-cbe93300-7a8c-11eb-87d5-e7c169b88581

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 [Block] Page List Affects the Page List Block CSS Styling Related to editor and front end styles, CSS-specific issues. [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants