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

Create hooks for @vx/responsive #485

Closed
pedroapfilho opened this issue Aug 27, 2019 · 7 comments
Closed

Create hooks for @vx/responsive #485

pedroapfilho opened this issue Aug 27, 2019 · 7 comments

Comments

@pedroapfilho
Copy link

We already have the HOC (and thanks for that), but what if we make it as a hook? So we can use it as: useScreenSize.

BTW, I can make the PR for that

@williaster
Copy link
Collaborator

love this idea, noting that we have discussed re-implementing the current HOCs + <ParentSize /> component to use react-use-measure.

would happily review a PR!

@pedroapfilho
Copy link
Author

Alright then! I'll create the PR with this hook, using this lib!

Whenever I get to the point where I can show something, I'll post it here! Thanks!

@pedroapfilho
Copy link
Author

After working on it for a bit, I think it's not worth creating it here. What we could do is add documentation to use react-use-measure. Their API is "perfect", and what I was doing was the same thing as they already did, so look at this example:

import useMeasure from 'react-use-measure'

function App() {
  const [ref, bounds] = useMeasure()

  return (
	<div ref={ref}>
		<svg width={bounds.width} height={bounds.height} />
	</div>
}

The only thing that would change from our implementation to theirs, is the way we import it, that would probably be:

import { useMeasure } from '@visx/responsive'

So what do you think of just adding this dependency to the docs?

Another alternative would be to create our own, and it would work too.

@pedroapfilho
Copy link
Author

Re-thinking about it, I think we can change the implementation to something like:

<MeasureProvider>
	<Component />
</MeasureProvider>

and we can return the bounds object to the hooks, like:

const bounds = useMeasure()

And then we could simplify the usage in our case. This would simplify a lot the creation of responsive charts, as we won't need to pass the width and height down the component tree.

@williaster and @hshoff what do you guys think?

@williaster
Copy link
Collaborator

williaster commented Jan 28, 2021

Hey @pedroapfilho 👋 . In the latest suggestion, how would the ref work with MeasureProvider? Would it render some type of element and handle all of the ref stuff?

That's an interesting thought. I do think sometimes users would want to be able to avoid rendering an additional div (or svg if you're measuring in an svg tree) and set their own ref on an element they're already rendering, so need to think more about how that might work 🤔

@pedroapfilho
Copy link
Author

What this provider would do is creating the ref for the user, by himself. It could accept props to pass its own ref, but it would just for certain use-cases, that aren't that important.

The MesureProvider would do something like:

<div ref={ref}>
	<Measure.Provider value={bounds}>
		{children}
	</Measure.Provider>
</div>

@hshoff
Copy link
Member

hshoff commented Mar 25, 2024

Closing this as it looks like hooks were added in #1783

@hshoff hshoff closed this as completed Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants