siemens / ix

Siemens Industrial Experience is a design system for designers and developers, to consistently create the perfect digital experience for industrial software products.
https://ix.siemens.io/
MIT License
197 stars 65 forks source link

[IxMenuCategory] Allow surrounding nested `IxMenuItem` with `<a>` tags #686

Closed robert-wettstaedt closed 1 year ago

robert-wettstaedt commented 1 year ago

Suggestion / feature request

I was playing around with the new IxMenuCategory because we have a use case for nested menu items.

The way I have used the regular IxMenu before is by surrounding each IxMenuItem with an <a> tag because they are just links to pages in our apps, like this:

<IxMenu>
  <a href="/">
    <IxMenuItem>
      Home
    </IxMenuItem>
  </a>
</IxMenu>

When doing this with the IxMenuCategory, the component will not recognize the IxMenuItems because they are not direct children. This means when expanding the UI element, the expanded region will not be big enough to show all of the children of IxMenuCategory. This line in the IxMenuCategory source explicitly causes this behavior.

I know I could just use an onClick event handler on the IxMenuItems and manually navigate to the requested page. But I would argue that this is not good UX nor is it accessible.

So I suggest to support surrounding IxMenuItems with <a> tags (or other tags for that matter). If you do not agree, what is your recommended way of defining links in the menu?

Example: https://codesandbox.io/s/allow-surrounding-nested-ixmenuitem-with-a-tags-qty7xn

danielleroux commented 1 year ago

Should already work like you described it. Just tested the your provided example and it works with a href.

With #689 we provided a fix to remove the default link styling.

robert-wettstaedt commented 1 year ago

@danielleroux I am not sure why it the provided example would work for you. This is how it looks for me:

Screenshot 2023-08-15 at 08 32 09

Notice how the Category with <a> only shows one child element even though two elements are defined in the code.

danielleroux commented 1 year ago

If you check the development console the item is there, but the animation is calculated based on the wrong item height. This is fixed in #689 (see description of PR)

robert-wettstaedt commented 1 year ago

Yes this is exactly what I was describing in my initial request.