Amphtml: amp-list: Unsynchronized rendering in action sequences

[leonalicious]

please visit this link https://m.aliexpress.com/wholesale/dress.html to try them out on both Safari and Chrome

Basically, when user clicks on Brand Wall

1. We will submit the form and during the Form Submission, we will show the loading animation
2. When the Form Submission is returned with success, we will hide the loading animation

please refer to the coding snippet as below:

As you may see, it is working perfectly on Safari

But not on Chrome, it is still showing previous items and then blink to render new set of items.

Any Suggestion / Advice so that I can achieve the same effect on Chrome as on Safari ?

[leonalicious]

btw, i have even tried to include reset-on-refresh to trigger out placeholder under amp-list but it is not working as expected

[leonalicious]

hi guys, any updates on this please ?

[choumx]

Sorry for the delay, just got back from vacation. I'll take a look this or next week!

[leonalicious]

np, welcome back and appreciate your assistance on this :)

[leonalicious]

hi @choumx , any updates on this please ?

[choumx]

So I think the problem is the way this set of actions is chained:

<form id="form-filter" method="GET" target="_top" 
  action="//m.aliexpress.com/api/search/products/dress/items" 
  action-xhr="//m.aliexpress.com/api/search/products/dress/items" 
  on="
    submit: 
      search-list.hide, 
      loading-animation.show, 
      show-more.hide;
    submit-success: AMP.setState({
        productsData: {
          data: {
            items: event.response.data.items.length > 0 ? event.response.data.items : null
          }
        },
        filterData: {
            start: 0 + 20,
            refine: event.response.data.refine
        }
      }), 
      search-list.show, 
      loading-animation.hide, 
      show-more.show" 
  novalidate="" class="i-amphtml-form">

Specifically, AMP.setState() doesn't wait until the amp-list#search-list-mode is finished rendering the updated productsData (which takes awhile). So search-list.show will often be triggered before the rendering completes, which ends up showing the old product list.

Instead of manually sequencing showing/hiding the product list and loading animation, set the loading animation as the amp-list's placeholder and use reset-on-refresh instead. Can you explain why that didn't work for you?

[leonalicious]

hey @choumx ,thank you for the suggestion on reset-on-refresh

based on my understanding, the workflow is listed as below:

1. User clicks on actions which triggers the form submission.
2. Form returns the updated data and bind into amp-state
3. amp-list detects there are any changes on [src] and re-render its contents.
4. and on this stage, we should use reset-on-refresh to trigger the amp-list's placeholder instead of manually sequencing loading animation.

I have somehow written some demo, here the link for the coding and found that reset-on-refresh is not working with form submission, I have tried both approaches below and reset-on-refresh is not being triggered !

1. <div on="tap: form.submit, list.refresh">
2. <form on="submit: list.refresh">

FYI, every form submission + amp-list's [src] scenarios are having the same issue, which includes the "View More Data" button action.

my wild guess, is reset-on-refresh only works with amp-list's src and NOT [src] ?

and also this doesn't explain why it is not working on chrome base browsers but Safari.

please do let me know anything we may provide which would be helpful to solve this issue ~

[choumx]

Ah, so reset-on-refresh only triggers when [src] is updated with a URL string rather than with the data directly as you're doing with the form response. Even if it did, you wouldn't see the placeholder while the form was being submitted (which is probably the majority of the latency after user click).

This means that removing the form#form-filter and using <amp-list [src]> directly should work. However, it looks like there are a lot of form inputs which may hit the expression complexity limit if you use string concatenation expressions (even though it's been doubled recently). 🙁

If you can remove the form and the expression complexity limit is still an issue, try using the new AMP_STATE(foo.bar) URL variable.

If removing the form won't work for you, we maybe can build something to make these action sequences easier to coordinate. /cc @aghassemi

[leonalicious]

thx @choumx , to be honest we would prefer latter, here are some reasons

1. if we cant control the sequence, it basically means that form submission + amp-list approach is not consistent to be used.
2. we would prefer form.submit approach rather than URL string concatenation because we did have experience of 2 sets of amp-state

<amp-state id="myState1">

<amp-state id="myState2">

we AMP.setState myState1 and not too sure why myState2 get affected as well and the entire myState2 will be re-rendered and the elements bound to it were blinked.

and also, from my personal point of view, form.submit is more controllable & direct-viewing.

what do you think ?

[aghassemi]

@leonalicious @choumx

I might be missing some details but for me both [src] change and .refresh() seem to work when tied with submit event of amp-form. See https://codepen.io/aghassemi/pen/aKmemR?editors=1000 (in your sample code, the list2 was being refreshed but it did not have reset-on-refresh attribute)

(Also you probably want submit-success event instead of submit)

[leonalicious]

hey @aghassemi , sorry for that silly mistake, I have rectified the source code

you may also try on here https://codepen.io/leonalicious/pen/PamZLY?editors=1000,

the reset-on-refresh is not working with AMP.setState, however, if i remove the AMP.setState, it turns to work fine~

[aghassemi]

Interesting, I can repro now. Not sure why setState stops the next action to fire even when the state being changed has no binding. Have to defer to @choumx. Another interesting issue I noticed is that if I move refresh action before setState, it works the first time but then it stops working on second+ clicks.

[leonalicious]

This is my wild guess of both Safari and Chrome engines on the amp-form + amp-list + amp-bind, how they process coding.

Currently we got 2 cases are using this approach, they are :

1. our Refine, trigger to refresh entire list.
2. Click "View More" to fetch new data and append into amp-list

Even if the workaround reset-on-refresh works, it can only solve our 1st case. Because for our 2nd case, when users clicks on 'View More', we are not supposed to render the ENTIRE list by triggering the 'placeholder' of amp-list.

i would agree with @choumx that we maybe can build something to make these action sequences easier to coordinate. /cc @aghassemi

[leonalicious]

guys, please give me some feedback, there are some issues require communications to take place in order to come out solutions, and i think this is one of them.

[choumx]

Thanks for pinging and for your continued patience. Sorry that this is taking months to resolve -- we are each juggling a bunch of different projects at any given time.

I'm not sure why it works on Safari. I think the behavior will be inconsistent across browsers since it's a race condition.

Unfortunately, I haven't come up with a generic, elegant way to coordinate/sequence action chains yet. I think for this particular use case, using a first-class component (i.e. amp-list) is really the "right" thing to do.

Looking at this form again, this is the sequence of actions that's happening:

1. Hide the product list
2. Show loading animation
3. Hide the "show more" button
4. Fetch the product data with new filters
5. Render the product list from the fetched data
6. Show the product list
7. Hide the loading animation
8. Show the "show more" button

I think it's pretty bad if AMP developers need to do this for every e-commerce product list page. Instead, we should encourage usage of amp-list for this and address the gaps that @leonalicious mentioned here.

1. if we cant control the sequence, it basically means that form submission + amp-list approach is not consistent to be used.
2. we would prefer form.submit approach rather than URL string concatenation because we did have experience of 2 sets of amp-state.... the entire myState2 will be re-rendered and the elements bound to it were blinked.

1. Correct. Again, I think using amp-list's built-in features is the right call here. Though maybe this can be solved if amp-form could be set as the "data source" for an amp-list. [*]
2. This is partially caused by the fact that amp-list does not do DOM diffing yet. Though I'm curious why the myState2 is being re-fetched and re-rendered -- amp-bind will only cause re-rendering if the JSON data has changed. A concrete example would be great.

[*] Abstractly, maybe the overarching problem is that AMP does not have a "data source" interface for components. E.g. there is a similar tension between amp-list and amp-state WRT fetched JSON. I'll spend some time thinking if there's a better way forward here.

[leonalicious]

@choumx @aghassemi Cool guys, this issue is fixed ! mind to share with us the story behind this ?

[choumx]

Might be the performance optimization in #17724 hiding the race condition.

[leonalicious]

Cool, since this issue is fixed, I will close it ~ thank you once again, appreciate it

[leonalicious]

I think the version released 2 days ago https://github.com/ampproject/amphtml/releases/tag/1810052256480 has brought back this issue on PC Chrome, but we have tested on both Real Device Android & iOS, both of them are still working fine. So, basically it is fine for us but I just want to leave some comment here to let your guys aware of this , cheers, much love <3
/cc @aghassemi @choumx @lannka

[leonalicious]

I think the version released 2 days ago https://github.com/ampproject/amphtml/releases/tag/1810052256480 has brought back this issue on PC Chrome, but we have tested on both Real Device Android & iOS, both of them are still working fine. So, basically it is fine for us but I just want to leave some comment here to let your guys aware of this , cheers, much love <3
/cc @aghassemi @choumx @lannka

Please ignore the thread before, I was wrong, the blinking issue was happening on both PC and mobile.

In fact, we have found the issue and solved it. Because recently we have added a new [class] (amp-bind) inside the amp-list, and it has slightly affected the performance of amp-list to be re-rendered inside the race condition against the search-list.show & animation-loading.hide, so we have no choice but have to remove the newly added [class] condition, to keep it clean and ensure there is no amp-bind involved inside the amp-list in order to stay out of blinking issue.

this is just a temporary fixed to ensure everything is working fine, but moving forward, as there will be more business requirements being involved into this page, we will need to find a way to code these actions to work in sequence based on our expectation. Cheers and much love <3

to : @aghassemi , @choumx

[choumx]

@leonalicious Re: (2) above, we have a new experiment that enables DOM diffing in amp-list. It should fix the "blinking" issue with amp-list.

Try it out by enabling "amp-list-diffing" here: https://cdn.ampproject.org/experiments.html

[ampprojectbot]

This issue hasn't been updated in awhile. @choumx Do you have any updates?