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

Consider switch to CSS range syntax for media queries in Optimization Detective #1696

Closed
westonruter opened this issue Nov 22, 2024 · 0 comments · Fixed by #1833 or #1839
Closed

Consider switch to CSS range syntax for media queries in Optimization Detective #1696

westonruter opened this issue Nov 22, 2024 · 0 comments · Fixed by #1833 or #1839
Labels
Needs Discussion Anything that needs a discussion/agreement [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

Feature Description

The od_generate_media_query() helper function is used to generate media queries like the following:

@media (min-width: 481px) and (max-width: 600px) { 
    #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 400px; } 
}
@media (min-width: 601px) and (max-width: 782px) { 
    #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } 
}

The max-width numbers are coming from od_get_breakpoint_max_widths() with the min-width numbers being one greater than the previous max-width.

However, could there be an issue where windows are sized sub-pixel dimensions, so that a window is for example 601.5px wide? In this case, neither of these rules would apply and it would be better to use the new CSS range syntax (Can I use?). See writeup. The above CSS could be rewritten instead as:

@media (480px < width <= 600px) { 
    #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 400px; } 
}
@media (600px < width <= 782px) { 
    #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } 
}

This is more concise and it addresses any sub-pixel concerns, although it seems this is only an issue in Firefox according to What to do about sub-pixel media-queries?

@westonruter westonruter added [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature Needs Discussion Anything that needs a discussion/agreement labels Nov 22, 2024
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Nov 22, 2024
westonruter added a commit that referenced this issue Feb 1, 2025
…th remains inclusive

This will allow for creating media queries that avoid any 1px gaps. The key code change is in `OD_URL_Metric_Group::is_viewport_width_in_range()` (and `isViewportNeeded` in `detect.js`). This also avoid the need to add the number 1 to the minimum viewport width which can be confusing given the known breakpoints. This instead moves the addition of 1 at the moment to `od_generate_media_query()` but this will be removed with resolution of #1696.

Require that a viewport width and height for a URL Metric be at least 1px.
@github-project-automation github-project-automation bot moved this from Not Started/Backlog 📆 to Done 😃 in WP Performance 2024 Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion Anything that needs a discussion/agreement [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: Done 😃
1 participant