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

Remove initial-scale=1 meta viewport property by default to disable tap delay #5903

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

westonruter
Copy link
Member

Summary

Fixes #5894

Behavior can be disabled via this plugin code:

add_filter(
	'amp_content_sanitizers',
	function ( $sanitizers ) {
		$sanitizers[ AMP_Meta_Sanitizer::class ]['remove_initial_scale_viewport_property'] = false;
		return $sanitizers;
	}
);

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added this to the v2.1 milestone Feb 19, 2021
@github-actions
Copy link
Contributor

Plugin builds for e4124eb are ready 🛎️!

@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #5903 (429ff34) into develop (ced3165) will decrease coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #5903      +/-   ##
=============================================
- Coverage      75.12%   75.12%   -0.01%     
- Complexity      5667     5670       +3     
=============================================
  Files            210      210              
  Lines          17023    17027       +4     
=============================================
+ Hits           12789    12792       +3     
- Misses          4234     4235       +1     
Flag Coverage Δ Complexity Δ
javascript 75.13% <ø> (ø) 0.00 <ø> (ø)
php 75.12% <75.00%> (-0.01%) 0.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
includes/sanitizers/class-amp-meta-sanitizer.php 79.79% <75.00%> (-0.21%) 35.00 <0.00> (+3.00) ⬇️

@pierlon
Copy link
Contributor

pierlon commented Feb 23, 2021

Will this resolve the page experience guide check? According to the linter rule, if the content value of the meta[name=viewport] property doesn't exactly equal width=device-width, the test will fail.

@westonruter
Copy link
Member Author

It should resolve it at least for the core themes which have width and (often) initial-scale, right?

@pierlon
Copy link
Contributor

pierlon commented Feb 23, 2021

Right, in the context of the core themes this would be fine.

Copy link
Contributor

@pierlon pierlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Themes that make use of other viewport properties (such as maximum-scale=1) will not benefit from this even when the content value is reduced to width=device-width, although I would assume such occurrences to be rare.

@westonruter
Copy link
Member Author

Yeah, for other themes then the PX check would complain and that's good. If a theme is using other viewport properties we can't safely automatically remove them as we can apparently do for initial-scale=1. For them the theme authors should fix the viewport, at least for now. Maybe we'll go further in the future.

@pierlon pierlon added WS:Core Work stream for Plugin core WS:Perf Work stream for Metrics, Performance and Optimizer and removed WS:Core Work stream for Plugin core labels Feb 23, 2021
@westonruter westonruter merged commit 4a91d53 into develop Feb 23, 2021
@westonruter westonruter deleted the remove/initial-scale-1-viewport-property branch February 23, 2021 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable tap delay by reducing meta viewport to just width=device-width
2 participants