roderickhsiao / react-aspect-ratio

Preserve space for your element to prevent browser reflow (layout shift)
https://roderickhsiao.github.io/react-aspect-ratio/
MIT License
103 stars 7 forks source link

Allow all div props to be passed to `AspectRatio` #78

Closed chrissantamaria closed 1 year ago

chrissantamaria commented 1 year ago

👋 tiny PR - currently, AspectRatio only accepts a handful of props:

https://github.com/roderickhsiao/react-aspect-ratio/blob/a52f99bc8923c75199c351aea3e6804cc0ba5955/src/react-latest.tsx#L4-L8

However, this prevents relatively simple cases like passing a ref:

function SomeComponent() {
  const ref = useRef<HTMLDivElement>(null);

  return (
    // Property 'ref' does not exist on type 'IntrinsicAttributes & { className?: string; ratio?: string | number; style?: CSSProperties; } & { children?: ReactNode; }'
    <AspectRatio ref={ref}>
      some content
    </AspectRatio>
  );
}

This PR expands the prop definition to use React.ComponentProps<'div'> in addition to the extra ratio prop.

roderickhsiao commented 1 year ago

lgtm, but just from the example, we didnt forward ref in the component, do you want to add it as well in case you thats the use case you want to use?

chrissantamaria commented 1 year ago

ref was just one example - we could include it somewhere in the README, but I'd probably want to be more broad and describe that extra props will get passed to the underlying div (as it has functionally done even before this PR)

If you think that would be helpful, I can add to this PR

roderickhsiao commented 1 year ago

fine for this PR, if we want to support ref we can add it :) Most of people dont need to control over this wrapper div, so can handle ref in the inner component