Amp-wp: The amp-lightbox-gallery component script is not enqueued when lightbox attribute is present on images

[vralle]

Hi!
amp-wp v1.0.1
wp v5.0.3

Theme setup:

add_theme_support(
    'amp',
    array(
        'paired' => true,
        'template_dir' => 'templates-amp',
        'templates_supported' => 'all',
    )
);

I'm trying to load the amp-lightbox-gallery component:

add_action(
    'amp_post_template_data',
    function ($data) {
        // Not work:
        // $data['amp_component_scripts']['amp-lightbox-gallery'] = true;
        // Not Work:
        array_merge(
            $data['amp_component_scripts'],
            array(
                'amp-lightbox-gallery' => 'https://cdn.ampproject.org/v0/amp-lightbox-gallery-0.1.js'
            )
        );

        return $data;
    }
);

But nothing happens. I can not understand what I'm doing wrong.

The example above should be enough to load an additional component or additional actions are needed?

Search by project has not brought results.

add_action(
    'wp_head',
    function () {
        echo '<script async custom-element="amp-lightbox-gallery" src="https://cdn.ampproject.org/v0/amp-lightbox-gallery-0.1.js"></script>';
    }
);

This code works, but I'm not sure that this way is correct...

[westonruter]

You should not add the AMP scripts yourself. The plugin should do this form you. All you should have to do is use the AMP components in the page and the plugin will add the required scripts automatically.

[vralle]

Yes, it happens to the components. But in the case of the amp-lightbox-gallery, it is not clear which scenario will trigger the component.

The current task was to make the gallery shortcode output like basic amp-lightbox-gallery example.

The Gallery Shortcode output is very simple:

...
$output = '<div id="gallery-id-generated" class="gallery>';
foreach ($attachments as $id => $attachment) {
       ...
        $attr['lightbox'] = '';
        $output .= '<figure class="gallery__figure figure">';
        $output .= wp_get_attachment_image($id, $atts['size'], false, $attr);
        $output .= '</figure>';
}
$output .= '</div>';

The example will not call amp-lightbox-gallery. What actions need to be taken?

[westonruter]

Could you please share complete functional code to make it easier to test? Something that can be dropped into a plugin would be ideal.

[vralle]

https://gist.github.com/vralle/e8111a4a3322bd0ebcb0a40e10db62bb

[vralle]

I could not figure it out, I had a few questions:

1. Sorry, no documentation was found to manage the gallery images.
2. AMP_Gallery_Embed_Handler->maybe_override_gallery() manipulate amp-lightbox and amp-carousel attributes. It is not clear what it does, if amp-lightbox=true and amp-carousel=false.
3. AMP_Gallery_Embed_Handler->shortcode() manipulate amp-lightbox attribute at the input.
   Attribute is transformed into lightbox after shortcode_atts() . This makes it somewhat confusing to use attributes in shortcodes (amp-lightbox) and when using the filter shortcode_atts_gallery filter (now this lightbox).
4. Nothing was found that automatically enqueue the amp-lightbox-gallery component. I have not found the right scenario. I'm not even sure that it exists.

[westonruter]

@vralle you're right. It's not automatically including the amp-lightbox-gallery script as I expected. Here is what you can do, given your above gist. Add the following code, after the is_feed() check you have:

    if ( vralle_by_is_amp() ) {
        wp_enqueue_script( 'amp-lightbox-gallery' );
    }

In other words:

diff --git a/gallery.php b/gallery.php
index 296dd22..bc433b6 100644
--- a/gallery.php
+++ b/gallery.php
@@ -104,6 +104,10 @@ function vralle_by_gallery($attr)
         return $output;
     }

+    if ( vralle_by_is_amp() ) {
+        wp_enqueue_script( 'amp-lightbox-gallery' );
+    }
+
     $columns = intval($atts['columns']);
     $selector = "gallery-{$instance}";
     $size_class = sanitize_html_class($atts['size']);

This makes the lightbox behavior work as expected for me.

[westonruter]

The above code will work in native/paired mode. It will also ensure that you don't have more than one copy of the amp-lightbox-gallery script added to the page, which is currently a possibility with the wp_head action you have in the description.

For classic mode, you'd need to continue filtering amp_post_template_data as you did above.

Nevertheless, there seems to be a bug in the tag-and-attribute sanitizer where it is not detecting the need for this component in the following logic:

https://github.com/ampproject/amp-wp/blob/12a0627d18e5ea37abd1838b7161a21fc389913f/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php#L543-L566

This is likely because almost all component scripts are related to elements, not to attributes. That is why it is likely being missed.

This is also showing up incorrectly in unit tests. In the following example:

https://github.com/ampproject/amp-wp/blob/12a0627d18e5ea37abd1838b7161a21fc389913f/tests/test-tag-and-attribute-sanitizer.php#L237-L241

The array() should actually be array( 'amp-lightbox-gallery' ).

Nevertheless, manually enqueueing the script will is a good approach and it won't break when the plugin is fixed to enqueue the component automatically.

[westonruter]

@vralle Please give #1855 a try.

[vralle]

@westonruter, tested it. works. but...
A map of amp images output

The time limit does not allow to answer the question of how you can manage different scenarios for images output.