Add web app manifest display setting in Reading settings page #985
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #985 +/- ##
=============================================
- Coverage 19.65% 19.38% -0.28%
Complexity 347 347
=============================================
Files 57 58 +1
Lines 2325 2358 +33
=============================================
Hits 457 457
- Misses 1868 1901 +33
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| register_setting( | ||
| 'reading', | ||
| 'web_app_manifest_display', | ||
| array( | ||
| 'type' => 'string', | ||
| 'sanitize_callback' => 'sanitize_text_field', | ||
| ) | ||
| ); |
There was a problem hiding this comment.
We should make sure this doesn't end up adding web_app_manifest_display as an autoloaded option, since it is only used in the web app manifest request.
I think this may mean adding a call to add_option before this call.
add_option( 'web_app_manifest_display', 'minimal-ui', '', false );Although there may be a better place for it.
I realize this should also be done for the short_name field.
There was a problem hiding this comment.
And for the offline_browsing option.
There was a problem hiding this comment.
For existing installs that have offline_browsing and short_name options that are autoloaded, they should switch to non-autoloaded.
| */ | ||
| function render_web_app_manifest_display_setting_field() { | ||
| // See: <https://developer.mozilla.org/en-US/docs/Web/Manifest/display#values>. | ||
| $allowed_values = array( 'fullscreen', 'standalone', 'minimal-ui', 'browser' ); |
There was a problem hiding this comment.
While probably out of scope of this UI, there is also a display_override property: https://developer.chrome.com/articles/display-override/
Fixes: #970