Skip to content

Hide tooltips that should be hidden#2988

Merged
swissspidy merged 1 commit intodevelopfrom
fix/tooltip-css
Aug 12, 2019
Merged

Hide tooltips that should be hidden#2988
swissspidy merged 1 commit intodevelopfrom
fix/tooltip-css

Conversation

@swissspidy
Copy link
Copy Markdown
Collaborator

Hides tooltip containers that don't seem to be actually used themselves. Only their content is extracted in amp-validation-tooltips.js.

Before:

Screenshot 2019-08-08 at 16 17 55

After:

Screenshot 2019-08-08 at 16 26 45

@swissspidy swissspidy added the Bug Something isn't working label Aug 8, 2019
@swissspidy swissspidy added this to the v1.2.1 milestone Aug 8, 2019
@swissspidy swissspidy requested a review from westonruter August 8, 2019 14:28
@googlebot googlebot added the cla: yes Signed the Google CLA label Aug 8, 2019
@westonruter
Copy link
Copy Markdown
Member

Where are you seeing these containers being shown? I don't see them at all on any of the compatibility tool admin screens.

@swissspidy
Copy link
Copy Markdown
Collaborator Author

On the Error Index screen, the Validated URLs screen, and on the Validated URLs details screen.

Example:

Screenshot 2019-08-09 at 11 29 46

Happens on my personal site as well as locally on my AMP plugin dev environment.

Copy link
Copy Markdown
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Humm. I don't see it on my sites. Perhaps it's due to a browser extension you have? In any case, if this PR fixes the problem for you and the tooltips still appear when clicked, then I don't see any problem with merging this.

@swissspidy
Copy link
Copy Markdown
Collaborator Author

Indeed seems to come from an injected stylesheet from an extension. I think it makes sense to explicitly hide the tooltip element to avoid such a conflict.

@swissspidy swissspidy merged commit 800560a into develop Aug 12, 2019
@swissspidy swissspidy deleted the fix/tooltip-css branch August 12, 2019 08:49
westonruter added a commit that referenced this pull request Aug 12, 2019
…p-bind-syntax

* 'develop' of github.com:ampproject/amp-wp:
  RTLCSS all the things (#2977)
  Fix AMP Story editor compatibility with code editor (#3007)
  Update dependency core-js to v3.2.1 (#3011)
  Update amphtml validator spec to v1907301630320 (#3003)
  Improve handling of unlisted Vimeo videos (#2986)
  Always hide AMP admin menu item and compatibility tool menu ite… (#3005)
  Update dependency dom-scroll-into-view to v2.0.1 (#3008)
  Hide tooltips that should be hidden (#2988)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working cla: yes Signed the Google CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants