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

🐛 Fix value_regex for decimal option-*-results-threshold attr values #31241

Merged
merged 1 commit into from
Nov 18, 2020

Conversation

westonruter
Copy link
Member

While importing the validator spec into the AMP-WP plugin (ampproject/amp-wp#5608), I noticed a but with the regex for the option-*-results-threshold attributes on the amp-story-interactive-results component. Namely, if you provide a decimal value, ten it is flagged as a validation error:

          <amp-story-interactive-results
              prompt-text="Your level is"
              option-1-results-category="Beginner" option-1-image="beginner.png"
              option-1-results-threshold="0.5"
              option-2-results-category="Expert" option-2-image="expert.png"
              option-2-results-threshold="80.5">
          </amp-story-interactive-results>

image

If I supply 80. instead of 80.5 then it passes. Clearly a capture group was intended rather than a character class.

@amp-owners-bot amp-owners-bot bot requested a review from Enriqe November 18, 2020 07:01
@amp-owners-bot
Copy link

amp-owners-bot bot commented Nov 18, 2020

Hey @gmajoulet, @mszylkowski, @ampproject/wg-caching! These files were changed:

extensions/amp-story-interactive/validator-amp-story-interactive.protoascii

@westonruter
Copy link
Member Author

cc @ampproject/wg-caching

@gmajoulet gmajoulet merged commit 39859a8 into ampproject:master Nov 18, 2020
banaag added a commit to banaag/amphtml that referenced this pull request Nov 24, 2020
@banaag banaag mentioned this pull request Nov 24, 2020
amaltas added a commit that referenced this pull request Dec 4, 2020
* cl/343164010 Revision bump for #31241

* cl/343904483 Implements gated module/nomodule validation with test cases

* cl/345313752 Revision bump for #31271

* Update validator-amp-accordion.protoascii

* Add htmlparser-interface.js in allowlist

* Fix invalid merge diff resolution

Co-authored-by: Allan Banaag <banaag@google.com>
Co-authored-by: honeybadgerdontcare <sedano@google.com>
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
* cl/343164010 Revision bump for ampproject#31241

* cl/343904483 Implements gated module/nomodule validation with test cases

* cl/345313752 Revision bump for ampproject#31271

* Update validator-amp-accordion.protoascii

* Add htmlparser-interface.js in allowlist

* Fix invalid merge diff resolution

Co-authored-by: Allan Banaag <banaag@google.com>
Co-authored-by: honeybadgerdontcare <sedano@google.com>
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.

6 participants