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

Mobile touch events cause multiple onSelectedItemChange calls #1534

Closed
Mighty683 opened this issue Sep 1, 2023 · 17 comments · Fixed by #1558
Closed

Mobile touch events cause multiple onSelectedItemChange calls #1534

Mighty683 opened this issue Sep 1, 2023 · 17 comments · Fixed by #1558

Comments

@Mighty683
Copy link

Mighty683 commented Sep 1, 2023

  • downshift version: 7.6.0
  • node version: 20

What you did:

On my project I have an issue that in some cases downshift calls onSelectedItemChange in a loop which crash application.
I made an reproduction which reproduce this problem with double call.

Seems to be the cause of issue: 1511

What happened:

onSelectedItemChange function is called multiple times on mobile devices, depending on how complex component is, number of calls increase

Reproduction repository:

Reproduction

  1. Open sandbox in a new window
  2. Turn on mobile simulation with touch events enabled
  3. Select some option and check console for counts of calls.

Problem description:

Issue seems to be connected to re-renders on onSelectedItemChange cause more calls it cause.

@Mighty683 Mighty683 changed the title Mobile touch events cause multiple onSelectedItemChange Mobile touch events cause multiple onSelectedItemChange calls Sep 1, 2023
@TetriPetri
Copy link

TetriPetri commented Sep 1, 2023

I ran into this issue today but I came to the conclusion that it is a problem that is related the responsive tool and simulated touch events.

For example it does not work in chrome when you select device type: mobile
image

I booted up my site with ngrok and ran on an iPhone 12 mini and ran into no issues with chrome and safari.

I have not tested it on a live site though.

Did you test on real devices?

Edit: I just ran your codesandbox on my iPhone 12 mini, chrome without issues. Maybe android related?
Edit 2: I was able to reproduce it on an Android device, Samsung browser and Chrome

@Mighty683
Copy link
Author

Yes I am able to reproduce this issue on real devices, also on Firefox mobile emulator.

@Mighty683
Copy link
Author

I ran into this issue today but I came to the conclusion that it is a problem that is related the responsive tool and simulated touch events.

For example it does not work in chrome when you select device type: mobile
image

I booted up my site with ngrok and ran on an iPhone 12 mini and ran into no issues with chrome and safari.

I have not tested it on a live site though.

Did you test on real devices?

Edit: I just ran your codesandbox on my iPhone 12 mini, chrome without issues. Maybe android related?

Did you opened it in new window? I noticed that sandbox on codepen needs to be opened separately to be able to reproduce this issue.

@TetriPetri
Copy link

Yes, it runs on my iPhone without any issues. Here are the exact steps:

  1. Open chrome on iPhone 12 mini.
  2. Go to: https://4r2943.csb.app/
  3. Toggle dropdown open
image
  1. Select option 2
image
  1. Repeat 3 and 4 without problems.

  2. Open safari on the iPhone

  3. Repeat 2-5 without problems

@Mighty683
Copy link
Author

I was testing on Android device

@TetriPetri
Copy link

TetriPetri commented Sep 5, 2023

I was testing on Android device

I can confirm, tested on Android device today and was able to reproduce!
Tested in Samsung browser and Chrome, on a Samsung S8 device.

@KaivG
Copy link

KaivG commented Sep 5, 2023

I am having the same issue with the newest downshift version, even on desktop without devtools. If I set the menu to stay open after clicking an item and then double click on an item it will crash the application. See my comment in this other issue that seems to be related: #1511 (comment)

@FdelMazo
Copy link

This is also affecting me on react 18 w/downshift 8.2.0

@silviuaavram
Copy link
Collaborator

If someone can investigate this issue it will be really helpful.

@silviuaavram
Copy link
Collaborator

I did some investigating. In React18, for touch events, there is one extra click event.

For instance, if you select an item by touch, the events are: click, mouse move, click. In 17, there is only mouse move then click.

Investigating further.

@silviuaavram
Copy link
Collaborator

Still looking into it, as I went along a wrong trail previously.

@silviuaavram
Copy link
Collaborator

This issue seems to happen when we have both onClick and onMouseMove that trigger dispatch. Updated the Issue on React: facebook/react#27666

I hope to find an elegant solution, as our state logic with useReducer is pretty complex and I would not want to refactor that.

@FdelMazo
Copy link

FdelMazo commented Nov 9, 2023

Thanks so much for taking a look at it @silviuaavram ! Your work is impressive

@silviuaavram
Copy link
Collaborator

silviuaavram commented Nov 13, 2023

Everyone, try downshift@8.2.4-alpha.2. If this solves the issue and does not break anything, we will release it as normal 8.2.4. Thank you!

https://www.npmjs.com/package/downshift/v/8.2.4-alpha.2

@silviuaavram
Copy link
Collaborator

It appears fixed with 8.2.4. I cannot repro it in that sandbox anymore.

@ramon-villain
Copy link

@silviuaavram I'm still facing this issue when running e2e with cypress, onClick is firing twice and as soon as I omit the onMouseMove property it works similarly to React 17.

I'll try to put together a repro branch over the next few days.

@silviuaavram
Copy link
Collaborator

@ramon-villain this is something that's related to React 18, I filed an issue there but no luck so far. I may need to dig into React myself.

For instance, this code will trigger the clicking action twice.

import * as React from 'react'

function reducer(state, action) {
  switch (action.type) {
    case 'incremented_age': {
      console.log('here')
      return {
        name: state.name,
        age: state.age + 1,
      }
    }
    case 'changed_name': {
      console.log('here 2')
      return {
        name: action.nextName,
        age: state.age,
      }
    }
    case 'haha': {
      console.log('here 3')
      return {
        name: 'haha',
        age: state.age,
      }
    }
    default: {
      throw Error('Unknown action: ' + action.type)
    }
  }
}

const initialState = {name: 'Taylor', age: 42}

export default function App() {
  return (
    // <React.StrictMode>
    <Form />
    // </React.StrictMode>
  )
}

function Form() {
  const [state, dispatch] = React.useReducer(reducer, initialState)

  function handleButtonClick() {
    dispatch({type: 'incremented_age'})
  }

  function handleInputChange(e) {
    dispatch({
      type: 'changed_name',
      nextName: e.target.value,
    })
  }

  function handleMove(e) {
    e.preventDefault()
    dispatch({type: 'haha'})
  }

  return (
    <>
      <input value={state.name} onChange={handleInputChange} />
      <button onClick={handleButtonClick} onMouseMove={handleMove}>
        Increment agess
      </button>
      <p>
        Hello, {state.name}. You are {state.age}.
      </p>
    </>
  )
}

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 a pull request may close this issue.

6 participants