amp-bind: Improve e-commerce example #8924
Conversation
| } | ||
| </style> | ||
| </head> | ||
|
|
||
| <body> | ||
| <button onclick="AMP.toggleExperiment('amp-bind');window.location.href=window.location.href;">Toggle <amp-bind> experiment</button> |
There was a problem hiding this comment.
shouldn't we keep the experiment toggles in for now?
There was a problem hiding this comment.
Really there just for convenience, but onclick is not a valid AMP attribute. We can still use AMP.toggleExperiment in dev console.
| <script type="application/json"> | ||
| { | ||
| "0": { | ||
| "color": "black", | ||
| "image": "./shirts/black.jpg" | ||
| "image": "./shirts/black.jpg", | ||
| "sizes": { |
There was a problem hiding this comment.
Maybe be a little more explicit in the comments above about why '0' has sizes but the others don't?
There was a problem hiding this comment.
Good idea, done. I'll also spell this out more explicitly in the code lab.
examples/bind/ecommerce.amp.html
Outdated
| <amp-img width=200 height=250 src="./shirts/black.jpg" [src]="shirts[selected.id].image"></amp-img> | ||
| </amp-carousel> | ||
|
|
||
| <p class="dots"> |
There was a problem hiding this comment.
Can we make these clickable to move around the carousel?
| </div> | ||
|
|
||
| <div class="options price"> | ||
| <h6>PRICE : <span [text]="shirts[selected.id].sizes[selectedSize] || '---'">---</span></h6> |
There was a problem hiding this comment.
Is this meant to be a heading?
| </button> | ||
| <button class="mdl-button mdl-button--raised hidden" | ||
| [class]="'mdl-button mdl-button--raised ' + | ||
| ((!selectedSize || !shirts[selected.id].sizes || shirts[selected.id].sizes[selectedSize]) ? 'hidden' : '')"> |
There was a problem hiding this comment.
Could you add a comment as to when this button is supposed to be shown vs hidden?
examples/bind/ecommerce.amp.html
Outdated
| @@ -146,6 +150,9 @@ | |||
| - Reintroduce variables in object keys (removed in #7582) | |||
|
|
|||
| <amp-state [src]="..." on="fetch:AMP.setState({"shirts": {selected: {id: event.response}}})" | |||
|
|
|||
| Note that shirt with ID == "0" contains size data (but other IDs don't) | |||
| because it's the default shirt color at page load. | |||
There was a problem hiding this comment.
Optional: Add "Size data for the remaining indices will be downloaded via XHR when their corresponding options are selected." or something like that.
* first pass on prettifying ecommerce sample * more progress on ecommerce sample * add more sizes * inline material design lite css * fix initial value issues * kevin's comments * elaborate comment
* first pass on prettifying ecommerce sample * more progress on ecommerce sample * add more sizes * inline material design lite css * fix initial value issues * kevin's comments * elaborate comment
In preparation for related code lab:
/to @kmh287