shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
75.84k stars 4.76k forks source link

Disabled Button state not transmitted to Link - asChild #1894

Closed MagicKitty closed 9 months ago

MagicKitty commented 1 year ago

Hello there, I have tried to implement this example in which I try to disable the button. Seems not working. The variant link added to the button isn't working either:

<Button asChild disabled={!isValid}>
  <Link to={'/final'}>
    Groot
  </Link>
</Button>

Is there a way to disable a Link?

Have a nice day! Great work! Nico

abheektripathy commented 1 year ago

@MagicKitty having a similar issue, did you find a fix?

MrLightful commented 1 year ago

Same here

Josevictor2 commented 1 year ago

try,
`

MagicKitty commented 1 year ago

As I understand the problem: When you declare a button with asChild, what happens is the Button is never rendered in the DOM. So basically no tailwindcss classes can be passed from Button to Link with group. I cannot really figure out a good solution. The only thing that can be OKish is to style Link as a button:

const disabled = false;
<Link
  to={'/'}
  className={cn(
    buttonVariants({ variant: 'outline' }),
    disabled && 'pointer-events-none opacity-50',
  )}
>
  Click here
</Link>
jeromemeichelbeck commented 10 months ago

Only using shadcn for 20 min and already disapointed :/

titusdecali commented 10 months ago

Only using shadcn for 20 min and already disapointed :/

I'm starting to feel the same way. Will switch to TailwindUI Catalyst when they have a Vue version available

shadcn commented 9 months ago

This issue has been automatically closed because it received no activity for a while. If you think it was closed by accident, please leave a comment. Thank you.

nikitaprokopov commented 7 months ago

When you are using asChild, the Button component simply passes props to the child component, in our case, to the Link component.

I am using @tanstack/router, so when the disabled prop is passed to the Link component, @tanstack/router sets the aria-disabled attribute.

Shadcn creates buttonVariants for you (this is the code from Shadcn WITHOUT SOLUTION):"

import { cva } from "class-variance-authority";

export const buttonVariants = cva(
  "tw-inline-flex tw-items-center tw-justify-center tw-whitespace-nowrap tw-rounded-md tw-text-sm tw-font-medium tw-ring-offset-background tw-transition-colors focus-visible:tw-outline-none focus-visible:tw-ring-2 focus-visible:tw-ring-ring focus-visible:tw-ring-offset-2 disabled:tw-pointer-events-none disabled:tw-opacity-50",
  {
    variants: {
      variant: {
        outline: "tw-border tw-border-input tw-bg-background hover:tw-bg-accent hover:tw-text-accent-foreground",
        destructive: "tw-bg-destructive tw-text-destructive-foreground hover:tw-bg-destructive/90",
        secondary: "tw-bg-secondary tw-text-secondary-foreground hover:tw-bg-secondary/80",
        default: "tw-bg-primary tw-text-primary-foreground hover:tw-bg-primary/90",
        link: "tw-text-primary tw-underline-offset-4 hover:tw-underline",
        ghost: "hover:tw-bg-accent hover:tw-text-accent-foreground",
      },
      size: {
        lg: "tw-h-11 tw-rounded-md tw-px-8",
        default: "tw-h-10 tw-px-4 tw-py-2",
        sm: "tw-h-9 tw-rounded-md tw-px-3",
        icon: "tw-h-10 tw-w-10",
      },
    },
    defaultVariants: {
      variant: "default",
      size: "default",
    },
  },
);

I added these classes to the end of the first line of cva, and it helped

cva(
"...aria-disabled:tw-opacity-50 aria-disabled:tw-pointer-events-none",
{
  variants: ...

And this will work:

import { Button } from "@/libs/shadcn/ui/button";
import { Link } from "@tanstack/react-router";

<Button variant="ghost" disabled={true} asChild>
  <Link to="/interviews/$interviewId" params={{ interviewId }}>
    Some link
  </Link>
</Button>;
colinleefish commented 7 months ago

Hello there, I have tried to implement this example in which I try to disable the button. Seems not working. The variant link added to the button isn't working either:

<Button asChild disabled={!isValid}>
  <Link to={'/final'}>
    Groot
  </Link>
</Button>

Is there a way to disable a Link?

Have a nice day! Great work! Nico

My solution for this case is to set asChild to be the ! of whatever the disabled value is.

For example, in your case

<Button asChild={isValid} disabled={!isValid}>
  <Link to={'/final'}>
    Groot
  </Link>
</Button>
zakandrewking commented 6 months ago

I settled on this approach which is nice enough:

  if (disabled) {
    return (
      <Button variant="link" className={className} disabled>
        {children}
        <ExternalLinkIcon size={"0.8em"} className="ml-1" />
      </Button>
    );
  }
  return (
    <Button variant="link" asChild className={className}>
      <Link href={href} target="_blank">
        {children}
        <ExternalLinkIcon size={"0.8em"} className="ml-1" />
      </Link>
    </Button>
  );

Respects the idea that a disabled link isn't really a link at all!

sntlln93 commented 6 months ago

When you are using asChild, the Button component simply passes props to the child component, in our case, to the Link component.

I am using @tanstack/router, so when the disabled prop is passed to the Link component, @tanstack/router sets the aria-disabled attribute.

Shadcn creates buttonVariants for you (this is the code from Shadcn WITHOUT SOLUTION):"

import { cva } from "class-variance-authority";

export const buttonVariants = cva(
  "tw-inline-flex tw-items-center tw-justify-center tw-whitespace-nowrap tw-rounded-md tw-text-sm tw-font-medium tw-ring-offset-background tw-transition-colors focus-visible:tw-outline-none focus-visible:tw-ring-2 focus-visible:tw-ring-ring focus-visible:tw-ring-offset-2 disabled:tw-pointer-events-none disabled:tw-opacity-50",
  {
    variants: {
      variant: {
        outline: "tw-border tw-border-input tw-bg-background hover:tw-bg-accent hover:tw-text-accent-foreground",
        destructive: "tw-bg-destructive tw-text-destructive-foreground hover:tw-bg-destructive/90",
        secondary: "tw-bg-secondary tw-text-secondary-foreground hover:tw-bg-secondary/80",
        default: "tw-bg-primary tw-text-primary-foreground hover:tw-bg-primary/90",
        link: "tw-text-primary tw-underline-offset-4 hover:tw-underline",
        ghost: "hover:tw-bg-accent hover:tw-text-accent-foreground",
      },
      size: {
        lg: "tw-h-11 tw-rounded-md tw-px-8",
        default: "tw-h-10 tw-px-4 tw-py-2",
        sm: "tw-h-9 tw-rounded-md tw-px-3",
        icon: "tw-h-10 tw-w-10",
      },
    },
    defaultVariants: {
      variant: "default",
      size: "default",
    },
  },
);

I added these classes to the end of the first line of cva, and it helped

cva(
"...aria-disabled:tw-opacity-50 aria-disabled:tw-pointer-events-none",
{
  variants: ...

And this will work:

import { Button } from "@/libs/shadcn/ui/button";
import { Link } from "@tanstack/react-router";

<Button variant="ghost" disabled={true} asChild>
  <Link to="/interviews/$interviewId" params={{ interviewId }}>
    Some link
  </Link>
</Button>;

This works perfectly fine. Just remember that if your Link component doesn't set the aria-disabled attribute you will have to do it yourself. In my case i was using Inertia so my PaginationLink (a button under the hood) got change to this

//From this
<PaginationLink
    ...
    disabled={previousLink.url === null}
    asChild>
        <Link
            ...
        >
        </Link>
</PaginationLink>

//To this
<PaginationLink
        ...
        asChild
>
    <Link aria-disabled={previousLink.url === null}
    ...
    >
        ...
    </Link>
</PaginationLink>

Prop disabled on the button doesn't do anything.

Other thing to keep in mind is to change "tw-" for the tailwind prefix your using.

soulchild commented 4 months ago

I came up with this, which basically always renders a (disabled) button when the disabled attribute is set:

- const Comp = asChild ? Slot : 'button';
+ const Comp = asChild && !disabled ? Slot : 'button';
wizzymore commented 1 month ago

If you want to have disabled state you need a button as the anchor tag can't be disabled. Simply do:

<Button asChild disabled={!isValid}>
-  <Link to={'/final'}>
+  <Link as="button" to={'/final'}>
    Groot
  </Link>
</Button>
WellingtonVell commented 1 month ago
import { default as NextLink } from 'next/link';

export interface LinkProps
    extends React.AnchorHTMLAttributes<HTMLAnchorElement>,
        VariantProps<typeof buttonVariants> {
    href: string;
    disabled?: boolean;
}

const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(
    ({ className, variant, size, disabled = false, href, ...props }, ref) => {
        if (!href) {
            throw new Error('The href prop is required for the Link component.');
        }

        return (
            <NextLink
                href={href}
                className={cn(
                    buttonVariants({ variant, size, className }),
                    disabled && 'pointer-events-none opacity-50',
                )}
                ref={ref}
                {...(disabled ? { 'aria-disabled': true } : {})}
                {...props}
            />
        );
    },
);
Link.displayName = 'Link';

my solution taking some suggestions from the comments above

sshmaxime commented 1 month ago

This issue needs to be re-opened please 🙏 @shadcn

irakli2206 commented 4 weeks ago

Still problematic

wizzymore commented 4 weeks ago

Anchor tags don't have a disabled state, use buttons, there is no problem with the Link component itself.

sshmaxime commented 3 weeks ago

@wizzymore Have you tried what we are talking about ? All we are saying is that as of now, if you disable the button and have asChild it doesn't disable it.

wizzymore commented 3 weeks ago

Well if you use asChild and the child is an anchor tag and not a button IT WILL NOT WORK. asChild will just pass the props down to the child component and render that. An anchor tag DOES NOT have a disabled state. If your Link component supports changing what it is you can do something like:

<Button disabled={...} asChild>
    <Link as="button">
        Link
    </Link>
</Button>
wizzymore commented 3 weeks ago

For people that are still confused on what is happening right here. All the stylings for the disabled state of the button is based on the "disabled" attribute put on the html element.

When you do something like <Button asChild><a>...</a></button> what it does behind the scenes is this:

<a class="button classes">...</a>

If you use a "Link" component or something else inside there it would look like this:

// What you write
<Button asChild disabled={true}>
    <Link>...</Link>
</Button>

// What is rendered
<Link class="button classes" disabled={true}>...</Link>

If YOUR component that is put as a child of the BUTTON, DOES NOT take a disabled prop and DOES NOT handle it correctly, you will never get the "disabled" attribute on your html element.

Also, "disabled" is not a valid html attribute for anchor tags ( <a /> ).

What you need to do is this:

<Button asChild disabled={true}>
    <LinkAsAButton>...</LinkAsAButton>
</Button>

This will render the following html and everything will work:

<button class="button classes" disabled="disabled">...</button>

The problem with this is that you can't open in new tab as it is now a button.

What i usually do is the following:

const disabled = true;

return (
    <Link as={disabled ? 'button' : 'a'} className={buttonVariants({variant: ...})} disabled={disabled} href={...}>Text goes here</Link>
)

This way, when it is enabled, it works correctly in the browser as an anchor tag, when it is not, it is a button that will be disabled and does nothing.

A clearer approach is this, in the end is the same thing:

const disabled = true;

return disabled ?
( 
    <Button disabled>Text goes here</Button>
) :
(
    <Button asChild>
        <Link href={...}>Text goes here</Link>
    </Button>
)