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

Fix overlay clickthroughs #160

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Fix overlay clickthroughs #160

merged 1 commit into from
Oct 15, 2024

Conversation

rossmc
Copy link
Contributor

@rossmc rossmc commented Oct 15, 2024

Remove the pointer-events-none class from the overlay. This class disables pointer events, allowing interaction with elements behind the overlay, which defeats its purpose, especially for an overlay containing a spinner.

I encountered this issue while working on the Hyvä checkout with Stripe. If the user double-clicked 'Proceed to review & payments,' the Google Wallet payment option would sometimes appear twice, as each click caused digital wallet methods (like Google and Apple Pay) to initialize multiple times.

Remove the pointer-events-none class from the overlay. This class disables pointer events, allowing interaction with elements behind the overlay, which defeats its purpose, especially for an overlay containing a spinner.

I encountered this issue while working on the Hyvä checkout with Stripe. If the user double-clicked 'Proceed to review & payments,' the Google Wallet payment option would sometimes appear twice, as each click caused digital wallet methods (like Google and Apple Pay) to initialize multiple times.
Copy link

@GrimLink GrimLink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rossmc the removal of the pointer-events-none feels like a band-aid solution.

The reason that this on the overlay is to prevent any clicks, from passing down.
If we used the HTML Dialog, you would keep the same issue with the native overlay (backdrop).

From what I read in your description, the solution should be to avoid extra clicks on the 'Proceed to review & payments' button.
This will solve the double render without breaking the overlay behavior.

@rossmc
Copy link
Contributor Author

rossmc commented Oct 15, 2024

Hi @GrimLink ,
I think you misunderstood me and we are suggesting the same thing...

The reason that this on the overlay is to prevent any clicks, from passing down.

That is the opposite of what pointer-events-none does.

Use pointer-events-none to make an element ignore pointer events. The pointer events will still trigger on child elements and pass-through to elements that are “beneath” the target.
https://tailwindcss.com/docs/pointer-events#controlling-pointer-event-behavior

Usually when a spinner overlay is used it is to stop users from clicking elements beneath it. Add the pointer-events-none allows users to click elements beneath it, adding unusual behavior for an overlay. Do you see this CSS rule applied to spinner overlays usually?

@GrimLink
Copy link

Your right @rossmc, my apologies, I have seem to have mixed the behavior around, we want to overlay from prevent clicks below, not the other way around, in that case I will approve the MR

Copy link

@GrimLink GrimLink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solves the issue from clicks passing trough the overlay @wpoortman,
the pointer-events-none should not be added here if we want to prevent any clicks from passing down.

I mixed the CSS Property around in my first review 😅 while my intentions where the same as @rossmc, so this MR is perfectly valid.

My other point still stands for keyboard users, since technically with a keyboard we can still click the button.
But we could add this in a separate MR and merge this one for at least mouse and touch clicks

@wpoortman wpoortman merged commit 54dd735 into magewirephp:main Oct 15, 2024
1 check passed
@wpoortman
Copy link
Collaborator

Thank you both for your contribution!

mdevrees pushed a commit to mdevrees/magewire that referenced this pull request Dec 5, 2024
Remove the pointer-events-none class from the overlay. This class disables pointer events, allowing interaction with elements behind the overlay, which defeats its purpose, especially for an overlay containing a spinner.

I encountered this issue while working on the Hyvä checkout with Stripe. If the user double-clicked 'Proceed to review & payments,' the Google Wallet payment option would sometimes appear twice, as each click caused digital wallet methods (like Google and Apple Pay) to initialize multiple times.
wpoortman pushed a commit that referenced this pull request Dec 9, 2024
commit ede4a91
Author: Willem Poortman <[email protected]>
Date:   Mon Dec 9 14:27:38 2024 +0100

    Feature/component loader overlay (#162)

    * Component specific loader overlay

    * Reverted the overlay spinner SVG

    * Removed experimental code

    * Removed unused use statement in ConfigMagewire class

    ---------

    Co-authored-by: Willem Poortman <[email protected]>

commit 838a3fe
Author: Willem Poortman <[email protected]>
Date:   Mon Dec 9 09:11:28 2024 +0100

    Removing a sponsor

commit 54dd735
Author: Ross <[email protected]>
Date:   Tue Oct 15 21:09:05 2024 +0200

    Update overlay.phtml (#160)

    Remove the pointer-events-none class from the overlay. This class disables pointer events, allowing interaction with elements behind the overlay, which defeats its purpose, especially for an overlay containing a spinner.

    I encountered this issue while working on the Hyvä checkout with Stripe. If the user double-clicked 'Proceed to review & payments,' the Google Wallet payment option would sometimes appear twice, as each click caused digital wallet methods (like Google and Apple Pay) to initialize multiple times.

commit c3839fa
Author: Willem Poortman <[email protected]>
Date:   Thu Sep 26 21:18:28 2024 +0200

    Update README.md

    README shield improvements

commit b246362
Author: Willem Poortman <[email protected]>
Date:   Tue Sep 10 09:05:28 2024 +0200

    Update README.md

commit e1e5fff
Author: Willem Poortman <[email protected]>
Date:   Tue Sep 10 09:04:40 2024 +0200

    Added M.acadamy as a sponsor

commit ffab4ba
Author: Mehmet Uygun <[email protected]>
Date:   Thu Aug 22 14:42:50 2024 +0100

    Add missing parenthesis (#159)

commit 1291fc8
Author: Willem Poortman <[email protected]>
Date:   Mon Jul 29 11:27:32 2024 +0200

    Minor controller code improvements before release 1.11.1

commit e5d4ecf
Author: Jisse Reitsma <[email protected]>
Date:   Mon Jul 29 11:11:29 2024 +0200

    Fix Livewire Browser Plugin component updates (#136)

    With the Livewire Browser Plugin you can modify component properties, which generate an AJAX call back to the server (similar to `form.set('address.country_id', 'NL')` via the MageWire JS API). However, in that case, the request parameters like the `fingerprint` are sent as part of a JSON body, and not with request parameters. This PR simply supports both.

commit 8e39f68
Author: Willem Poortman <[email protected]>
Date:   Fri Jul 26 14:52:58 2024 +0200

    Typo

commit 176ddb2
Author: Willem Poortman <[email protected]>
Date:   Fri Jul 26 14:31:23 2024 +0200

    Fix: catch any serialisation related error and return/response as json (#157)

    * fix: catch any serialisation related error and return/response as json

    * Avoid providing the frontend with sensitive data coming from the exception message

    * StyleCI fix

    ---------

    Co-authored-by: Anton Siniorg <[email protected]>
    Co-authored-by: Willem Poortman <[email protected]>

commit cb0ed5a
Author: Willem Poortman <[email protected]>
Date:   Fri Jul 26 09:36:12 2024 +0200

    Update version from 1.11.0 - 1.11.1

commit abf60a9
Author: Jeroen Noten <[email protected]>
Date:   Sat Jul 20 09:40:21 2024 +0200

    Prevent modal from closing when clicking on overlay (#156)

commit e26823f
Author: Kiel <[email protected]>
Date:   Wed Jul 17 15:23:31 2024 +0100

    Fix Loader Stop Event Name (#155)

commit c7f610e
Author: Willem Poortman <[email protected]>
Date:   Sat Jul 6 15:34:23 2024 +0200

    Kickstarter course removal

commit c4071a9
Author: Kiel <[email protected]>
Date:   Mon Apr 15 12:12:17 2024 +0100

    Add Info for JS-Based Emit Message Listeners (#149)

commit 3dec156
Author: Willem Poortman <[email protected]>
Date:   Mon Feb 5 19:27:09 2024 +0100

    Update README.md

commit 27c680a
Author: Willem Poortman <[email protected]>
Date:   Mon Feb 5 12:17:57 2024 +0100

    Update README.md

commit 55a10e0
Author: Willem Poortman <[email protected]>
Date:   Mon Feb 5 12:16:26 2024 +0100

    Added Zero-1 as sponsor

commit 40cc7cc
Author: Kamil Balwierz <[email protected]>
Date:   Mon Dec 18 16:48:54 2023 +0100

    Avoid TypeError in bussy queue (#143)

commit a0c1182
Author: Willem Poortman <[email protected]>
Date:   Tue Dec 12 14:57:13 2023 +0100

    Added a common separate issues section in the docs directory (#144)

    Co-authored-by: Willem Poortman <[email protected]>
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.

3 participants