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

JS transition end classes are applied at incorrect time (add_class, remove_class, and transition) #2484

Closed
paulstatezny opened this issue Feb 23, 2023 · 6 comments

Comments

@paulstatezny
Copy link
Contributor

paulstatezny commented Feb 23, 2023

Environment

  • Elixir version: 1.14.3
  • Phoenix version: 1.6.16
  • Phoenix LiveView version: 0.18.15
  • Operating system: MacOS Ventura 13.2.1
  • Browsers you attempted to reproduce this bug on: Chrome, Safari (desktop and mobile), Firefox
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes

Terminology

  • JS refers to Phoenix.LiveView.JS.
  • time refers to the :time option given to a JS function.
  • transition_classes, start, and end refer to the {transition_classes, start, end} tuple given as the :transition option for a JS function.

Actual behavior

When using JS.add_class, JS.remove_class, and JS.transition, the end classes are not applied until time has expired. Furthermore, they are never removed from the element.

In other words:

  1. The transition_classes and start classes are immediately added.
  2. After time milliseconds, the end classes are added and the transition_classes and start classes are removed.

Expected behavior

When using JS.add_class, JS.remove_class, and JS.transition, the end classes are applied immediately after the start classes. The end classes are removed after time has expired. This behavior would be consistent with JS.show, JS.hide, and JS.toggle.

In other words:

  1. The start classes are added.
  2. Immediately, in the next available animation frame after [1], the transition_classes are added.
  3. Immediately, in the next available animation frame after [2], the end classes are added and the start classes are removed.
  4. After time milliseconds, the transition_classes and end classes are removed.

Reference of Correct Implementation

The Expected Behavior is precisely how the toggle function in phoenix_live_view.js already works. This explains why JS.show, JS.hide, and JS.toggle behave correctly, since they all use that function behind the scenes.

let onStart = () => {
this.addOrRemoveClasses(el, inStartClasses, outClasses.concat(outStartClasses).concat(outEndClasses));
let stickyDisplay = display || this.defaultDisplay(el);
dom_default.putSticky(el, "toggle", (currentEl) => currentEl.style.display = stickyDisplay);
window.requestAnimationFrame(() => {
this.addOrRemoveClasses(el, inClasses, []);
window.requestAnimationFrame(() => this.addOrRemoveClasses(el, inEndClasses, inStartClasses));
});
};
el.dispatchEvent(new Event("phx:show-start"));
view.transition(time, onStart, () => {
this.addOrRemoveClasses(el, [], inClasses.concat(inEndClasses));
el.dispatchEvent(new Event("phx:show-end"));
});

Example LiveView

Clone this repository and run:

elixir live_view.exs

Then, open your browser to: http://localhost:5001

This runs a LiveView that demonstrates the inconsistent behavior using TailwindCSS classes. Note the mount and render functions in SamplePhoenix.SampleLive in live_view.exs.

Video Demonstration

This video shows what happens when you run the above example LiveView.

The :transition option is identical for every JS function call (with start and end sometimes flipped):

{"transition-opacity duration-1000", "opacity-20", "opacity-100"}

However, only JS.toggle, JS.show, and JS.hide correctly cause the opacity to transition smoothly.

Screen.Recording.2023-02-23.at.1.09.01.PM.mov
@paulstatezny
Copy link
Contributor Author

Thank you Chris and José for the wonderful JS tools! 🙂 (And everyone else who worked on them!)

@chrismccord
Copy link
Member

Thanks for the thorough issue and recreation. As a sanity check, I released a fix to 0.18.15 for transition, but you are using that here. Can you verify if the behavior you were seeing changed between 0.18.14 and 0.18.15? I'll take a look soon. Thanks!

@paulstatezny
Copy link
Contributor Author

paulstatezny commented Feb 23, 2023

Thanks for your quick response, @chrismccord!

I did notice the bugfix in 0.18.15. (It prompted me to see if this was fixed.)

No, this behavior has not changed between 0.18.14 and 0.18.15. I confirmed by locally changing to {:phoenix_live_view, "~> 0.18.14"} in the example repo.

Thank you for your help! 🙇‍♂️ 🙌

@paulstatezny paulstatezny changed the title JS transition end classes applied at incorrect time (add_class, remove_class, and transition) JS transition end classes are applied at incorrect time (add_class, remove_class, and transition) Feb 26, 2023
@aiwaiwa
Copy link
Contributor

aiwaiwa commented Mar 7, 2023

Will this fix affect the timing on JS.toggle, specifically, when the display gets applied?
I'm monitoring that display: flex applies immediately and that ruins the purpose of animation.

My code:

<a href="#"
     class="underline"
     phx-click={
              JS.toggle(
                to: "#buttons-block",
                display: "flex",
                in: "fade-in-scale",
                out: "fade-out-scale",
                time: 500
              )
            }>
.fade-in-scale {
	animation: fade-in-scale 500ms cubic-bezier(0.45, 0, 0.55, 1) 0s 1 normal forwards;
}
@keyframes fade-in-scale {
	0% {
		transform: scaleY(0);
	}
	100% {
		transform: scaleY(1);
	}
}

.fade-out-scale {
	animation: fade-out-scale 500ms cubic-bezier(0.45, 0, 0.55, 1) 0s 1 normal forwards;
}
@keyframes fade-out-scale {
	0% {
		transform: scaleY(1);
	}
	100% {
		transform: scaleY(0);
	}
}

@c4710n
Copy link
Contributor

c4710n commented May 10, 2023

The same issue also occurred in version 0.18.18.

@c4710n
Copy link
Contributor

c4710n commented May 10, 2023

I created a PR (#2622) for this issue. And the preview of the solution looks like this:

My.Movie.mp4

endoooo added a commit to camino-school/lanttern that referenced this issue Oct 2, 2023
updated `:phoenix_live_view` to 0.20.0 to leverage the bug fix for issue phoenixframework/phoenix_live_view#2484

- removed `preload/1` callback in favor of `update_many/1` in `LantternWeb.AssessmentPointEntryEditorComponent` (preload was deprecated in 0.20.0)

- adjusted `LantternWeb.AssessmentPointLiveTest` date assertion regex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants