react-native-android-checkbox icon indicating copy to clipboard operation
react-native-android-checkbox copied to clipboard

flicker on selection

Open vonovak opened this issue 9 years ago • 6 comments

I'm experiencing a weird flicker upon checkbox selection. I select checkbox, is is correctly ticked and after the tick is done, there is a second tick animation directly following it. After that the checkbox stays ticked.

What is this line supposed to do? this.props.value is always the opposite of event.nativeEvent.value, isn't is?

vonovak avatar Aug 25 '16 09:08 vonovak

I think it's similar to this line from React Native's Switch component https://github.com/facebook/react-native/blob/master/Libraries/Components/SwitchAndroid/SwitchAndroid.android.js#L60

Interestingly, this commit https://github.com/facebook/react-native/commit/1fd27dae5e498efb5a12e5eb6350540b25c48696 has changed the order of the onChange calls and setNativeProps.

I'm not experiencing any flickering (on emulator), is there anything I need to do specifically to get that to happen?

mikemonteith avatar Sep 18 '16 20:09 mikemonteith

I was able to fix this issue by changing (in index.js)

 this._onChange = (event) => {
      this.props.onChange && this.props.onChange(event);
      this.props.onValueChange && this.props.onValueChange(event.nativeEvent.value);

      this._checkboxComponent.setNativeProps({on: this.props.value});
}

to

this._onChange = (event) => {
      this.props.onValueChange && this.props.onValueChange(event.nativeEvent.value);
}

It is, in honesty, a bit of a mystery to me why this works. There is a known sort-of-issue with onCheckedChangeListener firing too many times (see, for example here), and in very vague terms I guess that the native checkbox handles its own state and somehow some of these oddities cancel each other out if you don't manually fire setNativeProps or onChange...

jamieparkinson avatar Dec 06 '16 13:12 jamieparkinson

Yeah, works for me too. My guess is that tapping the checkbox triggers a change in native which starts the animation. Then the new value is sent through the bridge and the change triggers another event, this time from JS. Hence the flicker.

Edit: the proposed change has one drawback - suppose user taps checkbox, it gets checked. Then the information is sent to JS and we, for whatever reason, decide we do not want it checked and so 'ignore' the event in the change handler. That will lead to a different state in JS (not selected) and native (selected). This, however, will not affect many.

vonovak avatar Dec 25 '16 14:12 vonovak

Thanks @jamieparkinson it works!

Additional info:

  • without the fix it works fine in emulator (android 7.1), but there is no animation after second tick on real device (android 5.1.1)
  • it works fine both on emulator and on real device after applying the fix

EDIT

It works, but only if onValueChange={onChange} changes the value value={whatever}. Otherwise, the checkbox changes on first click and then it doesn't change. Correct behavior is that it shouldn't change at all because the value didn't change as well.

ErikCupal avatar Dec 30 '16 20:12 ErikCupal

@ErikCupal is right in his edit: What if the checkbox gets ticked, but we don't want its value to change? e.g

  <Checkbox
    value={false} //always false!
  />

The reason we need the setNativeProps call is to make sure our component is always a controlled component. https://facebook.github.io/react-native/docs/switch.html

I don't have a physical android device at the moment so I can't test the flickering issue. 😩

Could the added check in https://github.com/facebook/react-native/commit/1fd27dae5e498efb5a12e5eb6350540b25c48696 help here too? Namely,

 if (this.props.value === event.nativeEvent.value || this.props.disabled) {
   return;
 }

mikemonteith avatar Feb 16 '17 00:02 mikemonteith

Personally I worked around it by using a timeout:

    this._onChange = (event) => {
      // this will set the prop to the one maintained in js
      // this is for case when the change is ignored (for some reason this checkbox cannot be checked) in js and this component does not receive the prop change. 
      // thus native would be out of sync with js.
      this.setToJsValue = setTimeout(()=>{
      	this._checkboxComponent.setNativeProps({on: this.props.value});
      }, 250)
      this.props.onChange && this.props.onChange(event);
      this.props.onValueChange && this.props.onValueChange(event.nativeEvent.value);
    }

the timeout has to be cleared in componentWillReceiveProps and componentWillUnmount

vonovak avatar Sep 06 '17 13:09 vonovak