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

[FEATURE]: Add a Loading Prop to Button Component #264

Open
jamesqquick opened this issue Aug 30, 2024 · 4 comments
Open

[FEATURE]: Add a Loading Prop to Button Component #264

jamesqquick opened this issue Aug 30, 2024 · 4 comments
Assignees
Labels
🚀 Enhancement Suggesting improvements to existing features.

Comments

@jamesqquick
Copy link
Collaborator

Although we are using the Button component consistently, we are manually handling loading states.

Describe the solution you'd like
Add a loading prop to the Button component and have it handle the logic for displaying the loader.

Describe the benefits of your solution
This will ensure consistency across all loading button states.

*idea from @joshmedeski

@joshmedeski joshmedeski added the 🚀 Enhancement Suggesting improvements to existing features. label Aug 30, 2024
@Ibarra11
Copy link

I’m interested in working on this issue. Could you please assign it to me?

@jamesqquick
Copy link
Collaborator Author

@Ibarra11 done!

@Ibarra11
Copy link

Ibarra11 commented Sep 30, 2024

Alright, I'm currently working on it. From what I can tell there are two buttons that use a loading state SubmitButton and SubscribeFormButton. The SubmitButton utilizes a Loading component that shows the spinner and SubscribeFormButton has its own spinner. I would assume that we would want to use the Loading component for the spinner because it has some accessibility stuff with it as well. One issue is that the spinners are different colors for each button, the SubmitButton is Black and the SubscribeFormButton is Teal. I'm not sure how you would go about handling this but just trying things, I added some disabled pseudo state class and within the button just inherit that text.

SubscribeFormButton

  <Button
      className="h  absolute right-1 top-1 border border-red-500 bg-transparent pr-[19px] transition-transform hover:translate-x-0.5 disabled:text-teal-500 md:top-4"
      disabled={pending}
      loading={pending}
      aria-label="Submit"
    >
const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
  (
    {
      className,
      variant,
      size,
      children,
      asChild = false,
      loading = false,
      ...props
    },
    ref
  ) => {
    const Comp = asChild ? Slot : 'button'
    return (
      <Comp
        className={cn(buttonVariants({ variant, size, className }))}
        ref={ref}
        {...props}
      >
        {loading ?
          <span className="text-inherit">
            <Loading />
          </span>
        : children}
      </Comp>
    )
  }
)

@jamesqquick
Copy link
Collaborator Author

Thanks for working on this. Could we update the color of the loader based on the variant of the button? Maybe pass in the color to the loader?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Enhancement Suggesting improvements to existing features.
Projects
None yet
Development

No branches or pull requests

3 participants