Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manual retry based on JS event #121

Closed
vanboom opened this issue Jun 27, 2020 · 3 comments
Closed

Manual retry based on JS event #121

vanboom opened this issue Jun 27, 2020 · 3 comments

Comments

@vanboom
Copy link

vanboom commented Jun 27, 2020

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 pushed a commit to vanboom/render_async that referenced this issue Jun 27, 2020
@vanboom
Copy link
Author

vanboom commented Jun 27, 2020

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.

vanboom pushed a commit to vanboom/render_async that referenced this issue Jun 27, 2020
@nikolalsvk
Copy link
Owner

@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
Copy link
Owner

nikolalsvk commented Oct 24, 2020

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 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants