-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Add ability to only show child pages #31471
Conversation
Size Change: +134 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
static $block_id = 0; | ||
$block_id++; | ||
|
||
$children_only = ( isset( $attributes['childrenOnly'] ) && $attributes['childrenOnly'] ); |
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.
Do you mind also adding support for this option in the PageList -> NavigationLinks conversion? This was added in #30390
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.
🤔 Looking at the convert to links, it is only allowed if the parent block is a navigation block - it took me a bit to figure this out because my test case is using the PageList in a widget in a sidebar, I didn't wrap it in a Navigation block.
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.
The way the Site Editor work now, I don't think we can do this. When in the editor and you click convert to links it will not have the context of what page you are in - so it will hard code the links in the template, but we want it to use the parent of the page you are browsing.
I created a short video that hopefully explains (with audio)
gutenberg-pr31471.mp4
Oh gotcha, this is filtering relative to the current page 👍 So taking a step back a bit, my question then is PageList a good fit, or would we like to add this elsewhere? It could make sense to add this feature in the block, but we'd also need to test this nested in the navigation block, and in various editors (widget, navigation, site, post ). cc @tellthemachines @jasmussen @talldan @getdave Would it more sense to extend capabilities in the Query component and loop to support this? I tested this out with the site editor and there are a few missing pieces to make this possible:
See example in:
cc @ntsekouras |
I can't fully figure out why you'd show only subpages inside the navigation block. But I also can't rule out that you might want to do this in exotic situations, so I'm not fundamentally opposed. But I can't think up how to make an intuitive UI for it; what would happen with a page list inside a navigation block that sits in a header template part but is set to show only subpages? It would likely show nothing, right? For that reason it might be best to not show the option when inside the navigation block. I want to also bring the attention briefly to #31879, and an adjacent conversation that spawned from the navigation overlay color property, on whether it might make sense to rewrite the Page List as a client block. I'm not making any value judgemnets on which approach is feasible or best here, I'll leave that to you — merely bring it up in case we're optimizing in either direction. |
I created a couple of issues that I thought would be useful for filtering Page List as well (#29584, #31063). I agree it all seems a bit like the query block. A suggestion I had in the past was to look into using Query to implement Page List.
As in a static block? The array of blocks can change length, so I think it'd always need a dynamic wrapper block, but I guess the items could be static blocks? |
To my understanding: still a dynamic block as the point is to keep it in sync with new or deleted pages on the site. CC @georgeh as he had looked into it for the overlay color. |
TBH I got a bit confused with the output in editor vs front-end in my testing, but I have also not experimented much with Navigation block and more 'complex' scenarios like lot's of parent/children relationships, deeply nested etc... Having said that an alternative for sure could be extra controls in Query and maybe another one could be having a block variation for |
Agree, I see this in the other template editing with dynamic content tii. I'm not sure if we have a way to set context for it or not. Overall, I think people will want to use the child page feature to generate links, likewise for the additional filters that @talldan mentions in #29584 I'm not sure if it fits in Query Block or not, quite similar but my mindset was that I wanted a list of pages for navigation, so Page List was what first came to mind. |
2556477
to
4100757
Compare
4100757
to
1f324e7
Compare
The video below shows some additional controls that will be required — not necessarily in the form you see, but stubbing them out to illustrate. The PageList block can be used in multiple contexts, for example in a Post, on a Page, or in a Widget area. page-list-children-only2.mp4The video above shows an example on a post so the context toggle is disabled by default, when inserted in a Page it could be toggled and disable the Page search control. The goal of the "children only" toggle is to allow specifying a parent page, and then the Page List will only include the children of the page specified. The context control is to show the children of the page being viewed without having to specify the page. This is looking forward to block themes that may want to create a single template for pages that show sub menus for children pages. I welcome thoughts on this approach and suggestions on a UX for setting and explaining. Additionally, keeping in mind filters for categories and tags? |
Looks like it could work. We need a better label than "Show only children". Nothing great comes to mind, perhaps "Limit to showing subpages"? The second half, "Use current page as parent" is where much of the complexity comes in. Would it be at all possible to only surface the "show only child pages" of the current page, and then explore separately whether we need additional controls at all? |
f052a1d
to
d18910b
Compare
@jasmussen @gwwar I've updated the behavior and I think it is close to where we want it. We may want to edit the wording on the toggle and help, open to suggestions. The video shows the Page List under three scenarios
page-list-post-page-template.mp4I want to update the unit tests with these scenarios, but technically it should be ready for review? |
d18910b
to
0960c2f
Compare
1. About the underline in panelIf I remove the 2. About verbiageI updated the text using your verbiage but keeping "child" instead of "sub". I like keeping it because Parent is the name used in the editor, in various functions (example), and in external documentation (one example) |
Cool, to keep things simple let's just keep the panelbody for now and the project will circle back as part of #27331. |
This looks promising @mkaz I did find a few corner cases while manually testing:
CleanShot.2021-09-29.at.10.13.26.mp4
CleanShot.2021-09-29.at.10.15.55.mp4
Should we be able to see the option in a template? I didn't spot it while testing. |
88167c6
to
6606c2f
Compare
Fixed issue with empty block (no child pages) in #35441 |
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.
Thanks for working on this! Testing the changes, there are a couple of unexpected things happening when "Limit to child pages" is set:
-
On top-level pages only children of the current page appear, but on child pages we can see both children and siblings, plus the siblings' whole descendant tree. This is confusing, especially because the description states "When enabled, the block lists only child pages of the current page."
-
Children are rendered as a flat list, even when there are multiple descendant levels. I'd expect hierarchy to be reflected in the list markup, as it is when Page List renders all pages.
Thanks @tellthemachines for reviewing, I'll look at further nesting of pages. With regards to the first behavior, that is what is desired, so maybe needs better explanatory labels. The idea is to enable creating a menu based on the parent-child relationship, so an entire section can be browsed. For example, if you navigate into one of the children, you should be able to navigate to other siblings. This is particularly useful for documentation sites. |
I fixed the nesting, so if there are "grand child" pages then they will show nested with the toggle flipped. I'm open to suggestions on naming the toggle, maybe @jasmussen and "Limit to sub-pages" is more accurate since it's not just children. The description could be: "When enabled, the block will list the sub-pages of the top most parent" 🤷♂️ |
It feels a bit random though, because siblings at any nesting level may or may not be part of a section. E.g. if I have:
it's not clear that "child 1" tree is relevant when we're browsing "child 2"; that depends fully on how your site is structured. We could make "also show sibling pages" an optional toggle when "show only child pages" is enabled. What feels odd is the unexplained discrepancy in behaviour between a top level and a child page. Hierarchies can have many levels, and the point at which it makes sense to show sibling pages will vary from site to site.
This is not quite accurate either. In the example above, if I set a page list to only show child pages in "grandchild 1 1", it'll show only itself and "grandchild 1 2", not any of its ancestors. |
- Adds a showOnlyChildPages attribute to Edit - Update render to use attribute and only pull in children - Do not show on post pages which will not have a parent - Allow showing on wp_template pages Fixes #31063
The toggle was set to show explictly on certain posts types but it is easier to hide on post types so it will show on others such was templates, template parts, or menus.
Moves check for top level and child checks to its own function so it came be a little more clear what it is doing and why. When the only_child_pages flag is set, we need to treat direct child pages to the parent as top level so they show. And treat grand children pages as child pages so they nest.
11611ab
to
ac19e71
Compare
This has now been achieved in #45861 |
Description
This scratches my own itch, I have a set of tutorial pages that has a single parent page, and then children pages off that parent. I want to build a table of contents that just include a listing of all the child pages.
Example in old theme here if curious the way it was built in the old theme is in this template page.
So for a block theme, I want to use the Pages List to do the same, but need to be able to filter that block and only show the children of the current page.
Fixes #31063
Implementation ? This uses
global $post
to get the$parent_id
which may not be ideal, butpost-comments
andlatest-posts
both use the same. Open to suggestions on a better way to fetch parent id.How has this been tested?
Types of changes