search

jolbol1 / jolly-ui

shadcn/ui compatible react aria components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://jollyui.dev
MIT License
544 stars 13 forks source link

fix: fix focus ring, add ref, add displayName #37

Closed nxsdev closed 2 weeks ago

nxsdev commented 2 weeks ago

The behavior of the original button click was different from shadcn, and the focus ring was displayed when clicked.

Image from Gyazo

After the modification, the focus ring is not displayed on click, but is displayed when the keyboard is in focus (data-focus-visible=true).

Image from Gyazo

Also, since the ref was not received, there was a problem that the ref could not be passed, for example, when replacing a Button called in shadcn's Carousel component with a jolly-ui Button.

vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jolly-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 16, 2024 2:45pm
jolbol1 commented 2 weeks ago

Is it possible to remove the ref logic?

I did previously have all components wrapped in forwardRef like shadcn, but with the news that this is going to be deprecated for React 19, elected to remove it. Going forward you will be able to pass ref as a prop. https://react.dev/blog/2024/04/25/react-19#ref-as-a-prop

This does remind me that I should document this decision somewhere though, a long with documentation on what changes to make when you need it. Happy to here thoughts on this though.

In terms of the styling, I picked this up recently too. Thank for fixing it here, I'm going to go over all the components and ensure I am using the data attribute variant of these selectors.

nxsdev commented 2 weeks ago

Thank you for your comment.

Sorry, I did not know that news. I think it's a great decision to look ahead to the future.

Earlier I wrote Pagination, but it was correctly Carousel. https://github.com/shadcn-ui/ui/blob/main/apps/www/registry/default/ui/carousel.tsx#L205

I would be happy to have documentation, but I thought it would be good to modify some of them, for example this Carousel and others, such as ref and onClick, so that jolly-ui's Button can be used, and add them to jolly-ui.

jolbol1 commented 2 weeks ago

@all-contributors add @nxsdev for design

allcontributors[bot] commented 2 weeks ago

@jolbol1

I've put up a pull request to add @nxsdev! :tada: