RetroAchievements / RAWeb

RetroAchievements.org Platform
https://retroachievements.org
GNU General Public License v3.0
237 stars 86 forks source link

Problem with alerts in system page #2478

Open gboquizosanchez opened 1 month ago

gboquizosanchez commented 1 month ago

In https://retroachievements.org/system/1/games you can see this behaviour when you press in buttons on backlog columns:

In mobile or I guess it's md, or sm there is a problem with z-index and the circles. Also, there is a problem in the alert position:

image

And in xl or lg. There is a problem with header z-index. Alert isn't shown properly:

image

Edit: When you press more than X times the button, the alert is tourned red. I think is caused for "Too many requests". But still it's weird, the color is changed permanently.

wescopeland commented 1 month ago

Thanks for reporting.

Over the short term, we should definitely fix this.

Over the long term, I think it'd be ideal if we switched to using some kind of robust toast library as we migrate more client-side interactivity to Livewire 3. I love react-hot-toast and its API, but unfortunately as the name implies, it's tightly coupled to React.

gboquizosanchez commented 1 month ago

Maybe livewire toaster can be a good solution.

wescopeland commented 1 month ago

Looked into this a few weeks ago. That package is challenging because RAWeb is currently on Laravel 10.

I can't help but wonder if a vanilla JS approach would be more desirable since it's a pure client-side interaction. If the toasts primarily are Livewire components themselves, that implies for one to appear there must first be a successful network round trip to the server to even hydrate the component.

I'd love to see some lightweight functions added to the global window object which can be called from JS or a Livewire component. I'm really envisioning an API like:

// Immediately pop success.
toast.success("Added to Want to Play Games!");

// Show loading while the function is running, then change status based on resolve or reject.
toast.promise(
    someAsyncFunction(),
    {
        loading: "Loading...",
        success: "Did the thing!",
        error: "Something went wrong.",
    }
);

From a Livewire context, these could be triggered via dispatches, eg:

$this->dispatch('toast-success', 'Added to Want to Play Games!');
wescopeland commented 1 month ago

There could be pitfalls to this I haven't considered. This is mostly my React brain talking 😁

gboquizosanchez commented 1 month ago

Well, a toast can be called on javascript with alpine.js directly from the component:

image

It's similar to this:

// Immediately pop success.
toast.success("Added to Want to Play Games!");

Am I right?

wescopeland commented 1 month ago

Yep!

The main thing I'm curious about with this library is if it can handle the use case I've described for pending toasts. I'm not sure if that's fully supported: https://github.com/masmerise/livewire-toaster?tab=readme-ov-file#sending-toasts-from-the-front-end

This is the behavior I want to support:

https://github.com/RetroAchievements/RAWeb/assets/3984985/93778e37-0d64-41c8-a487-5a3bda4d44e0

The code for this behavior is:

toast.promise(
  saveSettings(settings),
   {
     loading: 'Saving...',
     success: <b>Settings saved!</b>,
     error: <b>Could not save.</b>,
   }
 );
gboquizosanchez commented 1 month ago

I tested the behavior of PendingToast for livewire toaster.

It's launched when a condition is fulfilled, but... it's not the same you want it.

Probably a workaround similar can be made, but nope out of the box as React does.

wescopeland commented 1 month ago

Bummer. That's what I was afraid of.

For full disclosure, I'm trying to optimize towards the new INP core web vital.

Popping a toast only on the condition being fulfilled will cause a site to trend into an unhealthy score for this web vital over a long enough period of time, almost entirely from traffic on slower/unstable network connections like mobile.

The solution is to pop a toast instantly, which changes on success. But I think with livewire-toaster, the only way to do this is with two toasts, which could get noisy for users that have fast internet connections such as on desktop.

wescopeland commented 2 weeks ago

We've decided to begin migrating the UI from Blade/Livewire to Inertia/React.js.

A POC of this migration has been opened here: https://github.com/RetroAchievements/RAWeb/pull/2502

As we work through the migration, we'll likely pull in react-hot-toast or @radix-ui/react-toast for our toasts library.