NavList::Group - Pagination doesn't work if there's more than one group in the NavList

[strackoverflow]

Steps to reproduce

1. Create a NavList
2. Add 2 or 3 NavList::Group components with some items
3. Add a with_show_more_item slot to the 2nd or 3rd group to enable pagination
4. When the "Show more" link is clicked, the following JS exception should appear in the console:

DOMException: Failed to execute ‘insertBefore’ on ‘Node’: The node before which the new node is to be inserted is not a child of this node.

It looks like this is because each NavList::Group component has a data-target of nav-list.list, so when the <nav-list> component executes this.list.insertBefore, it's apparently only targeting the first group in the NavList.

[jonrohan]

Thanks for filing, we're going to move this to the backlog. When you have time, happy to help you contribute a fix.

[github-actions]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[strackoverflow]

So, I took a look at this bug this week. It looks like the specific issue referencing this.list.insertBefore and the related JS exception are no longer relevant after #1943 was merged, so that's good 🎉

On the other hand, users are still only able to add the "show more items" link to the first group in the NavList, which feels unexpected. Is this a use case that we intended to support?

For context, this is a pattern that GitHub's global navigation uses in the left hand side panel to paginate repos & teams, which are in separate groups. We ended up working around the limitation by extending the PVC classes into the app and overriding some methods, but it might be nice to support this at the component level.

/cc @lesliecdubs

[lesliecdubs]

Thanks for diving in @strackoverflow!

On the other hand, users are still only able to add the "show more items" link to the first group in the NavList, which feels unexpected. Is this a use case that we intended to support?

Good question! I think we likely want someone from Primer Design to weigh in on whether the component was designed to support this use case and, if not, whether we ought to consider it.

I'm going to move this into the backlog for discussion at the next Primer Patterns Working Session (GitHub staff only) so we can get input on this. @strackoverflow if you'd like to attend to be part of the conversation, you can ask for the invite in #primer-design or add the "Design Infrastructure" Google calendar to see the next instance.

[lesliecdubs]

Wow, sorry for the noise ^! Memex did some wild things with the labels on this issue while I was trying to get this added to the design queue.

[mperrotti]

We discussed this during Primer patterns, and we agree that it's ok to have a "show more" link per group, not just one "show more" for the whole NavList.

[lesliecdubs]

Thanks Mike!

Looks like we now have a clear path forward on this. @strackoverflow this might not be a bad issue to pick back up this week if ambiguity remains around the other workstreams you're looking into.