amp-wp
amp-wp copied to clipboard
Show link to exit mobile version on AMP pages even when mobile redirection is not enabled
Feature description
In versions of the plugin prior to 2.0, there was an “Exit Reader Mode” link that could be displayed in the header. We removed this in https://github.com/ampproject/amp-wp/issues/4573 because it conflicted with long site titles. We also deemed it redundant with the new Mobile Redirection functionality which shows such an exit link in the footer.
However, the exit mobile version only displays in the footer when Mobile Redirection is enabled. We should consider restoring the exit mobile version link when Mobile Redirection is disabled, especially when a site is in Reader mode. The link is probably not needed in Transitional mode because the AMP and non-AMP versions have much more parity. In Reader mode, however, the AMP version is intentionally streamlined so it makes sense that a user seeing the Reader version would always want to leave to go to the non-AMP version.
When mobile redirection is turned off, the link to return to the mobile version would not correspondingly be added to the non-AMP page.
This may require changing the Mobile Redirection section to instead be Paired Options, which could then incorporate the Paired URL Structure section.
More context: https://wordpress.org/support/topic/amp-2-0-exit-reader-mode/#post-13324157
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation brief
QA testing instructions
Demo
Changelog entry
I milestoned this for 2.1 but we can do it in a patch release as well.
Also apparently requested in support topic:
Also, if the reader mode is enabled I think we can give a link for the user with a button to redirect them to original page?
When mobile redirection is turned off, the link to return to the mobile version would not correspondingly be added to the non-AMP page.
Or maybe users should have the option to show the mobile switcher link even on non-AMP pages, but this would be less commonly needed than showing the non-AMP link on AMP pages.
@westonruter While working on #7288 we consider to fix this but due to some reason, we didn't move forward with it which is mentioned here - https://github.com/ampproject/amp-wp/pull/7288#discussion_r1035358414
If this is something we don't want to incorporate based on the above discussion I think we can close this ticket or change the milestone accordingly.
we didn't move forward with it which is mentioned here - #7288 (comment)
@thelovekesh I'm not sure that is the reason? I think all that is needed to close this out would be to add something like the following if
statement to MobileRedirection::register()
:
if ( ! $is_mobile_redirect_enabled && ! amp_is_canonical() ) {
add_action( 'template_redirect', function () {
if ( amp_is_request() ) {
$this->add_mobile_switcher_footer_hooks();
}
} );
}
@westonruter Sorry I missed https://github.com/ampproject/amp-wp/pull/7288#discussion_r1035348063 to include. Can you take a look once at this discussion as well?
There are three links involved:
- The
link
added to thehead
on non-AMP pages to indicate where the mobile (AMP) version is. - The hyperlink (
a
) added to the footer on AMP pages to allow the user to go back to the non-AMP version. - The hyperlink (
a
) added to the footer on non-AMP pages to allow the user to go back to the AMP version if they had previously clicked the non-AMP link on mobile (the previous point), which disables the automatic redirect to the AMP version. This link is only shown if using a mobile user agent, and this requires JS.
Point 1 is already implemented in #7288. Point 3 is what I think that comment is mostly about. What we need is point 2 which is to show the non-AMP link on AMP pages when mobile redirection is not enabled.
QA Passed ✅
Cross-checked the issue and the fix is working as expected. Now the exit mobile version link is visible even when the mobile redirection option is disable.
https://user-images.githubusercontent.com/44057535/216868716-8f8762cc-817d-4a1e-9323-554c8de645ec.mp4
@westonruter Just noticed that this fix also outputs the Exit mobile version
in transitional mode as well which should not be allowed. The logic here should be updated to:
https://github.com/ampproject/amp-wp/blob/68d9bb3f977967fa05fb09f5f52da561dc85bd72/src/MobileRedirection.php#L194-L199
public function maybe_add_mobile_switcher_link() {
- if ( amp_is_request() ) {
+ if ( amp_is_request() && AMP_Theme_Support::READER_MODE_SLUG === get_option( Option::THEME_SUPPORT ) ) {
$this->add_mobile_switcher_head_hooks();
$this->add_mobile_switcher_footer_hooks();
}
}
@thelovekesh Good catch. Please make it so.
QA Passed ✅
We are aiming to show Exit Mobile Version
link to the visitor in transitional mode as well. See: https://github.com/ampproject/amp-wp/pull/7453#issuecomment-1420233837