Amp-wp: Warning messages in console for templates

Created on 22 Apr 2019  路  9Comments  路  Source: ampproject/amp-wp

The templates inserter includes buttons within buttons at this moment which is forbidden.

Ideally, the BlockEdit would not include any buttons in the preview since they're not functional anyway. Alternatively, we should replace the button being the wrapper for preview.

Screen Shot 2019-04-22 at 3 07 27 PM

AMP Stories (obsolete) Bug

Most helpful comment

For reference: the wrapper button around the preview is here: https://github.com/ampproject/amp-wp/blob/amp-stories-redux/assets/src/components/template-inserter/index.js#L108

@kienstra might work on this ticket, @swissspidy was there any additional relevant information that would be useful here? I remember that you mentioned something related to this issue, perhaps you were planning to contribute a fix via Gutenberg to remove buttons from preview? Wondering how this would work for a case if there is a button block within in the preview.

Alternatively, for now, we could also replace the wrapper button in TemplateInserter.

All 9 comments

For reference: the wrapper button around the preview is here: https://github.com/ampproject/amp-wp/blob/amp-stories-redux/assets/src/components/template-inserter/index.js#L108

@kienstra might work on this ticket, @swissspidy was there any additional relevant information that would be useful here? I remember that you mentioned something related to this issue, perhaps you were planning to contribute a fix via Gutenberg to remove buttons from preview? Wondering how this would work for a case if there is a button block within in the preview.

Alternatively, for now, we could also replace the wrapper button in TemplateInserter.

Can any addition details be provided re: how to test? Not sure what I'm looking for here

@csossi Yes of course. So, before this change, when clicking on the template inserter in the editor, a bunch of errors would be shown in the console, similar to the one shown in the initial screenshot here. But now, these errors should not happen anymore. If you could verify that, that would be great.

Adding a template produces these "violation" messages, circled here. This expected?:
image

Note that the red error messages in this image happen on page load when adding a new story

Haven't reset the templates or anything yet, but here's what I currently get:

Screenshot 2019-05-14 at 12 35 35

The text block currently uses styles.display = 'flex';. I don't know why. This could be moved to the CSS file instead, makes it much simpler.

Second, there's a font size, even though the text block in the error message uses amp-fit-text, so the font size is redundant. In #2296 I've removed that part already.

Last but not least, there's the empty class="" attributes on the (wrong?) element. Maybe filterBlockAttributes needs to be changed here?

@swissspidy for some reason I'm not getting any errors locally.

Could you reset the templates and try then?

Moving this to Ready for Review since the validation errors seem to be not appearing after templates' reset. @swissspidy Feel free to close the ticket if that's the same for you, too.

Verified in https://github.com/ampproject/amp-wp/issues/2010#issuecomment-492776777 and also locally here 馃憤

Was this page helpful?
0 / 5 - 0 ratings

Related issues

swissspidy picture swissspidy  路  5Comments

miina picture miina  路  5Comments

westonruter picture westonruter  路  5Comments

miina picture miina  路  5Comments

miina picture miina  路  4Comments