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

(match-media): useBreakpointIndex defaultIndex option doesn't allow you to select the largest breakpoint #1191

Open
Toddish opened this issue Sep 17, 2020 · 1 comment
Assignees
Labels
help wanted Extra attention is needed

Comments

@Toddish
Copy link

Toddish commented Sep 17, 2020

Describe the bug
You can't specify the largest breakpoint as the defaultIndex in useBreakpointIndex when using ssr.

To Reproduce
Steps to reproduce the behavior:

  1. Create a new app using the base theme
  2. Import @theme-ui/match-media
  3. Create a component that uses useBreakpointIndex, setting the defaultIndex to 3
  4. Render the component using ssr

Expected behavior
useBreakpointIndex by default returns 0-3, because it gets the length of how many breakpoints it matches (of which there are three by default). I expect to be able to pass the max breakpoint index (3) as the default.

Additional context
There's a check for if the defaultIndex passed is < 0 || > breakpoints.length - 1, but really you're not passing in the actual breakpoint index you want, but the number of breakpoints the screen currently matches:

return breakpoints.filter(
  breakpoint =>
  window.matchMedia(`screen and (min-width: ${breakpoint})`).matches
).length

Which produces 0-3.

@atanasster
Copy link
Collaborator

@Toddish thanks for the report - makes sense. Although naming it useBreakpointIndex seems a bit counter-intuitive to me - if its called index it should be up to length -1 imho. And where its used inuseResponsiveValue, the returned index is subtracted by 1 since its not actually an index : return array[index >= array.length ? array.length - 1 : index]

What do you think, would you like to submit a PR?

@lachlanjc lachlanjc added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Dec 3, 2020
@lachlanjc lachlanjc changed the title useBreakpointIndex defaultIndex option doesn't allow you to select the largest breakpoint (match-media): useBreakpointIndex defaultIndex option doesn't allow you to select the largest breakpoint Dec 6, 2020
@lachlanjc lachlanjc added help wanted Extra attention is needed and removed bug Something isn't working labels Dec 6, 2020
@lachlanjc lachlanjc linked a pull request Dec 9, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants