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

Add "useBreakpoint" hook #279

Open
3 tasks
kettanaito opened this issue Jan 2, 2020 · 7 comments · May be fixed by #318
Open
3 tasks

Add "useBreakpoint" hook #279

kettanaito opened this issue Jan 2, 2020 · 7 comments · May be fixed by #318
Labels
feature needs:discussion Further information is requested scope:calculation This issue relates to the calculation logic
Milestone

Comments

@kettanaito
Copy link
Owner

kettanaito commented Jan 2, 2020

This is an API refinement story preparation for the first major release. It introduces a breaking change and will be released respectively.

What:

I suggest to add the following React hook:

type UseBreakpoint<T> = ((breakpointName: string) => T) => [string, T]

Why:

It should replace current useBreakpointChange hook with both:

  1. Information about the current breakpoint (first argument in a tuple),
  2. Updater function that is called on each breakpoint change (mirroring of useBreakpointChange.
  3. useBreakpoint will be called on initial render, when useBreakpointChange is not.

Main motivation is that it's currently lengthy to know which breakpoint is met. It requires to have useState updating its value on useBreakpointChange callback. With the suggested hook added it can be written in a single hook declaration.

Usage example:

Getting a breakpoint name

import { useBreakpoint } from 'atomic-layout'

const Component = () => {
  const [breakpointName] = useBreakpoint()
  return <p>You are on {breakpointName} screen</p>
}

Reacting to breakpoint change

import { useBreakpoint } from 'atomic-layout'

const dataMap = {
  xs: 'one',
  sm: 'two'
}

const Component = () => {
  const [breakpointName, data] = useBreakpoint((breakpointName) => {
    return dataMap[breakpointName]
  })
  return <p>You are on {breakpointName} screen with {data}</p>
}

How:

  • Deprecate useBreakpointChange
  • Add useBreakpoint
  • Add unit tests
@kettanaito kettanaito added scope:calculation This issue relates to the calculation logic feature labels Jan 2, 2020
@kettanaito kettanaito added this to the 1.0 milestone Jan 2, 2020
@kettanaito kettanaito added the needs:discussion Further information is requested label Jan 2, 2020
@kettanaito
Copy link
Owner Author

kettanaito commented Jan 2, 2020

Questions

  • Is breakpointName even useful? What can be the usage example?

It's about never you have interest in a matched breakpoint name without deriving something from it. To do the latter there is a callback function (hook argument), which replaces the need to return the breakpoint name. I'd probably make the useBreakpoint hook return the data only.

@vitordino
Copy link

i also find this interesting, i did came up with 2 hooks on my lib:

  1. useCurrentBreakpoint that returns a string with the key of the current breakpoint
  2. useBreakpoints that returns an array containing all the visible breakpoints keys

@kettanaito
Copy link
Owner Author

kettanaito commented Apr 6, 2020

I absolutely love the useCurrentBreakpoint hook! I think it would be a nice replacement of the useBreakpointChange, which exposes you a callback with a currently matched breakpoint name. It'd be a much more versatile pattern to return a breakpoint name instead, and let the end developer decide what to do with it:

import { useBreakpointChange } from 'atomic-layout'

const Usage = () => {
  // Instead of inline side-effect
  useBreakpointChange((currentBreakpointName) => {...})

  // Prefer getting the active breakpoint name and doing whatever is needed with it
  const breakpointName = useCurrentBreakpoint()
  useEffect(() => {...}, [breakpointName])
}

@vitordino, could you please share a use case for the useBreakpoints hook you've mentioned?

@kettanaito kettanaito linked a pull request Apr 6, 2020 that will close this issue
7 tasks
@kettanaito
Copy link
Owner Author

kettanaito commented Apr 6, 2020

I think I understood why multiplicity is relevant when matching breakpoints. I've addressed it in the pull request attached to this issue. Basically, it's possible for multiple breakpoints to match the same state of the viewport.

The call signature I'm suggesting in the pull request is:

import React, { useEffect } from 'react'
import { useCurrentBreakpoints } from 'atomic-layout'

const Usage = () => {
  const breakpoints = useCurrentBreakpoints()

  useEffect(() => {
    if (breakpoints.includes('sm')) {
      doAction()
    }
  }, [breakpoints])

  return null
}

I can see such hooks being used with some sort of comparison util. For example isInRage('sm', { to: 'md' }). I'm not sure there's a lot of value in such standalone usage, though.

But then, once such utility is introduced, I see no difference between it and useResponsiveQuery, which allows you to check if the current viewport matches the query range.

@kettanaito
Copy link
Owner Author

I mainly wonder what is the benefit of the useCurrentBreakpoint hook compared to:

import { useResponsiveQuery } from 'atomic-layout'

const Usage = () => {
  // Target a single breakpoint
  const isLargeScreen = useResponsiveQuery({ for: 'lg' })
  // Or a breakpoint range
  const isBiggerThanMedium = useResponsiveQuery({ from: 'md' })
}

Effectively one reaches to a hook like useCurrentBreakpoint to find out what the current breakpoint is, which implies that it matches against the current viewport. I believe that match is the exact useful information, not the breakpoint name per say.

It would be nice to have a usage example of useCurrentBreakpoint to assess if it's a useful hook at the first place.

@vitordino
Copy link

yep, you caught one idea of using an array on useBreakpoints.. i think it’s just a different way to express it, and maybe use it..

i’m not agains the from/to (or below/above) approaches, i even think that probably it’s more powerful too, but i believe both API’s can live simultaneously..

just some other ways to let users of the lib explore the api (some will prefer only using one of it, but maybe there are best uses for both) 🤷

@kettanaito
Copy link
Owner Author

kettanaito commented Apr 11, 2020

@vitordino, you have a point. While reading your reply I've also realized that useCurrentBreakpoints has a slightly different behavior than useResponsiveQuery. I'll elaborate below.

useResponsiveQuery

Designed to determine a viewport match against a breakpoint, or a range of breakpoints. More precisely, a range of breakpoints of the same nature (i.e. dimensional, orientational, or pixel density breakpoints). With this hook you cannot, say, get the match in case you would like to know if it's a medium breakpoint or a retina display. You would have to use such hook twice:

const isMedium = useResponsiveQuery({ for: 'md' })
const isRetina = useResponsiveQuery({ for: 'retina' })

Usage example above assumes you have got two breakpoints defined: md and retina, using Layout.configure({}).

useCurrentBreakpoints

This hook, on the other hand, returns the list of all breakpoints that match the current viewport. Breakpoints in the list may be of a different nature.

Taking the same md/retina example above, you could get the match information in a single useCurrentBreakpoints hook call:

const breakpoints = useCurrentBreakpoints()

if (breakpoints.includes('md') && breakpoints.includes('retina')) {
  // do something
}

This does not necessarily mean that this is a more performant method, as useResponsiveQuery() establishes a matchMedia() only against a breakpoint or a range you've provided, while useCurrentBreakpoint checks if any declared Layout.options.breakpoints math the current viewport.

Conclusion

I believe that both hook have a place to be. I'd love for the users to give them a try and see what are the use cases for both. I've prepared the implementation for useCurrentBreakpoints in #318, and will continue on it, since my original question was answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature needs:discussion Further information is requested scope:calculation This issue relates to the calculation logic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants