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

Improvements to useWindowSize hook #788

Open
joshfarrant opened this issue Oct 15, 2024 · 1 comment
Open

Improvements to useWindowSize hook #788

joshfarrant opened this issue Oct 15, 2024 · 1 comment
Labels

Comments

@joshfarrant
Copy link
Contributor

joshfarrant commented Oct 15, 2024

There are a couple of things which could be improved in the useWindowSize hook. The most notable is the behaviour of the is[Size] booleans returned by the hook.

The current behaviour of the hook is, in my opinion, slightly counter-intuitive. For example, isSmall will return true when the viewport width is any size greater than the small breakpoint. I'd expect isSmall to only return true when the viewport width is greater than or equal to the small breakpoint, and less than the medium breakpoint. Eg

- isXSmall: window.innerWidth >= 320,
- isSmall: window.innerWidth >= 544,
- isMedium: window.innerWidth >= 768,
- isLarge: window.innerWidth >= 1012,
- isXLarge: window.innerWidth >= 1280,
+ isXSmall: window.innerWidth >= 320 && window.innerWidth < 544,
+ isSmall: window.innerWidth >= 544 && window.innerWidth < 768,
+ isMedium: window.innerWidth >= 768 && window.innerWidth < 1012,
+ isLarge: window.innerWidth >= 1012 && window.innerWidth < 1280,
+ isXLarge: window.innerWidth >= 1280 && window.innerWidth < 1440,
isXXLarge: window.innerWidth >= 1440,

I can see the merits of the existing behaviour, but it caught me out the first time I looked at the hook, as it did @victoriaNine (link to Slack conversation).

Another potential improvement for the hook would be to have it use a context to ensure that only one 'resize' event listener is added to the window, regardless of how many times the hook is called. At the moment, every time the hook is called a new listener is added to the window, which is a bit redundant.

@simurai
Copy link
Contributor

simurai commented Oct 15, 2024

For "smaller than" (<), the values would need to be 1px less. E.g. < 543, < 767 etc.. Otherwise when the viewport is for example exactly 544, both isXSmall + isSmall would be true.

@rezrah rezrah added the brand label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants