Skip to content
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

Forbid creating non-AMP "amp-*" elements #9283

Closed
jridgewell opened this issue May 11, 2017 · 12 comments
Closed

Forbid creating non-AMP "amp-*" elements #9283

jridgewell opened this issue May 11, 2017 · 12 comments

Comments

@jridgewell
Copy link
Contributor

Examples include:

These should be i-amphtml-*, the amp- tag prefix is reserved for AMP elements only. Anything else should use public classnames, if they are style-able by publishers.

@dvoytenko
Copy link
Contributor

Overall, great idea. I need to check - we possibly may need to exclude <amp-body> from this though. I think it's explicitly public, for PWA/SD polyfill only.

@adelinamart
Copy link
Contributor

@dvoytenko @zhouyx any updates here? Thanks.

@zhouyx
Copy link
Contributor

zhouyx commented Jun 7, 2017

Unfortunately I'll have to exclude <amp-sticky-ad-top-padding> as well. It is documented, renaming it will bring a breaking change.
I can however add a presubmit check to prevent future non AMP element with tagName 'amp-*'. How does that sound? Or any other ideas?

@jridgewell
Copy link
Contributor Author

Presubmit would be great. Let's deprecate <amp-sticky-ad-top-padding> (just remove documentation for the tag name) and add a public class instead.

@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @dvoytenko Do you have any updates?

@jridgewell jridgewell assigned jridgewell and unassigned dvoytenko and zhouyx Oct 20, 2017
@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @jridgewell Do you have any updates?

2 similar comments
@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @jridgewell Do you have any updates?

@ampprojectbot
Copy link
Member

This is a high priority issue but it hasn't been updated in awhile. @jridgewell Do you have any updates?

@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. @jridgewell Do you have any updates?

2 similar comments
@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. @jridgewell Do you have any updates?

@ampprojectbot
Copy link
Member

This issue hasn't been updated in awhile. @jridgewell Do you have any updates?

@lannka
Copy link
Contributor

lannka commented Jul 2, 2019

seems already done.

@lannka lannka closed this as completed Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants