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

Add blocking: true to JS.dispatch #3615

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

SteffenDE
Copy link
Collaborator

Relates to: #3516

When integrating external animation libraries like motion.dev, the existing JS functions are not sufficient. Instead, the third party library needs to be triggered via JS. This has the downside of not being able to block the DOM until the animation is complete, which prevents this from working when elements are removed using phx-remove.

This commit introduces a new blocking: true option to JS.dispatch/3, which injects a done function into the event's detail object.

Using this with motion could look like this:

def render(assigns) do
  ~H"""
  <div :if={@show} phx-remove={JS.dispatch("motion:rotate", blocking: true)}>
    ...
  </div>
  """
end
const { animate } = Motion

window.addEventListener("motion:rotate", (e) => {
  animate(e.target, { rotate: [0, 360] }, { duration: 1 }).then(() => {
    if (e.detail.done) {
      e.detail.done()
    }
  })
})

It is still necessary to block the DOM while the remove animation is running, as the remove can happen because of a navigation, where the animation would otherwise not run as the whole LiveView is just replaced.

Relates to: #3516

When integrating external animation libraries like motion.dev, the existing
JS functions are not sufficient. Instead, the third party library needs to
be triggered via JS. This has the downside of not being able to block the DOM
until the animation is complete, which prevents this from working when
elements are removed using `phx-remove`.

This commit introduces a new `blocking: true` option to `JS.dispatch/3`,
which injects a `done` function into the event's `detail` object.

Using this with motion could look like this:

```elixir
def render(assigns) do
  ~H"""
  <div :if={@show} phx-remove={JS.dispatch("motion:rotate", blocking: true)}>
    ...
  </div>
  """
end
```

```javascript
const { animate } = Motion

window.addEventListener("motion:rotate", (e) => {
  animate(e.target, { rotate: [0, 360] }, { duration: 1 }).then(() => {
    if (e.detail.done) {
      e.detail.done()
    }
  })
})
```

It is still necessary to block the DOM while the remove animation is
running, as the remove can happen because of a navigation, where the
animation would otherwise not run as the whole LiveView is just replaced.
@josevalim
Copy link
Member

josevalim commented Jan 4, 2025

Given we already support :blocking in other commands, this is a no-brainer for me. 👍 (although I can't speak for the JS code).

In the long term we want to look at this holistically. Overall we have three distinct capabilities:

  1. Lock the whole UI
  2. Lock certain elements
  3. Annotate an element while there is an event in flux (such as classes we attach to the form)

We already have :blocking to mean 1 and :loading to mean 2 and 3, but we definitely want to expose more granular controls around them. If folks have use cases for them, please let us know, as it will help us refine the APIs (as mentioned in #3516). But once again, I don't think this applies here, :blocking is already available elsewhere, so this makes it consistent.

It is also worth saying that, if you can block the whole UI with the :blocking option, that's the best option, because it guarantees no inconsistencies. That's good as long as what you are doing is perceived by the user as fast. If you lock granular elements, then there is always a chance concurrent updates will interact in weird ways, for example, the element will be removed by a server update in the middle of an animation.

SteffenDE added a commit that referenced this pull request Jan 5, 2025
Relates to #3615.

Allowing beforeUpdate to cancel an update by returning false is **not**
implemented, as this would require more complex internal changes.
@SteffenDE
Copy link
Collaborator Author

Still todo: tests

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

Successfully merging this pull request may close these issues.

2 participants