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

♻️ Change amp-story-interactive version to 0.1 #30080

Merged
merged 9 commits into from
Sep 1, 2020

Conversation

mszylkowski
Copy link
Contributor

The components should be in 0.1 instead of 1.0

@amp-owners-bot
Copy link

amp-owners-bot bot commented Sep 1, 2020

Hey @gmajoulet! These files were changed:

extensions/amp-story-interactive/0.1/amp-story-interactive-abstract.js
extensions/amp-story-interactive/0.1/amp-story-interactive-binary-poll.css
extensions/amp-story-interactive/0.1/amp-story-interactive-binary-poll.js
extensions/amp-story-interactive/0.1/amp-story-interactive-poll.css
extensions/amp-story-interactive/0.1/amp-story-interactive-poll.js
extensions/amp-story-interactive/0.1/amp-story-interactive-quiz.css
extensions/amp-story-interactive/0.1/amp-story-interactive-quiz.js
extensions/amp-story-interactive/0.1/amp-story-interactive-results.css
extensions/amp-story-interactive/0.1/amp-story-interactive-results.js
extensions/amp-story-interactive/0.1/amp-story-interactive.css
extensions/amp-story-interactive/0.1/amp-story-interactive.js
extensions/amp-story-interactive/0.1/interactive-confetti.js
+16 more

Hey @newmuis, @Enriqe! These files were changed:

extensions/amp-story/1.0/amp-story-store-service.js

examples/amp-story/interactive_size.html Outdated Show resolved Hide resolved
@gmajoulet
Copy link
Contributor

Actually, I can't find where you changed this line in this PR https://github.com/ampproject/amphtml/blob/master/extensions/amp-story-interactive/validator-amp-story-interactive.protoascii#L22

Do you have a validation success test somewhere? I think we can't get to a valid story with the current configuration?

Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Approved, agree with other comments though... lets remove the new file since it appears unrelated.

@kristoferbaxter
Copy link
Contributor

@gmajoulet Is there a test for the protoascii configuration?

@gmajoulet
Copy link
Contributor

I can only see tests for invalid stories, we should also add one valid story. Matias, could you add it? Thanks!

@mszylkowski
Copy link
Contributor Author

Added test that passes, and contains one of each component

@mszylkowski
Copy link
Contributor Author

@rsimha for presubmit-checks.js owners approval

@jridgewell jridgewell changed the title ♻️Change version to 0.1 ♻️ Change amp-story-interactive version to 0.1 Sep 1, 2020
@mszylkowski mszylkowski merged commit 82dfe91 into ampproject:master Sep 1, 2020
@mszylkowski mszylkowski deleted the interactive_version branch September 1, 2020 17:44
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
* Changed to 0.1

* Missing "interactive"

* Changed validator to approve 0.1

* Removed new example

* Added valid validation test

* Updated comments and descriptions

* Added extra comments

* Update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants