nandorojo / dripsy

🍷 Responsive, unstyled UI primitives for React Native + Web.
https://dripsy.xyz
MIT License
2.01k stars 80 forks source link

getBreakpointIndex returns incorrect value #186

Closed JamesHemery closed 2 years ago

JamesHemery commented 2 years ago

Hi @nandorojo,

I think there is a problem with getBreakpointIndex. On the code below, we notice that when the index is found, we add 1 to this index.

However if I call the function with the default breakpoints (i.e. [576, 768, 992, 1200]) and a width of 1400, the index that the function sends is 4 instead of 3.

Is this behavior intentional?

export const getBreakpointIndex = ({
  width,
  breakpoints,
}: {
  width: number
  breakpoints: number[]
}) => {
  const breakpointPixels = [...breakpoints]
    .reverse()
    .find((breakpoint) => width >= breakpoint)

  let breakpointIndex = breakpoints.findIndex(
    (breakpoint) => breakpointPixels === breakpoint
  )
  breakpointIndex = breakpointIndex === -1 ? 0 : breakpointIndex + 1

  return breakpointIndex
}

Thanks,

nandorojo commented 2 years ago

Thanks for noting this. I believe the reason this happens is because we prepend 0 to the array when we map the props to style. (It's been a while since I've looked at this.) However, I could see there being incorrect behavior here, for hooks like useResponsiveValue. The code for finding the right breakpoint looks a bit hard to read 😬

If there are some test cases that prove the wrong behavior I'm more than happy to help fix / merge a PR.

Thanks!

JamesHemery commented 2 years ago

@nandorojo Ok, I'll try to make time for an PR.

nandorojo commented 2 years ago

great thanks! even a codesandbox showing the cases it works/breaks would be useful. I believe the cases would be, a component, useResponsiveValue, etc. The code is so convoluted for it, I should find a way to simplify it and add better tests.

JamesHemery commented 2 years ago

Finally, upon further analysis, it makes sense since breakpoints are handled like this:

Of course, these names are there to illustrate my props.

On my side I use a more friendly hook:

import { useBreakpointIndex } from 'dripsy';

const BREAKPOINTS_NAME = ['xs', 'sm', 'md', 'lg', 'xl'];

export default function useBreakpointName() {
  const breakpointSize = useBreakpointIndex();
  return BREAKPOINTS_NAME[breakpointSize];
}