Allow href to be set on an sl-menu-item

[KonnorRogers]

What issue are you having?

Allow menu-items to act as anchors.

Describe the solution you'd like

If you pass an href to an <sl-menu-item> it should render an anchor tag under the hood like <sl-button>

Describe alternatives you've considered

There's hacky ways like listening for sl-select event and manually firing a navigation event or constructing an anchor tag and clicking it.

Why?

Well, it's actually a pattern on MDN.

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/menuitem_role#examples

Additional notes

I know this opens the door to needing to support a whole slew of other attributes that anchors typically support.

[claviska]

This is a dup of #292. I'm open to revisiting it, but accessibility and misuse (i.e. using <sl-menu> for general navigation) are my biggest concerns.

@KonnorRogers see this comment and let me know your thoughts: #292 (comment)

[claviska]

The use of <sl-button href> Link Buttons may appear to be a convenience, but it's actually a liability that only promotes sloppy practice and adds unneeded complexity. I believe we should aim to eliminate this feature sooner rather than later, potentially in the next major release (3.0?).

It's my earnest hope that Shoelaces continues to uphold its commitment to clean, efficient coding standards in its future development.

This is as controversial as tabs vs. spaces. Buttons will continue to support href for the reasons mentioned here: https://www.abeautifulsite.net/posts/on-buttons-and-links/

You don't have to agree with it, but real world use cases occasionally demand it and I'm in favor of making things easier for users, not harder. Since the component handles everything under the hood for them, I don't see why this would be considered sloppy — but I'm also not interested in debating that tangent in this thread.

[claviska]

Ah, I missed the example you deep-linked to because the browser brought me to the top of the page. There's also this example, which clearly demonstrates menu items as anchors.

https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-navigation/

I'm OK with adding this as long as we continue to clarify that <sl-menu> is for system-style menus, not basic page navigation. I do have some questions though:

• Can a anchors be mixed with non-anchors in a role="menu" parent?
• Do we need to support role="menubar" (or any other menu-like roles) in <sl-menu>, or would those be different components?

[claviska]

FYI — I hid some of the comments above to keep the discussion on track and positive. Please don't take it personally, folks.

@CetinSert this does look like a valid use case with multiple sources that demonstrate it, so it's fair to entertain as a possibility. Again, we're here to enable users and this is something I've seen misused over and over again. Incorrectly wrapping or nesting anchors is quite common, even by experience devs, so I'm betting that supporting it will improve accessibility for many users.

[CetinSert]

FYI — I hid some of the comments above to keep the discussion on track and positive. Please don't take it personally, folks.

@CetinSert this does look like a valid use case with multiple sources that demonstrate it, so it's fair to entertain as a possibility. Again, we're here to enable users and this is something I've seen misused over and over again. Incorrectly wrapping or nesting anchors is quite common, even by experience devs, so I'm betting that supporting it will improve accessibility for many users.

Then why is #1351 (comment) marked as off-topic?
It shows the not incorrect way of using a light DOM <a> with <sl-menu-item>.
That is the best UX shoelace has today and the baseline on which this issue aims to explore convenience improvements.

[claviska]

Then why is #1351 (comment) marked as off-topic?
It shows the not incorrect way of using a light DOM with .

This was explained in my previous comment.

Here's a quick dump of problems many users will likely have with that:

• They won't remember to place the link on the inside
• They won't remember or understand why inert is necessary
• They won't want to use onclick which means now they have to write more JavaScript
• They will need to style the anchor so it spans 100% of the menu items' width and height, meaning custom CSS is required
• They will likely try to wrap menu items with links, which is incorrect (interactive inside an interactive)
• The onclick=this.children[0].click() example isn't reliable if they put a prefix/suffix icon in a slot before the link
• It's unclear if assistive tech will see a child link and associate it with the menu item's parent role (actually, since you've set tabindex=-1, it won't be recognized as a link at all)

I don't know why you're choosing to die on this hill but, once again, Shoelace is about enabling users to do things right with less thought and effort. If we add support for <sl-menu-item href>, you don't have to use it! Your example will still work as-is.

Can we please move on from this discussion now?

[easterday]

I really like shoelace and am migrating from another component library (material design) that has the href on menu-item feature.

The href feature is really useful and the workaround in shoelace to achieve the same result feels really cumbersome (below in case anyone is searching for it), especially considering how easy it is to use shoelace components in general.

Hope the href feature can be added in the future.

<sl-menu-item class="link"><a href="/admin/deliberations">Admin</a></sl-menu-item>

sl-menu-item::part(base) {
height: 60px;
align-items: center;
}

sl-menu-item.link::part(submenu-icon),
sl-menu-item.link::part(checked-icon) {
display: none;
}

sl-menu-item.link a {
display: flex;
height: 60px;
width: 100%;
padding-left: 24px;
align-items: center;
}

sl-menu-item.link a:link,
sl-menu-item.link a:visited,
sl-menu-item.link a:hover,
sl-menu-item.link a:active {
color: inherit;
text-decoration: none;
cursor: pointer;
-webkit-touch-callout: none;
-webkit-user-select: none;
-khtml-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
user-select: none;
}