amp-wp icon indicating copy to clipboard operation
amp-wp copied to clipboard

Show link to exit mobile version on AMP pages even when mobile redirection is not enabled

Open westonruter opened this issue 4 years ago • 3 comments

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

westonruter avatar Aug 28 '20 17:08 westonruter

I milestoned this for 2.1 but we can do it in a patch release as well.

westonruter avatar Aug 28 '20 17:08 westonruter

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?

westonruter avatar Sep 07 '20 15:09 westonruter

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 avatar Sep 15 '20 20:09 westonruter

@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.

thelovekesh avatar Jan 24 '23 19:01 thelovekesh

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 avatar Jan 24 '23 20:01 westonruter

@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?

thelovekesh avatar Jan 24 '23 21:01 thelovekesh

There are three links involved:

  1. The link added to the head on non-AMP pages to indicate where the mobile (AMP) version is.
  2. The hyperlink (a) added to the footer on AMP pages to allow the user to go back to the non-AMP version.
  3. 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.

westonruter avatar Jan 24 '23 21:01 westonruter

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

pavanpatil1 avatar Feb 06 '23 02:02 pavanpatil1

@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 avatar Feb 06 '23 16:02 thelovekesh

@thelovekesh Good catch. Please make it so.

westonruter avatar Feb 06 '23 18:02 westonruter

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

thelovekesh avatar Feb 07 '23 06:02 thelovekesh