Skip to content

refactor for v2 #147

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.

Sign up for GitHub

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

Closed
wants to merge 20 commits into from
Closed

refactor for v2 #147

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a658267
refactor for v2
May 1, 2019
7d16e88
fix lint errors
May 14, 2019
bd7cfdf
fix noCascade and disable problem
May 15, 2019
a49f9f0
fix or temporarily disable tests to allow build
May 15, 2019
3dba178
fix scss and less to pass linter for build
May 15, 2019
e4d7084
Revert "fix scss and less ..." -broke travis, worked locally
May 15, 2019
460af8c
merge v1.6-dev
May 28, 2019
e41fab8
minor changes from review
May 28, 2019
a5e7b4b
fix radio/onlyLeafNode problem, add comments
May 29, 2019
53f0876
remove id from PropTypes to pass eslint in build
May 29, 2019
c6340fb
fix disabled status for radio nodes
Jun 5, 2019
7695036
fix comments
Jun 5, 2019
e108df5
Revert "fix comments - stuff included by mistake"
Jun 5, 2019
da8fcf2
fix comments
Jun 5, 2019
90e5125
first try
Jun 5, 2019
ab48b5d
remove duplicate logic
Jun 6, 2019
4f6ed9f
add useCheckedArray prop and fix examples
Jun 6, 2019
8deb130
Merge branch 'add_checkedArray' into v2.0-proposal
Jun 6, 2019
1e146e7
remove console statements
Jun 6, 2019
d3138f0
remove useCheckedArray from BasicExampleObject
Jun 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion examples/src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,22 @@ <h2 class="project-tagline">A simple and elegant checkbox tree for React</h2>
</p>

<h1>Examples</h1>
<h2>Basic Example</h2>

<h2>Props Demo</h2>
<div id="props-demo-example"></div>

<h2>Basic Example - array input</h2>
<p>
The nodes prop for the CheckboxTree is provided as an array of child nodes of the root of the tree.
</p>
<div id="basic-example"></div>

<h2>Basic Example - object input</h2>
<p>
The nodes prop for the CheckboxTree is provided as the root node object instead of an array of child nodes of the root of the tree.
</p>
<div id="basic-example-object"></div>

<h2>Custom Icons Example</h2>
<div id="custom-icons-example"></div>

Expand Down
6 changes: 6 additions & 0 deletions examples/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from 'react';
import ReactDOM from 'react-dom';

import PropsDemoExample from './js/PropsDemoExample';
import BasicExample from './js/BasicExample';
import BasicExampleObject from './js/BasicExampleObject';
import CustomIconsExample from './js/CustomIconsExample';
import ClickableLabelsExample from './js/ClickableLabelsExample';
import DisabledExample from './js/DisabledExample';
Expand All @@ -11,7 +13,11 @@ import NoCascadeExample from './js/NoCascadeExample';
import LargeDataExample from './js/LargeDataExample';
import PessimisticToggleExample from './js/PessimisticToggleExample';

ReactDOM.render(<PropsDemoExample />, document.getElementById('props-demo-example'));

ReactDOM.render(<BasicExample />, document.getElementById('basic-example'));
ReactDOM.render(<BasicExampleObject />, document.getElementById('basic-example-object'));

ReactDOM.render(<CustomIconsExample />, document.getElementById('custom-icons-example'));
ReactDOM.render(<DisabledExample />, document.getElementById('disabled-example'));
ReactDOM.render(<NoCascadeExample />, document.getElementById('no-cascade-example'));
Expand Down
87 changes: 62 additions & 25 deletions examples/src/js/BasicExample.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
import React from 'react';
import CheckboxTree from 'react-checkbox-tree';

const nodes = [
const initialNodes = [
{
value: '/app',
label: 'app',
expanded: true,
children: [
{
value: '/app/Http',
label: 'Http',
expanded: true,
children: [
{
value: '/app/Http/Controllers',
label: 'Controllers',
expanded: true,
children: [{
value: '/app/Http/Controllers/WelcomeController.js',
label: 'WelcomeController.js',
checked: true,
}],
},
{
Expand All @@ -34,6 +38,55 @@ const nodes = [
},
],
},
{
value: '/radioGroup',
label: 'RadioTest',
expanded: true,
radioGroup: true,
children: [
{
value: 'radio1',
label: 'radio1',
},
{
value: 'radio2',
label: 'radio2',
children: [
{
value: 'radio2-1',
label: 'radio2',
},
{
value: 'radio2-2',
label: 'radio2-2',
},
{
value: 'radio2-3',
label: 'radio2-3',
},
],
},
{
value: 'radio3',
label: 'radio3',
radioGroup: true,
children: [
{
value: 'radio3-1',
label: 'radio3',
},
{
value: 'radio3-2',
label: 'radio3-2',
},
{
value: 'radio3-3',
label: 'radio3-3',
},
],
},
],
},
{
value: '/config',
label: 'config',
Expand Down Expand Up @@ -82,40 +135,24 @@ const nodes = [

class BasicExample extends React.Component {
state = {
checked: [
'/app/Http/Controllers/WelcomeController.js',
'/app/Http/routes.js',
'/public/assets/style.css',
'/public/index.html',
'/.gitignore',
],
expanded: [
'/app',
],
nodes: initialNodes,
};

constructor(props) {
super(props);

this.onCheck = this.onCheck.bind(this);
this.onExpand = this.onExpand.bind(this);
}

onCheck(checked) {
this.setState({ checked });
onCheck = (node, nodes) => {
this.setState({ nodes });
}
Copy link
Owner

@jakezatecky jakezatecky May 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
};

Any assignment operator should be terminated by a semicolon.

Copy link
Contributor Author

@worthlutz worthlutz May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix these. They are not caught by eslint.


onExpand(expanded) {
this.setState({ expanded });
onExpand = (node, nodes) => {
this.setState({ nodes });
}

render() {
const { checked, expanded } = this.state;
const { nodes } = this.state;

return (
<CheckboxTree
checked={checked}
expanded={expanded}
style={{ flex: '1' }}
Copy link
Owner

@jakezatecky jakezatecky May 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no style property for the main component and I would be extremely averse to adding such a property.

Copy link
Contributor Author

@worthlutz worthlutz May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That style prop is not a <CheckboxTree> prop. It is a prop of React.component which checkbox tree extends. I understand you not wanting to use it. I have tried to modify the css in the actual component. I have used it in the demos for fast testing. I can clean this up and add separate css files for the demos.

checkModel="all"
iconsClass="fa5"
nodes={nodes}
onCheck={this.onCheck}
Expand Down
170 changes: 170 additions & 0 deletions examples/src/js/BasicExampleObject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import React from 'react';
import CheckboxTree from 'react-checkbox-tree';

const initialNodes = {
label: 'Root',
Copy link
Owner

@jakezatecky jakezatecky May 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see the point in supporting a base object file. It makes the internals slightly more complicated and it is not too much to ask for the user just to have a top-level array. Also, it does not seem like this is actually doing much?

Copy link
Contributor Author

@worthlutz worthlutz May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I put in based upon my use and something I read in one of the issues somewhere. I will state my reasoning here and you can decide the path forward as this is your component.

In my use I may have several JSON configuration files which are used for input to the CheckTree. Each has a root node with a description of that tree. The User logs in and can choose from the configurations available to them. My current proposal allows passing this root node object in as the nodes prop. In this case, the root node is the same type object as as each child node object. I can achieve the same result by passing the root node's children array as you originally set it up.

I have also allowed the nodes prop to be passed in as an array. It is then put into a generated root object so that the processing could be handled the same as when a root object is passed in as described above.

One advantage of having the root object is that you can then add a prop, showRoot, to allow having a single node at the top of the tree. This allows you to collapse the tree to one line.

By defaulting showRoot to false you will have the same result as you originally provided. I have not yet added this prop but had intended to.

The showRoot prop will fix some of the concerns in issue #143. With showRoot === true and 'noCascade === false you do not need a 'checkAll or 'unCheckAll`. Just check or uncheck the root.

That is my case for adding the capability to input a root node. I have kept the ability to input an array of nodes for compatibility with previous versions. I can set it up with only the root node option, only the array of nodes option or both. You choose.

value: '*root*',
expanded: true,
children: [
{
value: '/app',
label: 'app',
expanded: true,
children: [
{
value: '/app/Http',
label: 'Http',
expanded: true,
children: [
{
value: '/app/Http/Controllers',
label: 'Controllers',
expanded: true,
children: [{
value: '/app/Http/Controllers/WelcomeController.js',
label: 'WelcomeController.js',
checked: true,
}],
},
{
value: '/app/Http/routes.js',
label: 'routes.js',
},
],
},
{
value: '/app/Providers',
label: 'Providers',
children: [{
value: '/app/Providers/EventServiceProvider.js',
label: 'EventServiceProvider.js',
}],
},
],
},
{
value: '/radioGroup',
label: 'RadioTest',
expanded: true,
radioGroup: true,
children: [
{
value: 'radio1',
label: 'radio1',
},
{
value: 'radio2',
label: 'radio2',
children: [
{
value: 'radio2-1',
label: 'radio2',
},
{
value: 'radio2-2',
label: 'radio2-2',
},
{
value: 'radio2-3',
label: 'radio2-3',
},
],
},
{
value: 'radio3',
label: 'radio3',
radioGroup: true,
children: [
{
value: 'radio3-1',
label: 'radio3',
},
{
value: 'radio3-2',
label: 'radio3-2',
},
{
value: 'radio3-3',
label: 'radio3-3',
},
],
},
],
},
{
value: '/config',
label: 'config',
children: [
{
value: '/config/app.js',
label: 'app.js',
},
{
value: '/config/database.js',
label: 'database.js',
},
],
},
{
value: '/public',
label: 'public',
children: [
{
value: '/public/assets/',
label: 'assets',
children: [{
value: '/public/assets/style.css',
label: 'style.css',
}],
},
{
value: '/public/index.html',
label: 'index.html',
},
],
},
{
value: '/.env',
label: '.env',
},
{
value: '/.gitignore',
label: '.gitignore',
},
{
value: '/README.md',
label: 'README.md',
},
],
};

class BasicExampleObject extends React.Component {
state = {
nodes: initialNodes,
};

onCheck = (node, nodes) => {
this.setState({ nodes });
}

onExpand = (node, nodes) => {
this.setState({ nodes });
}

render() {
const { nodes } = this.state;

return (
<CheckboxTree
style={{ flex: '1' }}
checkModel="all"
iconsClass="fa5"
nodes={nodes}
onCheck={this.onCheck}
onExpand={this.onExpand}
/>
);
}
}

export default BasicExampleObject;
Loading