Skip to content

Default to the homepage for the AMP Customizer menu item #3715

Merged
westonruter merged 1 commit intodevelopfrom
fix/3684-performance-issue
Nov 12, 2019
Merged

Default to the homepage for the AMP Customizer menu item #3715
westonruter merged 1 commit intodevelopfrom
fix/3684-performance-issue

Conversation

@pierlon
Copy link
Copy Markdown
Contributor

@pierlon pierlon commented Nov 11, 2019

Summary

Let the AMP Customizer menu item default to the homepage, instead of the latest AMP compatible post.

Before

After

Fixes #3684

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@googlebot googlebot added the cla: yes Signed the Google CLA label Nov 11, 2019
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.

Why are the links to navigate to the first AMP-compatible post being removed inside the Customizer? I thought these were going to be left intact. The performance problem in the admin is only due to setting the initial URL to be opened once you enter the Customizer. But once in the Customizer, there still should be a facility to easily access AMP in the preview via the notice. I believe f8aad23 should be reverted.

@westonruter westonruter added this to the v1.4.1 milestone Nov 12, 2019
@pierlon
Copy link
Copy Markdown
Contributor Author

pierlon commented Nov 12, 2019

Yes, you are right. My initial investigation led me to believe that the latest AMP post URL was being fetched on each page navigation within the Customizer as well, but that is not so. Reverting f8aad23 as requested.

@pierlon pierlon force-pushed the fix/3684-performance-issue branch from f8aad23 to 01e7aca Compare November 12, 2019 05:08
@pierlon pierlon changed the title Remove instances of fetching the latest AMP compatible post in Customizer Default to the homepage for the AMP Customizer menu item Nov 12, 2019
@westonruter westonruter merged commit 79e8949 into develop Nov 12, 2019
@westonruter westonruter deleted the fix/3684-performance-issue branch November 12, 2019 18:25
westonruter pushed a commit that referenced this pull request Nov 12, 2019
westonruter added a commit that referenced this pull request Nov 13, 2019
…ve-duplicate-amp-scripts

* 'develop' of github.com:ampproject/amp-wp: (66 commits)
  Improve display of validation errors for scripts (#3722)
  Conditionally run E2E tests (#3723)
  Tidy up validation error details (#3721)
  Add missing space after sentence (#3720)
  Default to the homepage instead of fetching the first AMP compatible post to customize (#3715)
  Include text content of style element in validation error (#3717)
  Fix summarizing error sources both parent theme and child theme (#3709)
  Exclude WordPress.PHP.DisallowShortTernary phpcs sniff
  Fix phpcs issues with date() and current_time()
  Exclude Generic.Arrays.DisallowShortArraySyntax from WordPress-Core
  Update dependency wp-coding-standards/wpcs to v2.2.0
  Improve specificity of JS doc
  Fix identifying sources for validation errors coming child themes (#3708)
  Fix failing E2E tests (#3707)
  Remove amp_validate query var from Validated URL 'View' row action
  Re-factor get_html_attribute_pattern as match_element_attributes
  Quote variables added to regex pattern
  Replace incorrect usage of esc_url() with esc_url_raw()
  Remove empty alt attributes
  Add object-fit=contain to amp-youtube placeholder image
  ...
westonruter added a commit that referenced this pull request Nov 14, 2019
* tag '1.4.1': (26 commits)
  Bump 1.4.1
  Update screenshots for 1.4.1
  Fix expected image name after upstream change (#3749)
  Use length property instead of count() method on DOMNodeList (#3727)
  Improve display of validation errors for scripts (#3722)
  Conditionally run E2E tests (#3723)
  Tidy up validation error details (#3721)
  Bump 1.4.1-RC1
  Default to the homepage instead of fetching the first AMP compatible post to customize (#3715)
  Add missing space after sentence (#3720)
  Include text content of style element in validation error (#3717)
  Use bitwise operator.
  Check if element is not in top toolbar.
  Fix user select for meta date and author
  Allow right click for meta blocks
  Fix summarizing error sources both parent theme and child theme (#3709)
  Fix identifying sources for validation errors coming child themes (#3708)
  Fix failing E2E tests (#3707)
  Remove amp_validate query var from Validated URL 'View' row action (#3706)
  Escape instances of unescapeed output in AMP settings screen code (#3703)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Signed the Google CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

amp_admin_get_preview_permalink throws query everywhere in Dashboard

3 participants