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

👌 IMPROVE(carousel): fix missing classname, use data attribute for thumbs #23

Merged
merged 2 commits into from
Mar 29, 2024
Merged

Conversation

ahmedrowaihi
Copy link
Contributor

to allow overwriting styles of active thumb, dynamically with isSlideActive using className

…umbs

to allow overwriting styles of active thumb, dynamically with isSlideActive using className
@BelkacemYerfa
Copy link
Owner

Thanks @ahmedrowaihi for your contribution , but i tested the code for the data-attr modification , and that will not work , since you have forget to add the data-active attr for the button.

As you can see here :

image

the proper fix will be to add the data-active={isSlideActive} to the button so it will now that the state of the active data attr has been changed.

The new modification will result :

image ,

the code for that is :

<Button
  ref={ref}
  size="icon"
  data-active={isSlideActive}
  className={cn(
    "h-1 w-6 rounded-full",
    "data-[active='false']:bg-primary/50 data-[active='true']:bg-primary",
    className
  )}
  onClick={() => onThumbClick(index)}
  {...props}
>
    <span className="sr-only">slide {index + 1} </span>
</Button>

Please consider this change if you want the pr get merged.

@BelkacemYerfa BelkacemYerfa added the enhancement New feature or request label Mar 29, 2024
@ahmedrowaihi
Copy link
Contributor Author

oopss! my bad, I forgot to add it

@ahmedrowaihi
Copy link
Contributor Author

there you go boss

@BelkacemYerfa
Copy link
Owner

BelkacemYerfa commented Mar 29, 2024

Thanks you for your help @ahmedrowaihi.

Welcome to contribution club , as the first contributor 💪. Don't forget to spread the word on Reddit or similar. Enjoy 🎉!

@BelkacemYerfa BelkacemYerfa merged commit a01c0c5 into BelkacemYerfa:master Mar 29, 2024
1 check passed
@ahmedrowaihi ahmedrowaihi deleted the patch-1 branch March 29, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants