Breakpoint is set even though screen width is greater than its query
Hi! I'm working on integrating this module into our prod site, and trying to align with our current breakpoint usage I ran into the following issue where xl breakpoint is set even when screen size is greater than its query, for example viewport.match('xl') is true on screen width 1600px while its query is (min-width: 1279px) and (max-width: 1536px)
I'd like $xl to be false on screen sizes greater than 1536px. Any ideas how to achieve that?
See minimal repro:
https://stackblitz.com/edit/github-gm83xj?file=plugins%2Fmixins.ts
Thank you for this great module!
@meirroth Hi. Oh, I though you have tested with your case.. Okay, I'll need some time to fix that.
@mvrlin I didn't catch this edge case 🤦♂️
@meirroth I created a branch for you https://github.com/mvrlin/nuxt-viewport/tree/max-width, try locally please, that should work
@mvrlin I tired your branch and it changed the media query of xl from (min-width: 1279px) and (max-width: 1536px) to (min-width: 1279px), It didn't fix the issue. Not even sure there is an issue with the queries. If you look at the repro link I shared above, the media queries look fine.
@mvrlin I found a temporary fix, by adding a dummy breakpoint to the end https://stackblitz.com/edit/github-gm83xj-gaurep?file=nuxt.config.ts
@meirroth What's wrong with my solution? When you set max-width: N, greater resolutions then the edge one will not be trigger, so because of that I made the last query the minimum of the penultimate
@mvrlin Thank you for taking the time to investigate this issue!
Your solution didn't solve the issue initially described. It replaced the media query of xl from being (min-width: 1279px) and (max-width: 1536px) to (min-width: 1279px) with my config, which isn't right.
xl should have a max-width query just like all other breakpoints, and it should be set to the N defined in the config.
Setting xl: 1536 currently produces media query (min-width: 1279px) and (max-width: 1536px) which is correct! It's the $viewport.breakpoint and .match() that are wrong, when screen is greater than the last breakpoint. Let me know if that makes sense.
@meirroth
Please try to test it with window.matchMedia
@mvrlin That perfectly demonstrates the issue, thanks for the tip.
See how on screen width 1600px window.matchMedia("(min-width: 1279px) and (max-width: 1536px)").matches evaluates to false, unlike this module which returns true from viewport.match('xl').
I would expect viewport.match('xl') to return false in this case. You see what I mean?
@meirroth Yes, but the issue here is that it sets the value based on the last truthy match. If you try to run every media query, you will see that on resolutions greater than 1536 every media query will be false
@mvrlin So do you agree that viewport.match('xl') should return false, and viewport.breakpoint should be null on resolutions greater than 1536, where xs: 1536 is configured?
@meirroth In this case yes, but you will be left with no breakpoint then, or as you did previously only by setting some large number which is not ideal
@mvrlin I think that makes most sense, because it's consistent with how CSS media queries work.
@meirroth If you want the $xl to false, change it from:
return this.viewport.isLessThan('xl') || this.viewport.match('xl');
to:
return this.viewport.match('xl');
@mvrlin That doesn't help. As I said before, viewport.match('xl') returns true on resolutions greater than 1536, where xs: 1536 is configured. I think that's a bug, considering the query for xl is (min-width: 1279px) and (max-width: 1536px) and window.matchMedia("(min-width: 1279px) and (max-width: 1536px)").matches evaluates to false.
@meirroth Are you referring to the latest release or the branch that I made for you?
@mvrlin Both.
@meirroth Yes, but the issue here is that it sets the value based on the last truthy match. If you try to run every media query, you will see that on resolutions greater than 1536 every media query will be
false
@meirroth This is because of that. You are switching between breakpoints and 1600px doesn't fit with any.
@mvrlin Right, so should it return null or undefined instead of keeping the last breakpoint?
@meirroth I don't think that it should return nullable value because that could break something easily.
@mvrlin Fine, so not sure if you want to change something in the module or not. For now I've deployed this workaround to prod, and it seems to work 😏
@mvrlin I found a temporary fix, by adding a dummy breakpoint to the end https://stackblitz.com/edit/github-gm83xj-gaurep?file=nuxt.config.ts
@meirroth I'm not sure what do you expect the breakpoint to be, because it cannot be nullable by design 😅
@meirroth I don't fully understand why it can't be nullable by design (can you please explain?), but I think it doesn't make sense for it to not match the media query, as described before:
...
viewport.match('xl')returnstrueon resolutions greater than 1536, wherexs: 1536is configured. I think that's a bug, considering the query forxlis(min-width: 1279px) and (max-width: 1536px)andwindow.matchMedia("(min-width: 1279px) and (max-width: 1536px)").matchesevaluates tofalse.
@meirroth Because if so there is something wrong with the query or the device
@mvrlin Got it.
If you agree with "it doesn't make sense for it to not match the media query" then what do you suggest we do to fix it?
Otherwise, we can close this issue and perhaps we mention this edge case in the README for those using maxWidth?
@mvrlin hey 👋 what's the latest on this?
@meirroth Hi! I decided to close the issues that were hanging here. Yes, I will mention about the edge case in the README.
@mvrlin Thanks!