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

Nav block Page List fallback displays too many Pages #44360

Closed
getdave opened this issue Sep 22, 2022 · 6 comments · Fixed by #44415
Closed

Nav block Page List fallback displays too many Pages #44360

getdave opened this issue Sep 22, 2022 · 6 comments · Fixed by #44415
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Block] Page List Affects the Page List Block [Type] Bug An existing feature does not function as intended

Comments

@getdave
Copy link
Contributor

getdave commented Sep 22, 2022

By default the Nav block falls back to a Page List. However on a site with a lot of pages this is overwhelming.

Screenshot 2022-09-22 at 10 52 31

The Page List does have an __unstableMaxPages attribute but this doesn't appear to be respected by Page List even though the Nav block index.php creates the block with this attribute.

This is strange as it was added in #36877.

We should make the Page List respect the limit.

@getdave getdave added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block [Block] Page List Affects the Page List Block labels Sep 22, 2022
@draganescu draganescu added the Needs Design Feedback Needs general design feedback. label Sep 22, 2022
@getdave
Copy link
Contributor Author

getdave commented Sep 22, 2022

Noting that the Nav block currently specifies 4 as the fallback

@draganescu
Copy link
Contributor

The problem here is that the '__unstableMaxPages' is used only when rendering the block, but not in the editor.

@carolinan
Copy link
Contributor

carolinan commented Sep 23, 2022

Aside, I have had users asking why this is limited to four in support requests, asking for this number to be increased.
So I think it can be confusing if not all pages show.

@getdave
Copy link
Contributor Author

getdave commented Sep 23, 2022

I've had a think about this overnight. I'm asking myself how many users will actually have this many top level pages? Answer is very few.

Let's optimise for the 80% use case here and avoid limiting the number of items shown. I'll raise a PR to remove the attribute from the Page List and another to remove the functionality from the Nav block.

Does that sound like a good course of action?

@getdave
Copy link
Contributor Author

getdave commented Sep 23, 2022

OK folks I raised a PR that will remove the max limit attribute from the Page list block. Please take a look and let me know if we're happy to accept that if we merge the Navigation block fallback will render unlimited numbers of Pages.

@jasmussen
Copy link
Contributor

I left a comment on the PR, but should've probably left it here. So copying it over:

Hey, thanks for this PR! Just catching up on the context, I would agree that the default hard lock of 100 captures edge-case sites. But also with the main purpose of this PR, to have just that, and no additional limits. The page list block is mainly created for that purpose, to just list all of your blocks. When used in the navigation block and especially with the recent fallback behaviors, can provide an default behavior for most users.

I would argue that if you have a lot of pages, too many for the menu, you're likely better served by pressing "Edit" to convert the menu to a fully customizable one. This is even despite the fact that custom menu editing still isn't the most intuitive, which is to say we have work there. I like to think that #42257 (comment) can play a key factor there.

@jasmussen jasmussen removed the Needs Design Feedback Needs general design feedback. label Sep 26, 2022
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Nov 24, 2022
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 [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants