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

Add target attribute to menu items #185

Closed
DarthGigi opened this issue Nov 15, 2023 · 7 comments
Closed

Add target attribute to menu items #185

DarthGigi opened this issue Nov 15, 2023 · 7 comments

Comments

@DarthGigi
Copy link

Describe the feature in detail (code, mocks, or screenshots encouraged)

TypeScript gives the following error if you try to add a target attribute to a DropdownMenu.Item component if it's a link (adding the href attribute):
Type '{ target: string; href: string; }' is not assignable to type '$$Props'. Object literal may only specify known properties, and '"target"' does not exist in type '$$Props'. ts(2322)

An a element should have support for the target attribute

What type of pull request would this be?

Enhancement

Provide relevant links or additional information.

No response

@leonardsimonse
Copy link

I can submit a PR for this, but I guess https://github.com/huntabyte/bits-ui would be the right place for it, right @huntabyte?

@huntabyte
Copy link
Owner

Hey @leonardsimonse, it would be in bits-ui, however, I'm currently in the process of a big internal refactor that I should be merging into main soonish that would likely cause some annoying conflicts if done before that PR.

During this refactor I did notice that we're typing the Menu items incorrectly when converting to links right now. They should be similar to how we handle the Button, where if an href is passed, it gets HTMLAnchorAttributes, otherwise HTMLDivAttributes. I'll transfer this issue over there as a reminder!

@huntabyte huntabyte transferred this issue from huntabyte/shadcn-svelte Nov 16, 2023
@leonardsimonse
Copy link

Getting the types in line with an anchor element would be the proper solution indeed @huntabyte. Let's wait for your merge.

@huntabyte
Copy link
Owner

Okay, the big PR has been merged so we can revisit this! I think ideally we do something similar to how we're handling it with the Button: https://github.com/huntabyte/bits-ui/blob/main/src/lib/bits/button/types.ts

Except the dropdown menu items aren't buttons, rather <div /> elements! If someone wants to tackle this, let me know and I'll assign it to ya, otherwise I'll get to it when I can!

@huntabyte huntabyte changed the title Add target attribute to DropdownMenu.Item Add target attribute to menu items Dec 4, 2023
@leonardsimonse
Copy link

I'd love to help out here. If you assign it me, I'll look at it in a couple of hours.

@leonardsimonse
Copy link

@huntabyte
Copy link
Owner

Wow I guess I did! My apologies 😅 lots of changes in that one! Thanks for taking a look anyways!

Closed by #184

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants