-
Notifications
You must be signed in to change notification settings - Fork 381
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
Rebrand Classic as Reader mode and Paired as Transitional mode; add Exit Reader Mode link #2034
Changes from 2 commits
3418a56
7d2c9ac
33a4cb4
d5cdd94
089fc62
09df6d8
982bf65
5f58a72
c269707
b360935
11ea84c
611f53c
5df747d
297e668
e35d629
a35e51f
c007f2f
cabdf2b
8026f7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -267,23 +267,25 @@ private function build_post_data() { | |
|
||
$this->add_data( | ||
array( | ||
'post' => $this->post, | ||
'post_id' => $this->ID, | ||
'post_title' => $post_title, | ||
'post_publish_timestamp' => $post_publish_timestamp, | ||
'post_modified_timestamp' => $post_modified_timestamp, | ||
'post_author' => $post_author, | ||
'post' => $this->post, | ||
'post_id' => $this->ID, | ||
'post_title' => $post_title, | ||
'post_publish_timestamp' => $post_publish_timestamp, | ||
'post_modified_timestamp' => $post_modified_timestamp, | ||
'post_author' => $post_author, | ||
'post_canonical_link_url' => get_permalink( $this->ID ), | ||
'post_canonical_link_text' => __( 'Exit Reader Mode', 'amp' ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, we could say "View Canonical". What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the average user knows what "Canonical" means in this context. Not to speak of translators. "Original" or "Full Version" would work better though
For comparison, Safari uses "Hide Reader View". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the point of "View Canonical", but it is true that is will be confusing for most users. OTOH, having "Exit Reader Mode" implies that the user is aware that she is in Reader Mode. Probably we can express the duality by a exit route with a message along these lines: "You are viewing the Reader Mode version of |
||
) | ||
); | ||
|
||
$this->build_post_featured_image(); | ||
$this->build_post_commments_data(); | ||
$this->build_post_comments_data(); | ||
} | ||
|
||
/** | ||
* Buuild post comments data. | ||
*/ | ||
private function build_post_commments_data() { | ||
private function build_post_comments_data() { | ||
if ( ! post_type_supports( $this->post->post_type, 'comments' ) ) { | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to discuss how to best phrase this here. Do we want to highlight that it's not respecting the theme's design? Should we maybe phrase this as "separate from your theme" to sound less negative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Display AMP responses in a simplified reader mode design separate from your theme
could workThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another alternative:
Generates AMP content using simplified templates, which are light but may not match the look-and-feel of your site