8

Im having an issue. When you click listItem (whole separated li element) I want to call the onChange function on the checkbox component inside listItem.

I could easily move that function from checkbox to parent but I will loose the checked prop.

Checking the checkbox itself doesnt work, but just open the console to see the results. I want handleToggle function to fire properly when whole element is clicked (not only checkbox)

  <List>
    {['first', 'second', 'third'].map((name, i) => (
      <ListItem key={i} dense button>
        <ListItemText primary={name} />
        <Checkbox
          disableRipple
          checked={false}
          onChange={(evt, checked) => this.handleToggle(evt, checked, name)}
        />
      </ListItem>
    ))}
  </List>

Code SandBox

Edit

I don't want to use state in this component at all. Summarizing - how to pass event from ListItem (parent) to it's children (Checkbox)?

Final edit: I've found out the way how to deal with it. No state needed. Just two simple lines of code.

Since checkbox state is fully controlled by redux, I just moved the onClick func to the ListItem element with one line on code in it's body:

...dispatch(toggle(!this.props.elements[nameOfElement], name));

Anyways thanks to everyone for help. Upvoted every answer.

T Doe
  • 91
  • 1
  • 4
  • Please put your [mcve] **in** the question instead of just linking it. Ideally, make it **runnable** using Stack Snippets (the `[<>]` toolbar button). Stack Snippets support React, including JSX; [here's how to do one](http://meta.stackoverflow.com/questions/338537/). – T.J. Crowder Dec 02 '17 at 13:47
  • @T.J.Crowder Does code snippets support dependencies my dear friend? – T Doe Dec 02 '17 at 14:06
  • Yes -- again, [here's how to do one](http://meta.stackoverflow.com/questions/338537/). Note the **Add an external library** button. – T.J. Crowder Dec 02 '17 at 14:12
  • @T.J.Crowder Is there a link somewhere to whole material ui library? I couldn't find it my friend. – T Doe Dec 02 '17 at 14:20
  • Probably (you can check on https://cdnjs.com/), but please read the [mcve] page I linked. I can't imagine you actually need the Material-UI library for an MCVE demonstrating the problem. – T.J. Crowder Dec 02 '17 at 14:22
  • @T.J.Crowder Im afraid that I need library to properly demonstrate the problem my dear friend. – T Doe Dec 02 '17 at 14:28
  • 1
    To demonstrate a problem passing a click on an `li` to a subordinate component? No, you don't. Think **Minimal**. (But again, it's entirely likely you can find it hosted online.) – T.J. Crowder Dec 02 '17 at 14:32
  • Please make your final edit an answer so that it is easier to find for other people. – tech4him Dec 11 '17 at 03:37

5 Answers5

3

Roby, beat me too it, but I handled the state change slightly differently.

The idea is to manage the state (checked on not) of all the check boxes in the parents state. Then you have a handleToggle function in the parent that will update the parents state with the checked value of all the checkboxes.

This state is then passed to each checkbox as a prop.

Also, it is not a good idea to use the index from map as a key in the ListItem. If you add and remove items from the list, React will get confused as to which item is which.

Here is the code:

import React from "react";

import { render } from "react-dom";
import Hello from "./Hello";

import List, {
  ListItem,
  ListItemSecondaryAction,
  ListItemText
} from "material-ui/List";
import Checkbox from "material-ui/Checkbox";

const styles = {
  fontFamily: "sans-serif",
  textAlign: "center"
};

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      list: [],
      listChecked: []
    };
  }

  componentWillMount() {
    this.setState({
      list: ["first", "second", "third"],
      listChecked: [{ first: false }, { second: false }, { third: false }]
    });
  }

  handleToggle = evt => {
    console.log(evt.target.checked);
    const name = evt.target.name;
    this.setState({ name: !this.state.listChecked[name]})
    // this.props.dispatch(x(checked, name));
  };

  render() {
    const { list } = this.state;
    return (
      <List>
        {list.map((name, i) => (
          <ListItem key={name} dense button>
            <ListItemText primary={name} />
            <Checkbox
              disableRipple
              checked={this.state.listChecked[name]}
              onChange={this.handleToggle}
            />
          </ListItem>
        ))}
      </List>
    );
  }
}

render(<App />, document.getElementById("root"));

and the CodeSandbox Example

bluesixty
  • 2,367
  • 3
  • 21
  • 21
  • I wish I could upvote, thanks for it but still it's a workaround, I dont want to use state in that component at all :( – T Doe Dec 02 '17 at 14:28
  • @bluesixty I think you forgot to save your codesandbox - it's still the OP's sample - and another thing I noticed, it's good habit to use `this.setState(...)` since it's the react way, otherwise I guess it's a different version of the same story – Andrei Roba Dec 02 '17 at 14:38
  • Thanks @Roby, saved now. I also updated to setState in toggle method. Good catch. – bluesixty Dec 02 '17 at 14:44
  • @TDoe, Why don't you want state in this component? Are you using Redux or Flux? Perhaps a little more info would help us help you. – bluesixty Dec 02 '17 at 14:46
  • Yeah, im using redux here. – T Doe Dec 02 '17 at 14:48
  • @TDoe, sorry can't help much with Redux, I haven't used it much. But I believe the solution is still the same. Store the current value of the checkbox in your store, use handleToggle to update the store, and props to set checkbox value. – bluesixty Dec 02 '17 at 15:10
3

One way to tackle this issue might be to use React refs to keep a reference of the child <ListItem/>, and probably use an uncontrolled component in order to detach your output from state updates, that is in this case, replace <Checkbox /> with <input type="checkbox"/>.

Checkboxes would then be updated either directly from the DOM element itself using onChange on <input />, or through React using onClick on the <ListIem/> that references the <input /> DOM element.

...
class App extends React.Component {
  constructor(props) {
    super(props);
    this.checkboxes = [
      {
        name: "first",
        checked: false
      },
      {
        name: "second",
        checked: false
      },
      {
        name: "third",
        checked: false
      }
    ];
  }

  handleListItemClicked = (evt, name) => {
    console.log("ListItem clicked :", name);
    this.checkboxes[name].checked = !this.checkboxes[name].checked;
  };

  handleInputChanged = (evt, name) => {
    console.log("input changed, evt.target :", evt.target);
    evt.target.checked = !evt.target.checked;
  };

  render() {
    return (
      <List>
        {this.checkboxes.map(({ name }, i) => (
          <div>
            <ListItem
              key={i}
              onClick={evt => this.handleListItemClicked(evt, name)}
              dense
              button
            >
              <ListItemText primary={name} />
              <input
                type="checkbox"
                name={name}
                ref={checkbox => {
                  this.checkboxes[name] = checkbox;
                }}
                onChange={evt => this.handleInputChanged(evt, name)}
              />
            </ListItem>
          </div>
        ))}
      </List>
    );
  }
}

...

Here is a fork of your initial Code Sandbox : https://codesandbox.io/s/mox93j6nry

Hope this helps!

Philippe Sultan
  • 2,111
  • 17
  • 23
  • It still uses state and its not dynamic and flexible, just hardcoded :( But thank u anyways! – T Doe Dec 08 '17 at 00:21
  • Yes, I realised this did not match with your requirements. I've updated my answer to not use the state, and replaced the `` with an ``. I hope that will at least provide you with a lead to solve this issue. – Philippe Sultan Dec 08 '17 at 14:14
3

Since you are planning to use Redux, I would suggest a simplest implementation of your case in terms of React Redux.

Reducer

const inititalState = {
  list: [ // keep all your data in redux store
    { id: 1, name: 'first', checked: false },
    { id: 2, name: 'second', checked: false },
    { id: 3, name: 'third', checked: false }
  ]
};

export function list(state = inititalState, action) {
  switch (action.type) {
    case 'CHANGE': // changing the list of items (checked property)
      return {
        ...state,
        list: state.list.map(i => 
          i.id === action.item.id ? { ...action.item, checked: action.value } : i
        ) 
      };
    default:
      return state
  }
}

Action

// just the only action to change 'checked' property of a given item in the list
export function change(item, value) {
  return { type: 'CHANGE', item, value };
}

At last, the Component connected with redux store:

class App extends React.Component {
  handleToggle = (item) => {
    // dispatching the "change" redux-action
    this.props.dispatch(change(item, !item.checked)); // here's the toggling logic (!)
  }

  render() {
    return (
      <List>
        {this.props.list.map((item, i) => ( // map the list from the redux store
          <ListItem key={item.id} dense button // (i)ndex for the key value would be ok too
            onClick={() => this.handleToggle(item)}> // to call the redux-action by click
            <ListItemText primary={item.name} />
            <Checkbox
              disableRipple
              checked={item.checked} // this is the main binding
            />                       // with no need of onChange handling
          </ListItem>
        ))}
      </List>
    )
  }
};

// pass the list from the redux-store to the component
function mapStateToProps(state, ownProps) {
  return { list: state.list };
}

const _App = connect(mapStateToProps)(App); // connect the component to the redux store

This is very basic implementation, I'd recommend to follow best practices, move App component out of index.js, the state logic (!item.checked) could be placed in the action creator or reducer etc... Also, I created a working demo based on your codesandbox: https://codesandbox.io/s/xpk099y40w.

dhilt
  • 18,707
  • 8
  • 70
  • 85
  • Thanks! So far the best answer, but still too heavy imo and lil bit too complicated :/ – T Doe Dec 09 '17 at 00:33
  • @TDoe That's the Redux, it takes some effort to catch the essence, but in the end you'll get a powerful instrument for the front-end developing. – dhilt Dec 09 '17 at 09:42
  • 1
    @TDoe I believe this is a very elegant and simple solution and it should also help you start with Redux. I'm upvoting. Mine is React only (and it loses the design of the `` component), however, trying to solve this issue made me learn new things, thanks guys! – Philippe Sultan Dec 10 '17 at 15:45
2

You don't really need to pass the checked prop to your parent. You could instead keep track of the checked rows in your component state:

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      checked: []
    }
  }

  isChecked(name) {
    return this.state.checked.indexOf(name) > -1
  }

  handleToggle = (evt, name) => {
    if (this.isChecked(name)) {
      this.setState({
        checked: this.state.checked.filter(i => i !== name)
      })
    } else {
      this.setState({
        checked: [...this.state.checked, name]
      })
    }
  }

  render() {
    return (
      <List>
        {['first', 'second', 'third'].map((name, i) => (
          <ListItem key={i}
            onClick={evt => this.handleToggle(evt, name)} dense button>
            <ListItemText primary={name} />
            <Checkbox
              disableRipple
              label={name}
              checked={this.isChecked(name)}
            />
          </ListItem>
        ))}
      </List>
    )
  }
};

Working sample.

Andrei Roba
  • 2,156
  • 2
  • 16
  • 33
  • Thank you, it works fine. However it is not what Im looking for (it's a workaround to the problem) – T Doe Dec 02 '17 at 14:21
0

Looks like you just want to toggle the checkbox when the associated text is clicked.

All you need to do is either wrap everything in a <label /> tag or just wrap the text in the <label htmlFor="inputId" /> tag with the appropriate htmlFor attribute:

No javascript magic is required.

timetowonder
  • 5,121
  • 5
  • 34
  • 48