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.show() race condition #3456

Open
silverdr opened this issue Oct 5, 2024 · 4 comments
Open

JS.show() race condition #3456

silverdr opened this issue Oct 5, 2024 · 4 comments

Comments

@silverdr
Copy link
Contributor

silverdr commented Oct 5, 2024

Environment

  • Elixir version (elixir -v): Elixir 1.17.3 (compiled with Erlang/OTP 27)
  • Phoenix version (mix deps): phoenix 1.7.14
  • Phoenix LiveView version (mix deps): phoenix_live_view 1.0.0-rc.6
  • Operating system: GNU/Linux, macOS 14.6.1
  • Browsers you attempted to reproduce this bug on (the more the merrier): Chrome and derivatives (Brave, Chromium), Firefox, Safari
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes

Actual behavior

When using JS.show() with animated transition

@keyframes fade-in-scale {
	0% {
		transform: scale(0.4) translateY(25px);
		opacity: 0;
	}

	90% {
		transform: scale(1.05) translateY(-3px);
		opacity: 0.9;
	}

	100%{
		transform: scale(1.0) translateY(0px);
		opacity: 1;
	}
}

.fade-in-scale {
	animation-direction: normal;
	animation-duration: .2s;
	animation-fill-mode: forwards;
	animation-name: fade-in-scale;
}

sometimes the element pops-up BEFORE the transition animation starts, then disappears and only then animates. Video showing the problem:

https://cld.silverdr.com/s/z8KbZkCDGw8Ffyx

Expected behavior

The transition flow is undisturbed every time.

The relevant elixirforum thread:
https://elixirforum.com/t/liveview-js-show-hide-race-condition/66405/

The Pull Request with a quick fix.

#3455

Please note that I am not familiar with the codebase so can't guarantee that it is the most "correct" way to deal with the problem.

@SteffenDE
Copy link
Collaborator

Hey @silverdr,

thank you for reporting! It would be helpful if you could provide a sample script to reproduce the problem. I cannot currently say if your quick fix has any consequences on transition based animations, so I'd need to experiment a little bit myself. A reproduction would help a lot, but I can try to come up with one myself when I find the time :)

@silverdr
Copy link
Contributor Author

@SteffenDE I apologise for the delayed response. I must have overlooked notification :-( Yes, I can prepare a more detailed example shortly but FWIW the problem is already visible when following the documentation examples as described in the linked Elixir forum thread. The trigger in my case is only a bit different but it doesn't make any change in behaviour:

phx-click={JS.show(to: "#window-title-window", transition: "fade-in-scale", display: "flex")}

As for the pull request – it shows what change "fixed" the problem at hand but the disclaimer obviously applies ;-)

@silverdr
Copy link
Contributor Author

silverdr commented Nov 18, 2024

Here comes the example. If you need anything else, please ping me back.

router.ex:

[...]
	pipeline :browser do
		plug :accepts, ["html"]
		plug :fetch_session
		plug :fetch_live_flash
		plug :put_root_layout, html: {MyApp.Layouts, :root}
		plug :protect_from_forgery
		plug :put_secure_browser_headers
	end

	scope "/", MyApp do
		pipe_through :browser

		live "/races", RacesLive
	end
[...]

races_live.ex:

defmodule MyApp.RacesLive do
	use MyApp, :live_view

	def mount(_params, _session, socket) do
		socket = socket
		|> assign(:page_title, "Races")

		{:ok, socket}
	end

	def render(assigns) do
		~H"""
		<div id="wrapper" class="mx-6 my-6 w-64 h-64">
			<div id="target" class="w-64 h-64 bg-red-600"></div>
		</div>
		<div id="buttons-wrapper" class="flex justify-around w-64">
			<button class="px-2 rounded border-2" phx-click={JS.show(to: "#target", transition: "fade-in-scale")}>Show</button>
			<button class="px-2 rounded border-2" phx-click={JS.hide(to: "#target", transition: "fade-out-scale")}>Hide</button>
		</div>
		"""
	end
end

app.css:

@keyframes fade-out-scale {
	100% {
		/* transform: scale(0.4); */
		transform: scale(0.4) translateY(25px);
		opacity: 0;
	}
}

.fade-out-scale {
	opacity: 1;
	animation: .2s ease-in fade-out-scale;
	animation-fill-mode: forwards;
}

@keyframes fade-in-scale {
	0% {
		transform: scale(0.4) translateY(25px);
		opacity: 0;
	}

	90% {
		transform: scale(1.05) translateY(-3px);
		opacity: 0.9;
	}

	100%{
		transform: scale(1.0) translateY(0px);
		opacity: 1;
	}
}

.fade-in-scale {
	animation-direction: normal;
	animation-duration: .2s;
	animation-fill-mode: forwards;
	animation-name: fade-in-scale;
}

Video of the results:

https://cld.silverdr.com/s/gZZfpDtjgwnzk3C

@silverdr
Copy link
Contributor Author

@chrismccord please let me know if I can help more with this one. Regards.

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

No branches or pull requests

3 participants