Change minimum viewport width to be exclusive whereas the maximum width 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.

Author: westonruter

plugins/optimization-detective/class-od-link-collection.php

@@ -18,7 +18,7 @@
  * @phpstan-type Link array{
  *                   attributes: LinkAttributes,
  *                   minimum_viewport_width: int<0, max>|null,
- *                   maximum_viewport_width: positive-int|null
+ *                   maximum_viewport_width: int<1, max>|null
  *               }
  *
  * @phpstan-type LinkAttributes array{
@@ -53,9 +53,9 @@ final class OD_Link_Collection implements Countable {
 	 *
 	 * @phpstan-param LinkAttributes $attributes
 	 *
-	 * @param array             $attributes             Attributes.
-	 * @param int<0, max>|null  $minimum_viewport_width Minimum width or null if not bounded or relevant.
-	 * @param positive-int|null $maximum_viewport_width Maximum width or null if not bounded (i.e. infinity) or relevant.
+	 * @param array            $attributes             Attributes.
+	 * @param int<0, max>|null $minimum_viewport_width Minimum width (exclusive) or null if not bounded or relevant.
+	 * @param int<1, max>|null $maximum_viewport_width Maximum width (inclusive) or null if not bounded (i.e. infinity) or relevant.
 	 *
 	 * @throws InvalidArgumentException When invalid arguments are provided.
 	 */
@@ -194,7 +194,7 @@ static function ( array $carry, array $link ): array {
 					&&
 					is_int( $last_link['maximum_viewport_width'] )
 					&&
-					$last_link['maximum_viewport_width'] + 1 === $link['minimum_viewport_width']
+					$last_link['maximum_viewport_width'] === $link['minimum_viewport_width']
 				) {
 					$last_link['maximum_viewport_width'] = max( $last_link['maximum_viewport_width'], $link['maximum_viewport_width'] );
 

plugins/optimization-detective/class-od-url-metric-group-collection.php

@@ -244,13 +244,13 @@ public function clear_cache(): void {
 	 * @return OD_URL_Metric_Group[] Groups.
 	 */
 	private function create_groups(): array {
-		$groups    = array();
-		$min_width = 0;
-		foreach ( $this->breakpoints as $max_width ) {
-			$groups[]  = new OD_URL_Metric_Group( array(), $min_width, $max_width, $this->sample_size, $this->freshness_ttl, $this );
-			$min_width = $max_width + 1;
+		$groups              = array();
+		$min_width_exclusive = 0;
+		foreach ( $this->breakpoints as $max_width_inclusive ) {
+			$groups[]            = new OD_URL_Metric_Group( array(), $min_width_exclusive, $max_width_inclusive, $this->sample_size, $this->freshness_ttl, $this );
+			$min_width_exclusive = $max_width_inclusive;
 		}
-		$groups[] = new OD_URL_Metric_Group( array(), $min_width, PHP_INT_MAX, $this->sample_size, $this->freshness_ttl, $this );
+		$groups[] = new OD_URL_Metric_Group( array(), $min_width_exclusive, PHP_INT_MAX, $this->sample_size, $this->freshness_ttl, $this );
 		return $groups;
 	}
 
@@ -285,7 +285,7 @@ public function add_url_metric( OD_URL_Metric $new_url_metric ): void {
 	 * @since 0.1.0
 	 * @throws InvalidArgumentException When there is no group for the provided viewport width. This would only happen if a negative width is provided.
 	 *
-	 * @param int $viewport_width Viewport width.
+	 * @param positive-int $viewport_width Viewport width.
 	 * @return OD_URL_Metric_Group URL Metric group for the viewport width.
 	 */
 	public function get_group_for_viewport_width( int $viewport_width ): OD_URL_Metric_Group {

plugins/optimization-detective/class-od-url-metric-group.php

@@ -32,12 +32,11 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer
 	private $url_metrics;
 
 	/**
-	 * Minimum possible viewport width for the group (inclusive).
+	 * Minimum possible viewport width for the group (exclusive).
 	 *
 	 * @since 0.1.0
 	 *
-	 * @var int
-	 * @phpstan-var 0|positive-int
+	 * @var int<0, max>
 	 */
 	private $minimum_viewport_width;
 
@@ -46,8 +45,7 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer
 	 *
 	 * @since 0.1.0
 	 *
-	 * @var int
-	 * @phpstan-var positive-int
+	 * @var int<1, max>
 	 */
 	private $maximum_viewport_width;
 
@@ -103,8 +101,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer
 	 * @throws InvalidArgumentException If arguments are invalid.
 	 *
 	 * @param OD_URL_Metric[]                $url_metrics            URL Metrics to add to the group.
-	 * @param int                            $minimum_viewport_width Minimum possible viewport width for the group. Must be zero or greater.
-	 * @param int                            $maximum_viewport_width Maximum possible viewport width for the group. Must be greater than zero and the minimum viewport width.
+	 * @param int                            $minimum_viewport_width Minimum possible viewport width (exclusive) for the group. Must be zero or greater.
+	 * @param int                            $maximum_viewport_width Maximum possible viewport width (inclusive) for the group. Must be greater than zero and the minimum viewport width.
 	 * @param int                            $sample_size            Sample size for the maximum number of viewports in a group between breakpoints.
 	 * @param int                            $freshness_ttl          Freshness age (TTL) for a given URL Metric.
 	 * @param OD_URL_Metric_Group_Collection $collection             Collection that this instance belongs to.
@@ -158,12 +156,12 @@ public function __construct( array $url_metrics, int $minimum_viewport_width, in
 	}
 
 	/**
-	 * Gets the minimum possible viewport width (inclusive).
+	 * Gets the minimum possible viewport width (exclusive).
 	 *
 	 * @since 0.1.0
 	 *
 	 * @todo Eliminate in favor of readonly public property.
-	 * @return int<0, max> Minimum viewport width.
+	 * @return int<0, max> Minimum viewport width (exclusive).
 	 */
 	public function get_minimum_viewport_width(): int {
 		return $this->minimum_viewport_width;
@@ -175,7 +173,7 @@ public function get_minimum_viewport_width(): int {
 	 * @since 0.1.0
 	 *
 	 * @todo Eliminate in favor of readonly public property.
-	 * @return int<1, max> Minimum viewport width.
+	 * @return int<1, max> Minimum viewport width (inclusive).
 	 */
 	public function get_maximum_viewport_width(): int {
 		return $this->maximum_viewport_width;
@@ -208,16 +206,18 @@ public function get_freshness_ttl(): int {
 	}
 
 	/**
-	 * Checks whether the provided viewport width is within the minimum/maximum range for.
+	 * Checks whether the provided viewport width is between the minimum (exclusive) and maximum (inclusive).
 	 *
 	 * @since 0.1.0
 	 *
+	 * @phpstan-param int<1, max> $viewport_width
+	 *
 	 * @param int $viewport_width Viewport width.
 	 * @return bool Whether the viewport width is in range.
 	 */
 	public function is_viewport_width_in_range( int $viewport_width ): bool {
 		return (
-			$viewport_width >= $this->minimum_viewport_width &&
+			$viewport_width > $this->minimum_viewport_width &&
 			$viewport_width <= $this->maximum_viewport_width
 		);
 	}

plugins/optimization-detective/class-od-url-metric.php

@@ -16,8 +16,8 @@
  * Representation of the measurements taken from a single client's visit to a specific URL.
  *
  * @phpstan-type ViewportRect array{
- *                                width: int,
- *                                height: int
+ *                                width: positive-int,
+ *                                height: positive-int
  *                            }
  * @phpstan-type DOMRect      array{
  *                                width: float,
@@ -249,12 +249,12 @@ public static function get_json_schema(): array {
 						'width'  => array(
 							'type'     => 'integer',
 							'required' => true,
-							'minimum'  => 0,
+							'minimum'  => 1,
 						),
 						'height' => array(
 							'type'     => 'integer',
 							'required' => true,
-							'minimum'  => 0,
+							'minimum'  => 1,
 						),
 					),
 					'additionalProperties' => false,
@@ -482,7 +482,7 @@ public function get_viewport(): array {
 	 *
 	 * @since 0.1.0
 	 *
-	 * @return int Viewport width.
+	 * @return positive-int Viewport width.
 	 */
 	public function get_viewport_width(): int {
 		return $this->data['viewport']['width'];

plugins/optimization-detective/detect.js

@@ -98,20 +98,30 @@ function error( ...message ) {
 /**
  * Checks whether the URL Metric(s) for the provided viewport width is needed.
  *
+ * The comparison logic here corresponds with the PHP logic in `OD_URL_Metric_Group::is_viewport_width_in_range()`.
+ *
  * @param {number}                 viewportWidth          - Current viewport width.
  * @param {URLMetricGroupStatus[]} urlMetricGroupStatuses - Viewport group statuses.
  * @return {boolean} Whether URL Metrics are needed.
  */
 function isViewportNeeded( viewportWidth, urlMetricGroupStatuses ) {
-	let lastWasLacking = false;
-	for ( const { minimumViewportWidth, complete } of urlMetricGroupStatuses ) {
-		if ( viewportWidth >= minimumViewportWidth ) {
-			lastWasLacking = ! complete;
-		} else {
-			break;
+	if ( viewportWidth === 0 ) {
+		return false;
+	}
+
+	for ( const {
+		minimumViewportWidth,
+		maximumViewportWidth,
+		complete,
+	} of urlMetricGroupStatuses ) {
+		if (
+			viewportWidth > minimumViewportWidth &&
+			viewportWidth <= maximumViewportWidth
+		) {
+			return complete;
 		}
 	}
-	return lastWasLacking;
+	return false;
 }
 
 /**

plugins/optimization-detective/detection.php

@@ -128,7 +128,8 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $
 		'urlMetricGroupStatuses' => array_map(
 			static function ( OD_URL_Metric_Group $group ): array {
 				return array(
-					'minimumViewportWidth' => $group->get_minimum_viewport_width(),
+					'minimumViewportWidth' => $group->get_minimum_viewport_width(), // Exclusive.
+					'maximumViewportWidth' => $group->get_maximum_viewport_width(), // Inclusive.
 					'complete'             => $group->is_complete(),
 				);
 			},

plugins/optimization-detective/helper.php

@@ -37,18 +37,18 @@ function od_initialize_extensions(): void {
  *
  * @since 0.7.0
  *
- * @param int|null $minimum_viewport_width Minimum viewport width.
- * @param int|null $maximum_viewport_width Maximum viewport width.
+ * @param int<0, max>|null $minimum_viewport_width Minimum viewport width (exclusive).
+ * @param int<1, max>|null $maximum_viewport_width Maximum viewport width (inclusive).
  * @return non-empty-string|null Media query, or null if the min/max were both unspecified or invalid.
  */
 function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_viewport_width ): ?string {
-	if ( is_int( $minimum_viewport_width ) && is_int( $maximum_viewport_width ) && $minimum_viewport_width > $maximum_viewport_width ) {
-		_doing_it_wrong( __FUNCTION__, esc_html__( 'The minimum width cannot be greater than the maximum width.', 'optimization-detective' ), 'Optimization Detective 0.7.0' );
+	if ( is_int( $minimum_viewport_width ) && is_int( $maximum_viewport_width ) && $minimum_viewport_width >= $maximum_viewport_width ) {
+		_doing_it_wrong( __FUNCTION__, esc_html__( 'The minimum width cannot be greater than or equal to the maximum width.', 'optimization-detective' ), 'Optimization Detective 0.7.0' );
 		return null;
 	}
 	$media_attributes = array();
 	if ( null !== $minimum_viewport_width && $minimum_viewport_width > 0 ) {
-		$media_attributes[] = sprintf( '(min-width: %dpx)', $minimum_viewport_width );
+		$media_attributes[] = sprintf( '(min-width: %dpx)', $minimum_viewport_width + 1 );
 	}
 	if ( null !== $maximum_viewport_width && PHP_INT_MAX !== $maximum_viewport_width ) {
 		$media_attributes[] = sprintf( '(max-width: %dpx)', $maximum_viewport_width );

plugins/optimization-detective/tests/storage/test-rest-api.php

@@ -573,7 +573,7 @@ static function () use ( $breakpoint_width ): array {
 		);
 		$url_metric_groups = iterator_to_array( $group_collection );
 		$this->assertSame(
-			array( 0, $breakpoint_width + 1 ),
+			array( 0, $breakpoint_width ),
 			array_map(
 				static function ( OD_URL_Metric_Group $group ) {
 					return $group->get_minimum_viewport_width();

plugins/optimization-detective/tests/test-class-od-link-collection.php

@@ -79,9 +79,9 @@ public function data_provider_to_test_add_link(): array {
 					),
 				),
 				'expected_html'   => '
-					<link data-od-added-tag rel="preload" href="https://example.com/foo.jpg" crossorigin="anonymous" fetchpriority="high" as="image" media="screen and (min-width: 100px) and (max-width: 200px)">
+					<link data-od-added-tag rel="preload" href="https://example.com/foo.jpg" crossorigin="anonymous" fetchpriority="high" as="image" media="screen and (min-width: 101px) and (max-width: 200px)">
 				',
-				'expected_header' => 'Link: <https://example.com/foo.jpg>; rel="preload"; crossorigin="anonymous"; fetchpriority="high"; as="image"; media="screen and (min-width: 100px) and (max-width: 200px)"',
+				'expected_header' => 'Link: <https://example.com/foo.jpg>; rel="preload"; crossorigin="anonymous"; fetchpriority="high"; as="image"; media="screen and (min-width: 101px) and (max-width: 200px)"',
 				'expected_count'  => 1,
 				'error'           => '',
 			),
@@ -116,15 +116,15 @@ public function data_provider_to_test_add_link(): array {
 							'as'            => 'image',
 							'media'         => 'screen',
 						),
-						201,
+						200,
 						300,
 					),
 				),
 				'expected_html'   => '
 					<link data-od-added-tag rel="preload" href="https://example.com/bar.jpg" as="image" media="screen">
-					<link data-od-added-tag rel="preload" href="https://example.com/foo.jpg" crossorigin="anonymous" fetchpriority="high" as="image" media="screen and (min-width: 100px) and (max-width: 300px)">
+					<link data-od-added-tag rel="preload" href="https://example.com/foo.jpg" crossorigin="anonymous" fetchpriority="high" as="image" media="screen and (min-width: 101px) and (max-width: 300px)">
 				',
-				'expected_header' => 'Link: <https://example.com/bar.jpg>; rel="preload"; as="image"; media="screen", <https://example.com/foo.jpg>; rel="preload"; crossorigin="anonymous"; fetchpriority="high"; as="image"; media="screen and (min-width: 100px) and (max-width: 300px)"',
+				'expected_header' => 'Link: <https://example.com/bar.jpg>; rel="preload"; as="image"; media="screen", <https://example.com/foo.jpg>; rel="preload"; crossorigin="anonymous"; fetchpriority="high"; as="image"; media="screen and (min-width: 101px) and (max-width: 300px)"',
 				'expected_count'  => 3,
 				'error'           => '',
 			),
@@ -135,7 +135,7 @@ public function data_provider_to_test_add_link(): array {
 							'rel'  => 'preconnect',
 							'href' => 'https://youtube.com/',
 						),
-						201,
+						200,
 						300,
 					),
 				),
@@ -154,7 +154,7 @@ public function data_provider_to_test_add_link(): array {
 							'href'  => 'https://youtube.com/',
 							'media' => 'tty',
 						),
-						201,
+						200,
 						300,
 					),
 				),