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

Type definition for BreadcrumbItem doesn't seem to match implementation #5249

Closed
henningeiben opened this issue Jun 23, 2020 · 5 comments
Closed

Comments

@henningeiben
Copy link

Describe the bug

While the implementation of the linkProps is being describes as "Additional props passed as-is to the underlying link for non-active items" and is declared as PropTypes.object, the types are declaring React.LinkHTMLAttributes<HTMLLinkElement>. This limits the link as being only a HTMLLinkElement.

Expected behavior

If I pass linkAs={Link} I would like to specify linkProps={{to:"/"}} in order to pass to:"/" to the Link compontent of react-router-dom.

Environment

  • React-Bootstrap Version 1.0.1

Additional context

When I change

export interface BreadcrumbItemProps {
  active?: boolean;
  href?: string;
  linkAs?: React.ElementType;
  target?: string;
  title?: React.ReactNode;
  linkProps?: React.LinkHTMLAttributes<HTMLLinkElement>;
}

to

export interface BreadcrumbItemProps {
  active?: boolean;
  href?: string;
  linkAs?: React.ElementType;
  target?: string;
  title?: React.ReactNode;
  linkProps?: any;
}

this seems more like the documentation.

@taion
Copy link
Member

taion commented Jun 23, 2020

Hmm, it looks like we should probably doubly-template this and do something like React.ComponentProps<LinkAs>. Care to send in a PR?

@henningeiben
Copy link
Author

I would love to - but I'm afraid ... at least currently I don't feel as deep into this. I don't really get, where I should make the adjustment you suggested 😊

And ... I just cloned the repo (for starters) and already fail to run the tests ☹

@jquense jquense closed this as completed in 4bd37b7 Jul 8, 2020
@henningeiben
Copy link
Author

Awesome! So ... since this is in the master-branch, this is however not (yet) part of the official build, right? As far as I can tell version 1.1.1 does not include this commit.

@jquense
Copy link
Member

jquense commented Jul 9, 2020

that's right, it will get into the next release (soon)

@henningeiben
Copy link
Author

BTW: the 1.2.0 version did solve this issue 😍

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