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: TouchEvent is not defined on Firefox desktop #601

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

Piezoid
Copy link
Contributor

@Piezoid Piezoid commented Mar 20, 2022

On Firefox desktop, we get undefined references errors to window.TouchEvent in the console when the color picker is loaded.

@matmen matmen added the GH - Bug Something isn't working label Mar 21, 2022
@matmen matmen self-assigned this Mar 21, 2022
@matmen matmen added this to the 1.18 milestone Mar 21, 2022
Copy link
Member

@matmen matmen left a comment

Choose a reason for hiding this comment

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

Sorry, but this doesn't seem right. Swapping the statements for TouchEvents and MouseEvents won't work here, because the order of checks is important (touch events only register on the card header, mouse events on the entire page, so this change breaks the behavior on all browsers).

@matmen
Copy link
Member

matmen commented Mar 21, 2022

I've gone ahead and fixed this issue (and another one I found) in the above linked PR. Thanks for your effort and reporting this.

Also, registers the event handlers for the duration of the drag only.
The event handler were not removed because the component is not unmounted.

Signed-off-by: Maël Kerbiriou <[email protected]>
@Piezoid
Copy link
Contributor Author

Piezoid commented Mar 21, 2022

You're right, fixed.

I noticed the event handler were not removed because the component is not unmounted.
Now the event handlers are only registered for the duration of the drag.

Tested with Firefox and Chrome on mobile and desktop.

Edit: Ok, no problem. thanks

@matmen
Copy link
Member

matmen commented Mar 21, 2022

Ah, nice. I think your solution is much more elegant and fixes some more issues. I'll close my PR, thanks for the fix 😉

@Piezoid
Copy link
Contributor Author

Piezoid commented Mar 21, 2022

Sorry I left the tab as a reminder for a while and didn't see your message.

I also noticed the access $el on undefined. With the fix, relativeMove should no longer be called after the card is gone.
It is still not quite clear to me why the beforeUnmount wasn't fired...

@pedrolamas pedrolamas merged commit 34173ad into fluidd-core:develop Mar 21, 2022
matmen pushed a commit to matmen/fluidd that referenced this pull request Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GH - Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants