renderedtext / render_async

render_async lets you include pages asynchronously with AJAX
https://rubygems.org/gems/render_async/
MIT License
1.08k stars 75 forks source link

Manual retry based on JS event #121

Closed vanboom closed 4 years ago

vanboom commented 4 years ago

Use case:

  1. Render a data table on a view using render async.
  2. Allow the user to edit a row in the table.
  3. Trigger a refresh of the table using the render_async plumbing. (Refreshing only the edited row is not feasible because the table contains totals and other calculated data)

I am thinking that we could register an event handler on the container to receive the async-retry event and re-run the original request. After reviewing the code, I see a potential breaking change that I think would be necessary to make this happen.

Around line 48 of _request_jquery.js.erb

      <% if interval %>
          container.empty();
          container.append(response);
        <% else %>
          container.replaceWith(response);
        <% end %>

Replacing the container in the non-interval path does not allow us to retain the container for future event handling. Instead of replacing the container, empty and append would keep the container intact.

This change would potentially break some view code where users expect the container div to disappear when the view is rendered.

I have implemented a possible solution on my fork, branch: immediate_setup. It required the addition of a configuration option for keep_container to keep the existing behavior intact and control whether or not the container would remain for further event handling. I welcome some advice or other options to implement this functionality in a cleaner way. Thanks.

vanboom commented 4 years ago

I just realized that adding the async-retry event would also benefit UIs where the user is given a refresh button.

Pseudocode: On button click -> trigger async-retry event on the container -> render_async reruns the original request and refresh of the container contents.

I realize that there is some overlap with the toggle functionality. Potentially the toggle code could be reviewed to leverage the proposed new async-retry event.

nikolalsvk commented 4 years ago

@vanboom exactly, we would need to switch up the toggle code a bit. The important thing is not to lose the original render_async container by replacing it with response contents.

nikolalsvk commented 4 years ago

I feel changes in this pull request can benefit this issue.

You can now do the following:

<%= render_async comments_path,
                 container_id: 'refresh-me',
                 replace_container: false %>

<button id="refresh-button">Refresh comments</button>

<script>
  var button = document.getElementById('refresh-button')
  var container = document.getElementById('refresh-me');
  button.addEventListener('click', function() {
    var event = new Event('refresh');
    // Dispatch 'refresh' on the render_async container
    container.dispatchEvent(event)
  })
</script>

Is this what you were looking for, @vanboom?

There is more information in the 2.1.8 release and docs here. In the meantime, I will close this issue (we can reopen it if the 2.1.8 version doesn't solve the issue).

Looking forward to your response, cheers 🍻