Preview AMP tooltip displays incorrectly in editor

[ernee]

Bug Description

In the editor, the tooltip to Preview AMP displays incorrectly (hidden behind another element).

Expected Behaviour

The tooltip should be fully visible.

Steps to reproduce

1. Edit a post or page (using WordPress Core or Gutenberg plugin)
2. Hover over the AMP preview icon at the top right
3. See the Preview AMP tool tip display incorrectly (hidden)

Screenshots

WordPress Core

Gutenberg plugin

Additional context

• WordPress version: 5.4
• Plugin version: 1.5.3
• Gutenberg plugin version (if applicable): 7.9.0
• AMP plugin template mode: Transitional, Reader
• PHP version: 7.3.5
• OS: MacOS
• Browser: Chrome
• Device: MacBook Air

---

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

[kienstra]

Hi @ernee!
Hope you've been doing great!

Yeah, this is definitely an issue. In my local, the tooltip is cut off below:

I'll work on this.

[kienstra]

If it's alright, I'll come back to this tomorrow.

There doesn't look to be a quick fix, though I'm not good at CSS.

For example, simply changing the z-index or position of the <Popover> in the <Tooltip> didn't fix this.

[westonruter]

It appears the Tooltip component can't be nested inside that button in the DOM. No amount of z-index will help apparently. The element containing the tooltip that appears for the Settings icon is located elsewhere in the DOM somehow.

[kienstra]

Yeah, there might not be a way to keep the tooltip.

[westonruter]

I'd look into how the Setting button has integrated the tooltip in Gutenberg.

[kienstra]

1. Tooltip might not be feasible

Hi @westonruter,
Sorry for the delay.

I couldn't find a way to prevent the <Tooltip> from being cut off. The problem looks to be that tooltips need to be further away in the DOM.

But this button is rendered into a single element in the DOM, and that element isn't aware of the block editor's React components. It's rendered like this because there wasn't a Slot/Fill or filter to render it properly.

So the tooltip renders inside the Button, and looks bad.

For example, with the 'Preview AMP' button, the tooltip is inside the button:

But for the 'More tools & options' button, which looks good, the tooltip is pretty far away from the button in the DOM:

[kienstra]

2. Suggestion

What do you think about simply removing the <Tooltip>?

diff --git a/assets/src/block-editor/components/amp-preview.js b/assets/src/block-editor/components/amp-preview.js
index dfbace46e..ed8a4fa1e 100644
--- a/assets/src/block-editor/components/amp-preview.js
+++ b/assets/src/block-editor/components/amp-preview.js
@@ -236,25 +236,22 @@ class AMPPreview extends Component {
 
                return (
                        isEnabled && ! errorMessages.length && ! isStandardMode && (
-                               <Tooltip text={ __( 'Preview AMP', 'amp' ) } >
-                                       <Button
-                                               className="amp-editor-post-preview"
-                                               href={ href }
-                                               label={ __( 'Preview AMP', 'amp' ) }
-                                               isSecondary
-                                               disabled={ ! isSaveable }
-                                               onClick={ this.openPreviewWindow }
-                                               ref={ this.buttonRef }
-                                       >
-                                               { 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>
-                                       </Button>
-                               </Tooltip>
+                               <Button
+                                       className="amp-editor-post-preview"
+                                       href={ href }
+                                       isSecondary
+                                       disabled={ ! isSaveable }
+                                       onClick={ this.openPreviewWindow }
+                                       ref={ this.buttonRef }
+                               >
+                                       { 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>
+                               </Button>
                        )
                );
        }

That's probably not great for usability.

But there's screen reader text inside the Button.

And the documentation for Button says:

All button types use text labels to describe the action that happens when a user taps a button. If there’s no text label, there should be an icon to signify what the button does (e.g. an IconButton component).

...and this does have an icon.

[westonruter]

There's a difference between the Settings button and the AMP Preview button in terms of the React components.

With React Dev Tools this is what the Settings tooltip looks like when it is open:

Compare with the AMP Preview button:

We're missing a Fill construct here.

Nevertheless, the “Settings“ button doesn't show a Tooltip being used here (source):

				<Button
					icon={ cog }
					label={ __( 'Settings' ) }
					onClick={ toggleGeneralSidebar }
					isPressed={ isEditorSidebarOpened }
					aria-expanded={ isEditorSidebarOpened }
					shortcut={ shortcut }
				/>

I'm not sure how the tooltip is being wired up for the Settings element, but it's being done differently than the AMP Preview button. I suspect that changing the tooltip to be added in the same way will fix the issue with the positioning.

[kienstra]

Hi Weston,
Thanks for your ideas. I tested something like you suggested, but I'll try it again.

The 'Settings' button has a Popover via its label prop. It renders a Tooltip using that label prop passed to it.

Then, the <Tooltip> renders <Fill>. I don't think there's a Slot that the fill can use, because this 'Preview AMP' button is rendered into a single DOM element, and doesn't have access to any of the block editor.

[kienstra]

Here's something pretty similar to the Settings button, and the tooltip is still covered.

diff --git a/assets/src/block-editor/components/amp-preview.js b/assets/src/block-editor/components/amp-preview.js
index dfbace46e..3b28af0e0 100644
--- a/assets/src/block-editor/components/amp-preview.js
+++ b/assets/src/block-editor/components/amp-preview.js
@@ -236,25 +236,17 @@ class AMPPreview extends Component {
 
                return (
                        isEnabled && ! errorMessages.length && ! isStandardMode && (
-                               <Tooltip text={ __( 'Preview AMP', 'amp' ) } >
-                                       <Button
-                                               className="amp-editor-post-preview"
-                                               href={ href }
-                                               label={ __( 'Preview AMP', 'amp' ) }
-                                               isSecondary
-                                               disabled={ ! isSaveable }
-                                               onClick={ this.openPreviewWindow }
-                                               ref={ this.buttonRef }
-                                       >
-                                               { 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>
-                                       </Button>
-                               </Tooltip>
+                               <Button
+                                       icon={ ampFilledIcon( { viewBox: '0 0 62 62', width: 18, height: 18 } ) }
+                                       label={ __( 'Preview AMP', 'amp' ) }
+                                       className="amp-editor-post-preview"
+                                       href={ href }
+                                       label={ __( 'Preview AMP', 'amp' ) }
+                                       isSecondary
+                                       disabled={ ! isSaveable }
+                                       onClick={ this.openPreviewWindow }
+                                       ref={ this.buttonRef }
+                               />
                        )
                );
        }

[westonruter]

OK, how about just using a classic title attribute instead?

[kienstra]

Good idea, that works locally.

[amedina]

The tooltip is showing properly, but the styling of the buttons changed. The way they were styled before looked better (see screenshots at the beginning of this issue; and see attachments in this comment). Now the Preview and the AMP Preview look disconnected.

[westonruter]

@amedina this is expected. The button is required to be disconnected in current versions of Gutenberg because the Preview dropdown component is not extendable.

[amedina]

I see. It looks weird disconnected because the semantics of the AMP symbol is not conveyed as clearly.

Let's ship it and we will revise during UX review.