Make WordPress Core

Opened 5 weeks ago

Last modified 5 weeks ago

#65841 new enhancement

theme.json settings.viewport: warn when a breakpoint is dropped

Reported by: bejignesh Owned by:
Priority: normal Milestone: Awaiting Review
Component: Editor Version: 7.1
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

Follow-up to #65833, requested by @wildworks in comment:3.

WP_Theme_JSON::sanitize_viewport_settings() drops tablet in two cases and says nothing either time:

  • tablet is not larger than mobile. Existing behaviour, unchanged by #65833.
  • mobile and tablet are measured against different bases, one px and the other em or rem, so their order cannot be determined. Added in #65833.

In both cases the theme gets a two state layout instead of the three states it configured, and nothing reports why.

A notice would match how the same class already reports invalid settings. settings.spacing.spacingScale does this in compute_spacing_sizes():

wp_trigger_error(
	__METHOD__,
	sprintf(
		/* translators: 1: theme.json, 2: settings.spacing.spacingScale */
		__( 'Some of the %1$s %2$s values are invalid' ),
		'theme.json',
		'settings.spacing.spacingScale'
	),
	E_USER_NOTICE
);

The two cases are not equivalent and may deserve different wording, or possibly only the first deserves a notice at all. A tablet smaller than mobile is a plain configuration mistake that is visible in the theme.json itself. Mixed bases are not something an author can reason about from the file or from the generated CSS, because the font size the media query resolves against belongs to the browser rather than to the page.

Not suitable for 7.1, since a notice needs a translatable string and 7.1 is past hard string freeze. That is why the fix in #65833 is silent.

Change History (2)

This ticket was mentioned in PR #12974 on WordPress/wordpress-develop by @hasnainashfaq.


5 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Fixes the silent failure described in #65841.

When WP_Theme_JSON::sanitize_viewport_settings() drops the tablet breakpoint because its value is not larger than mobile, it now triggers a wp_trigger_error() E_USER_NOTICE informing theme authors of the reason.

What the problem was:
When a theme.json sets a tablet viewport breakpoint whose value is equal to or smaller than mobile, the tablet key is silently removed. The theme gets a two-state layout instead of the three states it configured, with no indication of why.

What the fix does:
Adds an elseif branch in sanitize_viewport_settings() that fires wp_trigger_error() with E_USER_NOTICE when tablet is present but not larger than mobile. This matches the pattern already used in compute_spacing_sizes() for invalid spacingScale values.

Testing:

  • Adds a unit test that hooks wp_trigger_error_run and asserts the notice fires with the correct message content when tabletmobile.
  • Existing test test_get_viewport_media_queries_omits_tablet_when_its_breakpoint_is_not_larger_than_mobile() continues to verify the sanitized output is correct.

Notes:
The ticket also describes a second case (mixed em/px bases, added by #65833) that cannot be addressed here until #65833 lands. This PR covers only Case 1 from the ticket description.

See https://core.trac.wordpress.org/ticket/65841

#2 @hasnainashfaq
5 weeks ago

I've opened a PR for Case 1 (tablet not larger than mobile): https://github.com/WordPress/wordpress-develop/pull/12974

The fix adds an elseif branch in sanitize_viewport_settings() that fires wp_trigger_error() with E_USER_NOTICE when the tablet breakpoint is dropped, following the same pattern used in compute_spacing_sizes() for invalid spacingScale values.

Case 2 (mixed em/px bases) is left for a follow-up once #65833 lands.

Note: See TracTickets for help on using tickets.