Remove deprecated controls from editor sidebar and fix AMP Lightbox and AMP Carousel in Gallery block #6833
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
delawski
changed the title
ampLayout and ampNoLoading controls from editor sidebar
|
Plugin builds for 83f143c are ready 🛎️!
|
…r-amp-settings-panel * 'develop' of github.com:ampproject/amp-wp: Render value as fallback instead of entire parent source object Fix logic inversion for sources.length Ensure `sources` is non-empty array Revert "Ensure error `sources` is never `null`" Ensure error `sources` is never `null` Ensure `sources` is iterated over only if it is non-empty array Fix test to account for gutenberg_render_layout_support_flag() (#6850) Support converting YouTube iframes with 100% width to amp-youtube with fixed-height (#6837) Update Gutenberg package dependencies Update Gutenberg package dependencies Allow composer normalize plugin to run Normalize Composer file Update Composer to use AMP toolbox v0.10.0
…r-amp-settings-panel * 'develop' of github.com:ampproject/amp-wp: (48 commits) Update comments to note where related skip link logic is located Improve nested CSS indentation Use AMP prefix for styles injected by sanitizer Remove needless escaping in DOM context Simplify obtaining main element and body element Ensure Navigation block script is registered to fix tests Add missing test coverage for dequeue_block_navigation_view_script Use function exists check for render_block_core_navigation to determine if test should be skipped Add attribute value escaping for good measure Fix phpcs warnings Add more test cases for skip link Fix indentation Add assertions that no SQL queries run when data deletion is disabled Add test specifically for transient deletion Harden term deletion logic for WP<4.4 Add assertions for term deletion Flesh out tests specifically for post and term deletion Add test specific to delete_user_metadata Add test specifically for the delete_options function Remove deletion of non-option ...
delawski
commented
Jan 27, 2022
|
@westonruter Thanks for updating the gallery block sanitizer. I tested it on my local and now the AMP Carousel works as expected with the new Gallery block markup. |
westonruter
changed the title
|
I think this PR is now ready for a final review and merge. @dhaval-parekh – since both @westonruter and myself worked on this branch, can you do a code review? |
| @@ -7,6 +7,7 @@ | |||
|
|
|||
| use AmpProject\AmpWP\Embed\HandlesGalleryEmbed; | |||
| use AmpProject\Html\Tag; | |||
| use AmpProject\Dom\Element; | |||
There was a problem hiding this comment.
Nit, Not a blocker.
This could be above use AmpProject\Html\Tag;
There was a problem hiding this comment.
I'm not sure I understand. There is already use AmpProject\Html\Tag. The use of AmpProject\Dom\Element is due to this new line:
dhaval-parekh
approved these changes
Jan 28, 2022
…r-amp-settings-panel * 'develop' of github.com:ampproject/amp-wp: (37 commits) Revert "Use different selector in E2E test" Revert "Ensure AMP plugin is activated before running E2E tests" Ensure get_page_cache_detail return value is array Increase verbosity of hasScannableUrls check Ensure AMP plugin is activated before running E2E tests Use different selector in E2E test Improve strings and comments Remove redundant checks for header value being non-empty Refine comments Remove debug code for catching external requests Scope styles for dashicons Simplify logic in get_page_cache_detail Reuse threshold variable Remove erroneous P from wrapping UL Fix label when page caching detected and yet response still slow Add formatting for millisecond values Improve test coverage and simplify conditional Simplify translation string Speed up tests by using usleep() instead of sleep() Only show lack of page caching plugin if no caching response headers present ...
westonruter
reviewed
Jan 28, 2022
|
I tried creating a post with the various cases and there were no block deprecation issues. The settings were removed as expected:
|
|
In 5.8 I also had added 4 gallery blocks:
When viewing these 4 on the frontend in 5.9 before block migration, they all rendered as expected. And after I edited the post to migrate the gallery blocks to the nested format, they also rendered as expected.
|
westonruter
approved these changes
Jan 29, 2022
westonruter
added a commit
that referenced
this pull request
Jan 29, 2022
…nd AMP Carousel in Gallery block (#6833)
westonruter
added
the
Changelogged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6508
Fixed #6832
Related #4989
Fixes #6848
Here are the main changes that are introduced by this PR:
ampLayoutandampNoLoadingcontrols from the block editor sidebar (see: Empty AMP Settings panel for video block #6508 (comment)).Checklist