Amp-wp: Support for landscape orientation

[swissspidy]

Eventually we need to add support for supports-landscape to support landscape orientation and a full bleed desktop experience. There have been user requests for this already too.

Many things need to be considered for landscape support:

• Assets handling (#2332)
• Landscape mode while editing
• Landscape preview for templates (?)
• Calculation of block position for portrait and landscape (that's gonna be fun)
• Poster image support

[westonruter]

• Calculation of block position for portrait and landscape (that's gonna be fun)
• Landscape preview for templates (?)

Since the positions are already percentage-based, would there need to be any change? Granted, if you were making a supports-landscape story then you'd have to be even less particular about exact placement of elements across responsive displays. In this way, desktop could just be a portrait display that is super wide 😄

• Landscape mode while editing

Being able to toggle between desktop and mobile aspect ratios is the key change here I think, if we don't maintain two separate layouts for each page.

• Poster image support

Poster images for the stories should be fine as-is, since there is already a STORY_LANDSCAPE_IMAGE_SIZE. But poster image for a video will be different. Perhaps it should just be the same as a background image, except without the focal point picker (and just centered in the viewport). This then raises the question of whether or not background video should get a focal point picker. Or should it be presumed that a suitable video should always be shot with the center being acceptable focal point?

[westonruter]

I created a test story:

And I modified the single_amp-story.php to add the attribute:

diff --git a/includes/templates/single-amp_story.php b/includes/templates/single-amp_story.php
index 8d015ee7..d25db1d8 100644
--- a/includes/templates/single-amp_story.php
+++ b/includes/templates/single-amp_story.php
@@ -39,6 +39,7 @@ the_post();
        $poster_landscape = wp_get_attachment_image_url( $thumbnail_id, AMP_Story_Post_Type::STORY_LANDSCAPE_IMAGE_SIZE );
        ?>
        <amp-story
+           supports-landscape
            standalone
            publisher-logo-src="<?php echo esc_url( $publisher_logo_src ); ?>"
            publisher="<?php echo esc_attr( $publisher ); ?>"

When viewing in DevTools emulating as Pixel 2XL it looks as expected:

And in desktop (1280x1024):

So it's actually pretty good by default. That's the beauty of being mindful of the fundamentally responsive nature of stories.

[westonruter]

As suggested by @jdelia in #2448, what if we just started out with a toggle for whether the story should have the supports-landscape attribute added? Maybe that would be premature and there should rather be a filter for whether the attribute is added? That would facilitate experimenting with landscape support now, and then we can explore the UI implications later for the editor.

[jdelia]

Thanks, @westonruter :) Perhaps if there was a setting in the plugin to enable supports-landscape globally and then a toggle in each story to override? In any event, a filter would be helpful immediately so that we do not have to modify the plugin.

[westonruter]

@swissspidy What do you think of this:

diff --git a/includes/templates/single-amp_story.php b/includes/templates/single-amp_story.php
index 8d015ee7..8f4600dd 100644
--- a/includes/templates/single-amp_story.php
+++ b/includes/templates/single-amp_story.php
@@ -40,6 +40,17 @@ the_post();
        ?>
        <amp-story
            standalone
+           <?php
+           /**
+            * Filters whether the story supports landscape.
+            *
+            * @param bool    $supports_landscape Whether supports landscape. Currently false by default, but this will change in the future (e.g. via user toggle).
+            * @param wp_Post $post               The current amp_story post object.
+            */
+           if ( apply_filters( 'amp_stories_supports_landscape', false, get_post() ) ) {
+               echo 'supports-landscape';
+           }
+           ?>
            publisher-logo-src="<?php echo esc_url( $publisher_logo_src ); ?>"
            publisher="<?php echo esc_attr( $publisher ); ?>"
            title="<?php the_title_attribute(); ?>"

A site could opt-in to landscape stories via just:

add_filter( 'amp_stories_supports_landscape', '__return_true' );

I hesitate a bit with the default value. It's a similar story for amp_skip_post. When that filter returns true then the “Enable AMP” toggle is disabled entirely (and conversely #2314). Similarly, when a post type does not have amp post type support, the toggle is disabled, and there is a amp_post_status_default_enabled to change the default state.

In the same way, if someone forcibly applies a filter for amp_stories_supports_landscape then should any such toggle be disabled (forced on or off)?

[swissspidy]

What do you think of this:

That works for me. It's a good first step.

if someone forcibly applies a filter for amp_stories_supports_landscape then should any such toggle be disabled (forced on or off)

While a user toggle would influence that filter's default value, I don't think the filter should necessarily influence the toggle.

But we can discuss the exact logic here at some other point in the future.

[westonruter]

@jdelia Opened PR #2468 which introduces an amp_story_supports_landscape filter.

[westonruter]

With #3003 the amp-story-grid-layer element supports the position attribute (being landscape-half-left or landscape-half-right).

[westonruter]

Here's a plugin that allows you to force a story to support landscape even though we don't yet directly support it in the UI: https://gist.github.com/westonruter/4e4ee2b5790c6218fab40a7b3f90331f