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

Update spec generated from amphtml to file revision 642 (commit 3953b25) #1172

Merged
merged 5 commits into from Jun 13, 2018

Conversation

Projects
None yet
2 participants
@westonruter
Member

westonruter commented May 24, 2018

Major changes include:

  • Add amp-geo
  • Add amp-addthis
  • Replace amp-document-recommendations with amp-next-page
  • Add amp-story-bookend
  • Add amp-story-consent
  • Remove foreignobject
  • Add input[type=file]

Todo

  • Add tests for some of the new elements.
  • Verify that input[type=file] is working (it doesn't seem to be right now).
  • Figure out why amp-carousel is complaining about the layout/width/height attributes.

Fixes #1152.

@westonruter westonruter added this to the v0.7.2 milestone May 24, 2018

@westonruter westonruter referenced this pull request May 24, 2018

Closed

Support for amp-geo #1152

@westonruter westonruter changed the base branch from develop to 0.7 May 24, 2018

@westonruter westonruter force-pushed the update/spec branch from 9235534 to 8699090 May 24, 2018

@westonruter westonruter changed the title from Update spec generated from amphtml to file revision 642 (commit 3953b25) to [WIP] Update spec generated from amphtml to file revision 642 (commit 3953b25) May 25, 2018

westonruter added some commits May 24, 2018

Fix handling of mandatory ancestor checks for spec names including at…
…tributes

Fixes issue with input[type=file] not being valid

@westonruter westonruter force-pushed the update/spec branch from 8699090 to 9910967 Jun 12, 2018

@westonruter westonruter changed the base branch from 0.7 to develop Jun 12, 2018

@westonruter westonruter changed the title from [WIP] Update spec generated from amphtml to file revision 642 (commit 3953b25) to Update spec generated from amphtml to file revision 642 (commit 3953b25) Jun 12, 2018

@westonruter westonruter requested a review from amedina Jun 12, 2018

@westonruter westonruter modified the milestones: v0.7.2, v1.0 Jun 12, 2018

@westonruter westonruter requested a review from kienstra Jun 12, 2018

@kienstra

Approved

Hi @westonruter,
Thanks, this PR looks good. The updated spec seems to work well, having tested this PR with Native AMP child themes of Twenty Sixteen and Twenty Seventeen.

Also, the input[type=file] displayed well inside a <form>:

<form method="post">
    <input name="test-file" type="file">Upload</input>
</form>
*/
private function parse_tag_and_attributes_from_spec_name( $spec_name ) {
static $parsed_specs = array();
if ( isset( $parsed_specs[ $spec_name ] ) ) {

This comment has been minimized.

@kienstra

kienstra Jun 12, 2018

Collaborator

Nice use of static to prevent re-parsing the spec.

true === $attr_value ? $node->hasAttribute( $attr_name ) : strtolower( $node->getAttribute( $attr_name ) ) === $attr_value
);
if ( ! $match ) {
continue 2;

This comment has been minimized.

@kienstra

kienstra Jun 12, 2018

Collaborator

Good use of continue 2, I didn't know about that. It looks like that skips to the next iteration in the while loop outside of this foreach loop.

@westonruter westonruter merged commit 764236d into develop Jun 13, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the update/spec branch Jun 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment