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

Checkbox behavior inexplicable when preventDefault() is used #3446

Closed
ghost opened this issue Mar 18, 2015 · 5 comments
Closed

Checkbox behavior inexplicable when preventDefault() is used #3446

ghost opened this issue Mar 18, 2015 · 5 comments

Comments

@ghost
Copy link

ghost commented Mar 18, 2015

var React = require('react'),
    Backbone = require('backbone'),
    Flux = require('flux'),
    dispatcher = new Flux.Dispatcher();

var storeListeningMixin = {
  componentDidMount: function () {
    if (this.props.store && this.onStoreChanged) {
      this.props.store.on('change:emit', this.onStoreChanged, this);
    }
  },
  componentWillUnmount: function () {
    if (this.props.store && this.onStoreChanged) {
      this.props.store.off('change:emit', this.onStoreChanged);
    }
  }
};

var dispatchingMixin = {
  componentWillMount: function () {
    this.dispatch = function (payload) {
      dispatcher.dispatch(payload);
    };
  }
};

function register(store) {
  store.dispatcher = dispatcher;
  store.dispatchId = dispatcher.register(store.onDispatch.bind(store));
}

function unregister(store) {
  delete store.dispatcher;
  dispatcher.unregister(store.dispatchId);
}


var Store = Backbone.Model.extend({
  checked: [false, false],
  onDispatch: function (payload) {
    if (payload.actionType === 'ha') {
      console.log('dispatch received for checkbox' + payload.ind + '.');
      this.checked[payload.ind] = !this.checked[payload.ind];
      this.trigger('change:emit');
    }
  }
});

var store = new Store();

var Checkboxa = React.createClass({
    mixins: [dispatchingMixin],
    onChange: function(e) {
      console.log('dispatching...');
      e.preventDefault();  // This makes the checkbox behave weirdly - if removed things are normal
      this.dispatch({actionType: 'ha', ind: this.props.ind});
    },
    render: function() {
      var checked = this.props.store.checked[this.props.ind];
      console.log('checkbox rendering, should be checked: ' + checked);
      return (
        <li><input type="checkbox" checked={checked} key={this.props.ind}
             onChange={this.onChange}/><span>{this.props.ind}</span></li>
      );
    }
});

var CheckboxaList = React.createClass({
  mixins: [storeListeningMixin],
  onStoreChanged: function() {
    this.forceUpdate();
  },
  render: function() {
    return (
      <ul>
        <Checkboxa ind={0} key={0} store={this.props.store} />
        <Checkboxa ind={1} key={1} store={this.props.store} />
      </ul>
    );
  }
});

register(store);

React.render(
  <CheckboxaList store={store}/>,
  document.getElementById('renderArea')
);

Notice how clicking the checkbox does not make React render it according to the checked state, but clicking the other checkbox causes the checkbox to re-render correctly.
In theory the preventDefault() happened when the component was dispatching and shouldn't affect the rendering? In any case the component shouldn't be rendered incorrectly?

@jquense
Copy link
Contributor

jquense commented Mar 18, 2015

this is because under the hood react listens for Click events instead of Change to normalize browser differences. The accepted solution is: don't use preventDefault (use stopPropagation instead) or make use of a timeout. more reading here #3005

@ghost
Copy link
Author

ghost commented Mar 18, 2015

@jquense Can you maybe detail exactly what is going on? The effect is still observed when you use onClick directly. Can you maybe give me a flow of how this is happening?

@jquense
Copy link
Contributor

jquense commented Mar 18, 2015

@tigergrid you can read the issue I linked, zpao covers it quite nicely. #3005 (comment)

essentially you are running into a limitation of how react implements the onChange event for checkboxes

@zpao
Copy link
Member

zpao commented Mar 18, 2015

This is unfortunate but the way it is for now. If you don't have a good reason for using preventDefault(), don't.

@zpao zpao closed this as completed Mar 18, 2015
sersorrel added a commit to wsi-cogs/frontend that referenced this issue Jul 30, 2019
The problem: if the user clicked on a checkbox in the
MultiselectDropDown (rather than clicking on the text adjacent to the
checkbox), the state of the checkbox would appear visually to stay the
same, but everything else -- its props, the parent element's state, etc.
-- correctly believed that the checkbox has been toggled.

To fix this, we force an update of the entire component every time a
checkbox is clicked, which rerenders the checkbox and results in the
visual appearance matching the React/DOM state again. Both the call to
setTimeout and the two arrow functions are required; without the arrow
functions, various React code doesn't get the correct `this`, and
without the setTimeout, it Just Doesn't Work.

It's unclear exactly why this works. There are several issues in the
React repo and elsewhere describing similar behaviour:

facebook/react#2766
facebook/react#3005
facebook/react#3446
react-bootstrap/react-bootstrap#2485

Unfortunately, none of the suggestions made appear to be directly
applicable to this situation.

This workaround is bad for two reasons:
- it's ugly and nonobvious
- it forces a rerender of far too many components

Nevertheless, it fixes the problem.

Closes #1.
ademidun added a commit to ademidun/atila-client-web-app that referenced this issue Sep 28, 2019
@skipjack
Copy link

Man, 8 years later and I'm still seeing the same issue. Messed with my head for a solid hour or two, seems like onMouseDown for the input and preventDefault in a container's onClick are one way around it if you must use preventDefault. I initially thought it was something quirky in terms of state management or re-rendering on our end.

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

No branches or pull requests

3 participants