Add autosuggest examples and amp-bind and action-impl changes it requires#9553
Add autosuggest examples and amp-bind and action-impl changes it requires#9553cvializ merged 6 commits intoampproject:masterfrom
Conversation
|
Should this be renamed to |
extensions/amp-bind/0.1/bind-impl.js
Outdated
| applyBinding_(boundProperty, element, newValue) { | ||
| const property = boundProperty.property; | ||
| switch (property) { | ||
| case 'value': |
There was a problem hiding this comment.
Bind should already support binding to the value attribute on input elements. If this code is removed, does your example break?
There was a problem hiding this comment.
It sets the attribute, but that only works the first time. Subsequent changes to the value attribute don't change the text inside the element : / Changing the value property of the element consistently updates the text.
examples/autocomplete.amp.html
Outdated
| layout="fixed-height" | ||
| height="120" | ||
| src="/form/autocomplete/query" | ||
| [src]="'/form/autocomplete/query?q=' + autocomplete" |
There was a problem hiding this comment.
If autocomplete isn't defined in the first AMP.setState action, the src attribute will be changed to '/form/autocomplete/query?q=null"
I recommend changing this to
[src]="'/form/autocomplete/query' + (autocomplete ? '?q=' + autocomplete : '')that way, null can't get into the query param and the default evaluation of the expression for [src] is equal to the initial vlaue for src.
aghassemi
left a comment
There was a problem hiding this comment.
LGTM, pending Will//Kevin approval for bind changes.
build-system/app.js
Outdated
| }); | ||
| }); | ||
|
|
||
| const autocompleteLanguages = ['ActionScript', 'AppleScript', 'Asp', 'BASIC', |
There was a problem hiding this comment.
list is missing my favourite languages, will not approve PR.
| @@ -0,0 +1,118 @@ | |||
| <!doctype html> | |||
There was a problem hiding this comment.
plz run through validator to be sure valid then add to test-example-validation
|
a little late but LGTM 😅 |
Related-to #8825