Hide AMP Layout control if block does not have specified setting #5573
Conversation
|
Plugin builds for 4ce49d8 are ready 🛎️!
|
pierlon
changed the title
| * @param {Object} props Props. | ||
| * | ||
| * @return {ReactElement} Element. | ||
| */ | ||
| const AmpLayoutControl = ( props ) => { | ||
| const { name, attributes: { ampLayout }, setAttributes } = props; | ||
|
|
||
| if ( ! ampLayout ) { |
There was a problem hiding this comment.
See my comment here https://github.com/ampproject/amp-wp/pull/5574/files#r518529646. We probably want this check to be for undefined instead of false.
There was a problem hiding this comment.
Good catch. Addressed in 5993490.
pierlon
force-pushed
the
enhancement/4554-deprecate-amp-layout-settings
branch
from
762af24
to
5993490
Compare
Codecov Report
@@ Coverage Diff @@
## develop #5573 +/- ##
=============================================
+ Coverage 73.41% 73.62% +0.20%
- Complexity 5384 5424 +40
=============================================
Files 187 187
Lines 16236 16364 +128
=============================================
+ Hits 11920 12048 +128
Misses 4316 4316
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
This tests well. However, there's one additional change that would be worthwhile: could you add a warning notice to precede the AMP Layout setting that says:
The AMP Layout setting is deprecated and is slated for removal. Please report if you need it.
westonruter
deleted the
enhancement/4554-deprecate-amp-layout-settings
branch
| const { name, attributes: { ampLayout }, setAttributes } = props; | ||
|
|
||
| if ( undefined === ampLayout ) { |
There was a problem hiding this comment.
Per https://wordpress.org/support/topic/seeing-a-the-amp-layout-setting-is-deprecated-on-images/ this may need to be changed to:
if ( undefined === ampLayout || '' === ampLayout ) {
Summary
Fixes #4554.
When the control is displayed, a deprecated warning notice is shown:
Checklist