Closed GODrums closed 1 month ago
great catch
there's no need for any
there, we can change to string
Hey,
why not make the entire thing way more compatible by extending the interface by React.HTMLAttributes<HTMLDivElement>
interface MagicContainerProps extends React.HTMLAttributes<HTMLDivElement> {}
This would allow easy accessing of underlying instance properties/methods of HTML elements (e.g. className
, onClick
, children
, etc.).
See this cheatsheet for a more detailed explanation on why this makes sense and what the use-cases for this are.
Example result for className
:
Even then, if you want to enforce property usage for e.g. className
, you can still just add:
interface MagicContainerProps extends React.HTMLAttributes<HTMLDivElement> {
className: string
}
Cheers, Julian
@Julian-AT good idea this approach is better
I recently updated the way magic card is implemented in general to simplify it even further
What are your thoughts about the new approach?
Hey @dillionverma,
just opend a PR #234 on how I would improve the component.
Key benefits:
useCallback
, enhancing efficiencygradientOpacity
. In the current deployment the prop exists in the interface but it isn't implemented anywhere.Cheers, Julian
thanks so much for adding this! read through it today and tested it myself. looks good to me!
The MagicCard component currently uses
any
as type for theclassName
property in the Magic Card component.https://github.com/magicuidesign/magicui/blob/9331f3c750d4a96fe09bd7b955e9cc75f515c7ed/registry/components/magicui/magic-card.tsx#L40-L43
If there is no specific reason for
any
, a change tostring
should make it cleaner to use. Especially since many linters dislike explicit any usage, see ESLint (https://typescript-eslint.io/rules/no-explicit-any/).