-
Notifications
You must be signed in to change notification settings - Fork 385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relocate AMP Preview Button #7914
Relocate AMP Preview Button #7914
Conversation
@@ -221,37 +220,36 @@ class AmpPreviewButton extends Component { | |||
isEnabled && | |||
!errorMessages.length && | |||
!isStandardMode && ( | |||
<Button | |||
<PluginPreviewMenuItem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to make sure that if PluginPreviewMenuItem
doesn't exist due to not being on the latest version of WordPress that we short-circuit this function so we don't get an error.
b3f30f6
to
67bf045
Compare
@@ -1,4 +1,5 @@ | |||
FROM wordpress | |||
# @todo: switch to wordpress:latest once it's available | |||
FROM wordpress:beta-6.7-php8.3-apache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required to test AMP preview button changes.
JS Unit tests are (not) failing because codecov is unable to upload reports of the PR from fork. |
export const name = 'amp-preview-menu-item'; | ||
|
||
export const icon = ( | ||
<AMPFilledIcon viewBox="0 0 62 62" height={18} width={18} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I make the icon have the 24x24 pixel dimensions then it isn't cropped. But then it's probably too big.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise @thelovekesh, The latest build doesn't have the cropping issue on any breakpoints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work well. The only outstanding item is the slight cropping on the icon. |
Summary
PluginPreviewMenuItem
.Checklist