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

support for iterable structures within state #79

Merged
merged 5 commits into from
Aug 23, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
141 changes: 141 additions & 0 deletions src/react/JSONTree/JSONIterableNode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import React from 'react';
import reactMixin from 'react-mixin';
import { ExpandedStateHandlerMixin } from './mixins';
import JSONArrow from './JSONArrow';
import grabNode from './grab-node';

const styles = {
base: {
position: 'relative',
paddingTop: 3,
paddingBottom: 3,
paddingRight: 0,
marginLeft: 14
},
label: {
margin: 0,
padding: 0,
display: 'inline-block'
},
span: {
cursor: 'default'
},
spanType: {
marginLeft: 5,
marginRight: 5
}
};

@reactMixin.decorate(ExpandedStateHandlerMixin)
export default class JSONIterableNode extends React.Component {
defaultProps = {
data: [],
initialExpanded: false
};

// flag to see if we still need to render our child nodes
needsChildNodes = true;

// cache store for our child nodes
renderedChildren = [];

// cache store for the number of items string we display
itemString = false;

constructor(props) {
super(props);
this.state = {
expanded: this.props.initialExpanded,
createdChildNodes: false
};
}

// Returns the child nodes for each element in the array. If we have
// generated them previously, we return from cache, otherwise we create
// them.
getChildNodes() {
if (this.state.expanded && this.needsChildNodes) {
let childNodes = [];
for (const entry of this.props.data) {
let key = null;
let value = null;
if (Array.isArray(entry)) {
[key, value] = entry;
} else {
key = childNodes.length;
value = entry;
}

let prevData;
if (typeof this.props.previousData !== 'undefined') {
prevData = this.props.previousData[key];
}
const node = grabNode(key, value, prevData, this.props.theme);
if (node !== false) {
childNodes.push(node);
}
}
this.needsChildNodes = false;
this.renderedChildren = childNodes;
}
return this.renderedChildren;
}

// Returns the "n Items" string for this node, generating and
// caching it if it hasn't been created yet.
getItemString() {
if (!this.itemString) {
const { data } = this.props;
let count = 0;
if (typeof data.count === 'function') {
count = data.count();
} else {
count = data.length;
}
this.itemString = count + ' entr' + (count !== 1 ? 'ies' : 'y');
Copy link

Choose a reason for hiding this comment

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

Change to the following

      if (typeof data.count === 'function') {
        count = data.count();
      } else if (typeof data.size !== 'undefined') {
        count = data.size;
      } else {
        count = data.length;
      }

To handle the length of a Map()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree. I had the size there before, but then I've noticed that MoriJS has only count method.

I am thinking if it wouldn't be better to use iterator even here to count entries. That would be unified way. I would like to avoid adding another property/method in a future if some other data structure implementing iterator would use different name for this. Some library could even have a like size being method or count being property.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any conclusion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I've change it. It uses the size property as it's the most common (only mori doesn't have it), otherwise it counts entries through the iterator. I am not using length because it's deprecated in Immutable anyway and others doesn't have it at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another small change to use Number.isSafeInteger for the size property. It should be safer way in case some library has size as a function.

}
return this.itemString;
}

render() {
const childNodes = this.getChildNodes();
const childListStyle = {
padding: 0,
margin: 0,
listStyle: 'none',
display: (this.state.expanded) ? 'block' : 'none'
};
let containerStyle;
let spanStyle = {
...styles.span,
color: this.props.theme.base0E
};
containerStyle = {
...styles.base
};
if (this.state.expanded) {
spanStyle = {
...spanStyle,
color: this.props.theme.base03
};
}
return (
<li style={containerStyle}>
<JSONArrow theme={this.props.theme} open={this.state.expanded} onClick={::this.handleClick}/>
<label style={{
...styles.label,
color: this.props.theme.base0D
}} onClick={::this.handleClick}>
{this.props.keyName}:
</label>
<span style={spanStyle} onClick={::this.handleClick}>
<span style={styles.spanType}>()</span>
{this.getItemString()}
</span>
<ol style={childListStyle}>
{childNodes}
</ol>
</li>
);
}
}
3 changes: 3 additions & 0 deletions src/react/JSONTree/grab-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import objType from './obj-type';
import JSONObjectNode from './JSONObjectNode';
import JSONArrayNode from './JSONArrayNode';
import JSONIterableNode from './JSONIterableNode';
import JSONStringNode from './JSONStringNode';
import JSONNumberNode from './JSONNumberNode';
import JSONBooleanNode from './JSONBooleanNode';
Expand All @@ -13,6 +14,8 @@ export default function(key, value, prevValue, theme) {
return <JSONObjectNode data={value} previousData={prevValue} theme={theme} keyName={key} key={key} />;
} else if (nodeType === 'Array') {
return <JSONArrayNode data={value} previousData={prevValue} theme={theme} keyName={key} key={key} />;
} else if (nodeType === 'Iterable') {
return <JSONIterableNode data={value} previousData={prevValue} theme={theme} keyName={key} key={key} />;
} else if (nodeType === 'String') {
return <JSONStringNode keyName={key} previousValue={prevValue} theme={theme} value={value} key={key} />;
} else if (nodeType === 'Number') {
Expand Down
6 changes: 5 additions & 1 deletion src/react/JSONTree/obj-type.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
export default function(obj) {
return Object.prototype.toString.call(obj).slice(8, -1);
const type = Object.prototype.toString.call(obj).slice(8, -1);
if (type === 'Object' && typeof obj[Symbol.iterator] === 'function') {
Copy link

Choose a reason for hiding this comment

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

When using the babel Map polyfill, the .toString() returns 'Map'.

Change to

  if ((type === 'Object' || type === 'Map') && typeof obj[Symbol.iterator] === 'function') {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure this is correct approach. There might be even more iterable types returning its own toString representation.

Originally I had there just check for iterator existence, but that was wrong for String which implements iterator for its characters. Similar goes for the Array.

It might be safer bet to just check typeof obj === 'object' and excluding array from it. Any other ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be safer bet to just check typeof obj === 'object' and excluding array from it. Any other ideas?

Fair.

return 'Iterable';
}
return type;
}