鉁╝mp-experiment: Base functionality for Demo #21734
Conversation
| return false; | ||
| }, | ||
| src: value => { | ||
| return value.match(/^https:\/\//g); |
There was a problem hiding this comment.
may want to use assertHttpsUrl here and below
Line 297 in f52c37a
| } | ||
|
|
||
| // Allow Color | ||
| if (value.match(/^color:\s#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/g)) { |
There was a problem hiding this comment.
Should the space be optional here?
| } | ||
|
|
||
| // Allow Background color | ||
| if (value.match(/^background-color:\s#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/g)) { |
There was a problem hiding this comment.
Im fine with this for now, but I feel like eventually the property validation should be separated from the value validation.
There was a problem hiding this comment.
Totally agree, after our demo, we will probably be moving this to the Validator. If not, I added an note here: #21705 馃槃
There was a problem hiding this comment.
nit/bikeshedding:
Why did you go with just \s and not \s* after the colon (:)?
And why no whitespace before the colon or before/after the entire value set?
not nit-picking:
Why the /g? shouldn't there be only one background-color? what does the global accomplish or am I misunderstanding it (happens with regexs)
馃榿
|
@calebcordry Made requested changes / replied to comments 馃槃 |
|
馃憤馃憡馃帀 |
|
馃挭 mode! |
|
Thanks @calebcordry @jeffjose 馃槃 Merging... |
relates to #20225
This implements some base functionality for amp-experiment for an upcoming initial demo. This includes support for some of the
attributestype mutations (style withcolorandbackground-color, src, and href), as well ascharacterDatamutations 馃槃Example