Skip to content

🐛Add date-template and info-template to amp-date-picker validation.#15106

Merged
cvializ merged 2 commits intoampproject:masterfrom
cvializ:validator/date-picker
May 7, 2018
Merged

🐛Add date-template and info-template to amp-date-picker validation.#15106
cvializ merged 2 commits intoampproject:masterfrom
cvializ:validator/date-picker

Conversation

@cvializ
Copy link
Copy Markdown
Contributor

@cvializ cvializ commented May 4, 2018

These were missing, making the date picker example on amp-by-example invalid.

@cvializ cvializ requested review from aghassemi and cathyxz May 4, 2018 22:56
@cvializ cvializ changed the title 🐛Add date-template and info-tempalte to amp-date-picker validation. 🐛Add date-template and info-template to amp-date-picker validation. May 4, 2018
}
}

# HTML5, 4.11.3 The template element
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template tag is defined in the amp-mustache extension as well. Please add a comment here noting that and also there noting these are defined here. See https://github.com/ampproject/amphtml/blob/master/extensions/amp-mustache/validator-amp-mustache.protoascii

spec_name: "amp-date-picker > template [date-template] [dates]"
mandatory_parent: "AMP-DATE-PICKER"
requires_extension: "amp-mustache"
attrs: {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please alphabetize by attribute name (here and below).

}
}

# HTML5, 4.11.3 The template element
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to repeat this comment or have blank lines between tags: {}

@cvializ
Copy link
Copy Markdown
Contributor Author

cvializ commented May 7, 2018

Thanks for the review! PTAL

Copy link
Copy Markdown
Contributor

@honeybadgerdontcare honeybadgerdontcare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thx!

@cvializ cvializ merged commit 6f97c3c into ampproject:master May 7, 2018
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request May 10, 2018
…mpproject#15106)

* Add date-template and info-tempalte to amp-date-picker validation.

* Cleanup validator protoascii with review feedback
honeybadgerdontcare added a commit that referenced this pull request May 10, 2018
* Revision bump for #14969

* Revision bump for #14937

* Simplify the script tag error.

* Whitelist pro.fontawesome.com in the AMP font list.

* Error category remapping

* Revision bump for #15106

* Revision bump for #15129

* Revision bump for #15155/15164/15191/15187
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants