AMP preview button looks off in WP 5.4-RC1

[westonruter]

Bug Description

The AMP preview button (introduced in #3323) which is available in Transitional and Reader modes, appears a bit off:

Hover state:

Expected Behaviour

It should look rather like:

Steps to reproduce

1. Activate Transitional or Reader mode.
2. Edit a post.
3. Look at Preview button and hover over it.

---

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]

cc @kienstra

[kienstra]

Hi @westonruter,
Thanks for letting me know about this. Yeah, the AMP button is unfortunately pretty susceptible to changes.

I can work on this if you'd like, but maybe not until Tuesday.

[westonruter]

Note that Gutenberg now has a mobile device preview dropdown, and the pointer is incorrectly placed under the AMP button:

However, when opening the preview dropdown there is the “preview externally” item:

Can we remove the top-level AMP preview link in favor of adding a new “Preview AMP externally” dropdown item?

[kienstra]

Good question. I'll look at this.

[kienstra]

It'd be great if there was a way to add a 'Preview AMP' link under the 'Preview externally' link shown in #4368 (comment).

But it doesn't look like there's a good way to do that, like with a React Slot/Fill.

The only way to add it is how the AMP link is currently added, which is brittle:

amp-wp/assets/src/block-editor/helpers/index.js

Lines 932 to 935 in d077842

	render(
	<PreviewComponent />,
	buttonWrapper,
	);

[kienstra]

Question About Button Screenshots

Hi @westonruter,
Good to see you this morning.

What do you think about these 3 button scenarios? #3 isn't very good.

I had a hard time styling it right for all of the combinations of Core and Gutenberg active/inactive. Though somebody that's better than me at CSS might be able to do this.

1. WP 5.4-RC2, Gutenberg plugin inactive

2. WP 5.4-RC2, Gutenberg plugin active

3. WP 5.3.2, Gutenberg plugin inactive

An alternative, like you mentioned, would be to simply add an AMP preview link to the document <InspectorControls> (though this screenshot doesn't have an AMP preview link):

For reference, here's the diff for those 3 screenshots above:

diff --git a/assets/css/src/amp-block-editor.css b/assets/css/src/amp-block-editor.css
index c25372f0f..7a0b091a9 100644
--- a/assets/css/src/amp-block-editor.css
+++ b/assets/css/src/amp-block-editor.css
@@ -6,15 +6,15 @@
 /* AMP preview button wrapper */
 .wp-core-ui #amp-wrapper-post-preview {
        margin-left: -6px;
+       margin-right: 8px;
 }
 
 /* AMP preview button */
 .wp-core-ui .amp-editor-post-preview {
-       height: 34px;
-       padding-left: 6px;
-       padding-right: 6px;
+       padding: 6px 12px;
        border-top-left-radius: 0;
        border-bottom-left-radius: 0;
+       height: 34px;
 }
 
 .wp-core-ui .amp-editor-post-preview svg {
diff --git a/assets/src/block-editor/components/amp-preview.js b/assets/src/block-editor/components/amp-preview.js
index e8dfe1587..aa3e0f44b 100644
--- a/assets/src/block-editor/components/amp-preview.js
+++ b/assets/src/block-editor/components/amp-preview.js
@@ -8,7 +8,7 @@ import PropTypes from 'prop-types';
  * WordPress dependencies
  */
 import { Component, createRef, renderToString } from '@wordpress/element';
-import { Icon, IconButton } from '@wordpress/components';
+import { Button, Icon } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 import { withSelect, withDispatch } from '@wordpress/data';
 import { DotTip } from '@wordpress/nux';
@@ -237,27 +237,21 @@ class AMPPreview extends Component {
 
                return (
                        isEnabled && ! errorMessages.length && ! isStandardMode && (
-                               <IconButton
-                                       icon={ ampFilledIcon( { viewBox: '0 0 62 62' } ) }
-                                       isLarge
-                                       className="amp-editor-post-preview"
-                                       href={ href }
-                                       label={ __( 'Preview AMP', 'amp' ) }
-                                       target={ this.getWindowTarget() }
-                                       disabled={ ! isSaveable }
+                               <Button
                                        onClick={ this.openPreviewWindow }
+                                       className="amp-editor-post-preview components-button is-secondary"
+                                       disabled={ ! isSaveable }
                                        ref={ this.buttonRef }
+                                       label={ __( 'Preview AMP', 'amp' ) }
                                >
+                                       { ampFilledIcon( { viewBox: '0 0 62 62', width: 18, height: 18 } ) }
                                        <span className="screen-reader-text">
                                                {
                                                        /* translators: accessibility text */
                                                        __( '(opens in a new tab)', 'amp' )
                                                }
                                        </span>
-                                       <DotTip tipId="amp/editor.preview">
-                                               { __( 'Click “Preview” to load a preview of this page in AMP, so you can make sure you are happy with your blocks.', 'amp' ) }
-                                       </DotTip>
-                               </IconButton>
+                               </Button>
                        )
                );
        }

Thanks, Weston!

[kienstra]

Also, that diff removes the <DotTip>, as it caused a console warning from a <button> being a child of a <button>.

[westonruter]

@swissspidy What do you think of this approach?

[westonruter]

@kienstra It looks good to me, but I am not an authority on the Gutenberg components. I'd say open a PR and ask perhaps @pierlon to review.

[kienstra]

Sounds good, thanks. Just opened a PR.

[kienstra]

Testing Instructions

1. Ensure Paired mode is selected
2. Go to a post editor
3. Expected: the AMP Preview button is visible
4. Click the button
5. Expected: Like before, it opens another tab with a preview of the front-end in AMP