phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6.11k stars 915 forks source link

phx-click-loading issue on modals, style appears all buttons are clicked when closing #3421

Open brainlid opened 1 week ago

brainlid commented 1 week ago

Environment

Actual behavior

Being inspired by Jose's video, I thought adding phx-click-loading classes to a button click would be a great way to give visual feedback of a click that is handled by LiveView. This is particularly important on mobile clients when hover styles have been disabled (because those cause bad UX).

It worked great until I closed the modal it was on.

The docs here explain:

While loading classes are extremely handy, they only apply to the element currently clicked.

This is not entirely true. The default Tailwind setup (also in the docs), adds the following variant:

plugins: [
  plugin(({ addVariant }) => {
    addVariant("phx-click-loading", [".phx-click-loading&", ".phx-click-loading &",]);
    // ...
  }),
],

When I close a modal by either hitting "Escape" or the "Close" button (with triggers the on_cancel), then a phx-click-loading class is added to the modal. This causes ALL buttons on the modal with phx-click-loading:bg-indigo-600 phx-click-loading:text-white to appear as though they were clicked.

I tried changing the Tailwind variant to look like this:

plugins: [
  plugin(({ addVariant }) => {
    addVariant("phx-click-loading", [".phx-click-loading&",]);
    // ...
  }),
],

Making the effect not apply to it's children. Essentially, trying to make the docs true about ONLY applying to the clicked item. However, this doesn't work well when a clicked element has a nested span or div. The nested elements not longer gets the style applied.

Desired behavior

Don't apply the phx-click-loading to a modal, or at least provide a way to NOT automatically apply it. It breaks the usability of phx-click-loading on elements inside the modal.

SteffenDE commented 6 days ago

I'd say this is a bug. We shouldn't add phx-click-loading if there was no click.

The issue comes from JS.patch issuing a pushHistoryPatch with the source element (in this case the modal) as target:

https://github.com/phoenixframework/phoenix_live_view/blob/67cb944e7575fb89e17f243d919a2911317dd6ee/assets/js/phoenix_live_view/js.js#L86-L88

https://github.com/phoenixframework/phoenix_live_view/blob/67cb944e7575fb89e17f243d919a2911317dd6ee/assets/js/phoenix_live_view/live_socket.js#L792-L801

https://github.com/phoenixframework/phoenix_live_view/blob/67cb944e7575fb89e17f243d919a2911317dd6ee/assets/js/phoenix_live_view/view.js#L1295-L1297

So one way to fix it would be to just not pass the source element there, but that would probably break

<button phx-click={JS.patch(...)}>

@chrismccord