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

[Feat]: sp-menu component to provide support for virtualizing sp-menu-item #3213

Open
1 task done
monkeyjabs opened this issue May 18, 2023 · 4 comments
Open
1 task done
Labels
Component: Menu DSRR Can be visited as part of the research into DOM Scope Request Resolution. enhancement New feature or request

Comments

@monkeyjabs
Copy link
Contributor

monkeyjabs commented May 18, 2023

Code of conduct

  • I agree to follow this project's code of conduct.

Impacted component(s)

sp-menu, sp-menu-item

Description of the requested feature

For a large menu list using the sp-menu we would like to be able to virtualized the sp-menu-item for performance gain. While this does partially work right out the box, there is a problem with tab navigation. Once the focus is set into the menu item, using the arrow key to navigate can only navigate to a subset of the list.

Here is an example:
https://studio.webcomponents.dev/edit/oPXMxECYa2Hl67I0mEG4/src/index.ts?branch=swc-menu-virtualizer%40x6l1txiIKneNlFAZY2ShSrq382h2&p=stories

Mockups or screenshots

No response

Implementation notes or ideas

No response

@monkeyjabs monkeyjabs added enhancement New feature or request triage An issue needing triage labels May 18, 2023
@Westbrook
Copy link
Contributor

Some off the cuff thought:

  • have you tried this with the virtualize directive rather than a virtualiser element?
  • Likely this runs afoul of the connected/disconnected logic in <sp-menu-item> as it recycles DOM instead of removing it. There’s not a lot of “state” in a Menu Item, but would you expect to directly leverage value here as a possible reactive hook to trigger updates, instead?
  • An <sp-grid> or <sp-table> element would accept an array of items directly, a similar API is proposed by <sp-combobox>, how would you compare that approach to composing a virtualizer element as a child of the container?
  • Your demo show 100 items in the list. Have you done any tracing at which point you’d see a virtual list as being preferable to a static list?
  • A virtualiser doesn’t inherently need to know about its entire list, but a menu would, particularly when converting of an accessibility API. Whether or not you render them all to the page, will you know the entirely of you Menu Items in order to fulfil requirements in this are?
  • In either Table or Grid, I’d have to confirm, we take a render item as the insides of an item and then wrap that content with an item to ensure the accessibility contract, would you see that as practical for you in this case? Or would you prefer to take on accessibility responsibilities as well?

Excited to dig further into this convo!

@monkeyjabs
Copy link
Contributor Author

The main reason that we're exploring this approach is from a performance analysis from Google Lighthouse, one of the area that gave us low score.
PixelSnap 2023-05-18 at 13 56 18@2x

I have not explored the virtualize directive yet, that was my next thing to explore. As for <sp-grid> and <sp-table> element I'm not sure if it it will our design.

PixelSnap 2023-05-18 at 14 13 56

One of the main reasons to use SWC is to get the accessibility features right out of the box, so we definitely want to avoid having to handle the accessibilities ourselves.

@monkeyjabs
Copy link
Contributor Author

virtualize directive yield with the same result as the virtualizer element:
https://studio.webcomponents.dev/edit/collection/P69Fy1cWB7KlLVvStVRK/5X38x8tuk5qh22qcPGri/src/index.ts?p=stories

@najikahalsema najikahalsema added Component: Menu and removed triage An issue needing triage labels Oct 5, 2023
@Westbrook Westbrook added the DSRR Can be visited as part of the research into DOM Scope Request Resolution. label Jan 3, 2024
@najikahalsema
Copy link
Collaborator

Blocked by #4101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Menu DSRR Can be visited as part of the research into DOM Scope Request Resolution. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants