✅ Add visual test for amp-autocomplete #22045
Conversation
Remove second test case
caroqliu
force-pushed
the
visual-test-split
branch
from
0f5737d
to
dfd35eb
Compare
| </script> | ||
| <script async src="https://cdn.ampproject.org/v0.js"></script> | ||
| <script async custom-element="amp-autocomplete" src="https://cdn.ampproject.org/v0/amp-autocomplete-0.1.js"></script> | ||
| <script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-latest.js"></script> |
There was a problem hiding this comment.
I think we prefer to use specific versions for the extensions. There are other visual tests that use *-latest.js, but unless these tests have a good reason to use latest, let's use amp-mustache-0.2.js and amp-form-0.1.js
@danielrozenberg to verify
There was a problem hiding this comment.
I don't have a specific preference here and there's definitely no strict policy. Your reasoning is sound though, it makes sense to pin this to a specific version because you're not testing amp-mustache here :)
There was a problem hiding this comment.
Done. This also helped me notice that amp-mustache isn't needed for this test, so removed it for now.
| </script> | ||
| <script async src="https://cdn.ampproject.org/v0.js"></script> | ||
| <script async custom-element="amp-autocomplete" src="https://cdn.ampproject.org/v0/amp-autocomplete-0.1.js"></script> | ||
| <script async custom-template="amp-form" src="https://cdn.ampproject.org/v0/amp-form-0.1.js"></script> |
There was a problem hiding this comment.
nit: sorry I missed this last time, but this should say custom-element="amp-form" instead of custom-template="amp-form"
Add visual test for amp-autocomplete rendering and displaying items on focus for plain-text inline data.