Skip to content

Make components stateless #12

Closed
Closed
@williamboman

Description

@williamboman
Contributor

Would make data direction much clearer. Currently the component hinges on working against object references passed as props.

So, something like this;

<CheckboxTree
  nodes={this.state.nodes}
  checked={this.state.checked}
  collapsed={this.state.collapsed}
  onCollapse={this.onCollapse}
  onCheck={this.onCheck} />

where

onCollapse(item) {
  // return // uncommenting would disable collapsing functionality entirely
  this.setState({
    collapsed: [item, ...this.state.collapsed]
  })
}

onCheck(item) {
  this.setState({
    checked: [item, ...this.state.checked]
  })
}

Had some issues figuring this component out, took quite some time to debug an issue I had with breaking object references (see #10). I think this is the more idiomatic way of doing reusable components.

@jakezatecky Thoughts? Would you have time to do such a refactor should you agree?

Activity

jakezatecky

jakezatecky commented on Oct 10, 2016

@jakezatecky
Owner

I definitely agree that this should be made stateless and I agree that the data flow is sometimes...unclear. This is something that I was intending to work on. I have some available toward the end of this week so I will slate some time for this and other changes around there.

williamboman

williamboman commented on Oct 10, 2016

@williamboman
ContributorAuthor

That would be absolutely awesome. I don't have time to work on this, but holla at me if you need help with anything.

added this to the v0.3.0 milestone on Oct 10, 2016
added a commit that references this issue on Oct 14, 2016
jakezatecky

jakezatecky commented on Oct 14, 2016

@jakezatecky
Owner

I was a bit on a time-crunch for some other work, but the component is now stateless. Needs to be cleaned up a bit before being merged into master. Will finish it up within the week.

  • Clean mutations of the checked property
    Provide example in README
williamboman

williamboman commented on Nov 10, 2016

@williamboman
ContributorAuthor

It still mutates the checked and expanded props, doesn't it? I'm running the feature branch but still getting unexpected behavior. Bypassing it by;

const {expanded, checked} = this.state
<CheckboxTree
    expanded={[...expanded]}
    checked=([...checked]}
    onCheck={checked => this.setState({checked})
    onExpand={expanded => this.setState({expanded}) />
added a commit that references this issue on Jan 14, 2017
b6715c0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @jakezatecky@williamboman

        Issue actions

          Make components stateless · Issue #12 · jakezatecky/react-checkbox-tree