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.

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

Jump to bottom

Refactor/component-types #212

Merged
merged 6 commits into from
May 6, 2016
Merged

Refactor/component-types #212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a43e648
Refactor createClass and child components to a simpler API
Apr 30, 2016
27ba48d
Code cleanup for PR #212
May 2, 2016
cc82e23
Merge branch 'release/1.0.0' into refactor/component-types
May 5, 2016
b65ee7d
Don't run text fields through build hybrid by default
jondlm May 5, 2016
aa8558c
Merge branch 'release/1.0.0' into refactor/component-types
May 6, 2016
bde4b1c
reapply changes from b65ee7dec9d420623e8c875ca53e21c105bec31b that were
May 6, 2016
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
2 changes: 1 addition & 1 deletion src/components/Autocomplete/Autocomplete.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import _ from 'lodash';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass } from '../../util/component-types';
import * as reducers from './Autocomplete.reducers';
import * as KEYCODE from '../../constants/key-code';
import DropMenu from '../DropMenu/DropMenu';
Expand Down
3 changes: 2 additions & 1 deletion src/components/Autocomplete/Autocomplete.spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mount, shallow } from 'enzyme';
import assert from 'assert';
import sinon from 'sinon';
import describeWithDOM from '../../util/describe-with-dom';
import { findTypes } from '../../util/component-types';
import _ from 'lodash';
import { common } from '../../util/generic-tests';
import Autocomplete from './Autocomplete';
Expand Down Expand Up @@ -59,7 +60,7 @@ describe('Autocomplete', () => {
/>
);

const options = DropMenu.Option.findInAllAsProps(wrapper.find('DropMenu').props());
const options = _.map(findTypes(wrapper.find(DropMenu).props(), DropMenu.Option), 'props');

assert.equal('Portland', options[0].children);
assert.equal('portal', options[1].children);
Expand Down
2 changes: 1 addition & 1 deletion src/components/Badge/Badge.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass } from '../../util/component-types';

const boundClassNames = lucidClassNames.bind('&-Badge');

Expand Down
2 changes: 1 addition & 1 deletion src/components/Banner/Banner.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from 'lodash';
import React from 'react';
import ReactCSSTransitionGroup from 'react-addons-css-transition-group';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass } from '../../util/component-types';
import DangerIcon from '../Icon/DangerIcon/DangerIcon';
import InfoIcon from '../Icon/InfoIcon/InfoIcon';
import SuccessIcon from '../Icon/SuccessIcon/SuccessIcon';
Expand Down
3 changes: 2 additions & 1 deletion src/components/Button/Button.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import ReactDOM from 'react-dom';
import _ from 'lodash';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass } from '../../util/component-types';
const boundClassNames = lucidClassNames.bind('&-Button');

const {
Expand All @@ -24,6 +24,7 @@ const {
*/
const Button = createClass({
displayName: 'Button',
propName: 'Button',
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes it so that when u call findTypes(props, Button) it will also look for the prop Button on the props object in addition to props.children.

propTypes: {
/**
* disables the button by greying it out
Expand Down
13 changes: 8 additions & 5 deletions src/components/ButtonGroup/ButtonGroup.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from 'lodash';
import Button from '../Button/Button';
import React from 'react';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass, findTypes } from '../../util/component-types';
import reducers from './ButtonGroup.reducers';

const boundClassNames = lucidClassNames.bind('&-ButtonGroup');
Expand All @@ -24,8 +24,11 @@ const {
const ButtonGroup = createClass({
displayName: 'ButtonGroup',

childProps: {
Button: { ...Button.propTypes }
components: {
Button: createClass({
Copy link
Contributor

Choose a reason for hiding this comment

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

can you not just use a reference to the Button component here?

Copy link
Contributor

@sodiumjoe sodiumjoe May 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

er, i guess not since it has a render fn... this part seems a little weird

seems like it would be good to be able to get at least propTypes off of Button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right we should reference it as is right here instead of creating a new type, but the goal of this change was a 1:1 change in how child components are defined and filtered, we should have another PR that cleans up any redundancies that we identify with child components

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

won't it be weird since Button has a render function, tho

displayName: 'ButtonGroup.Button',
propName: 'Button'
})
},

reducers: reducers,
Expand Down Expand Up @@ -70,7 +73,7 @@ const ButtonGroup = createClass({

handleSelect({ event, props: childProps }) {
const { callbackId } = childProps;
const clickedButtonProps = ButtonGroup.Button.findInAllAsProps(this.props)[callbackId];
const clickedButtonProps = findTypes(this.props, ButtonGroup.Button)[callbackId];

// If the consumer passed in an `onClick` to the child `ButtonGroup.Button`
// component, we should make sure to call that in addition to the
Expand All @@ -90,7 +93,7 @@ const ButtonGroup = createClass({
...others
} = this.props;

const buttonChildProps = ButtonGroup.Button.findInAllAsProps(this.props);
const buttonChildProps = _.map(findTypes(this.props, ButtonGroup.Button), 'props');

return (
<span
Expand Down
2 changes: 1 addition & 1 deletion src/components/Checkbox/Checkbox.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from 'lodash';
import React from 'react';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass } from '../../util/component-types';

const boundClassNames = lucidClassNames.bind('&-Checkbox');

Expand Down
30 changes: 19 additions & 11 deletions src/components/ContextMenu/ContextMenu.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import _ from 'lodash';
import Portal from '../Portal/Portal';
import { createClass } from '../../util/component-definition';
import { createClass, findTypes } from '../../util/component-types';
import { getAbsoluteBoundingClientRect } from '../../util/dom-helpers';
import { lucidClassNames } from '../../util/style-helpers';

Expand Down Expand Up @@ -64,11 +64,18 @@ const ContextMenu = createClass({
portalId: string
},

childProps: {
Target: {},
FlyOut: {
style: object
}
components: {
Target: createClass({
displayName: 'ContextMenu.Target',
propName: 'Target',
}),
FlyOut: createClass({
displayName: 'ContextMenu.FlyOut',
propName: 'FlyOut',
propTypes: {
style: object
}
})
},

getDefaultProps() {
Expand Down Expand Up @@ -186,10 +193,11 @@ const ContextMenu = createClass({
flyOutHeight
} = this.state;

const targetProps = _.first(ContextMenu.Target.findInAllAsProps(this.props));
const targetChildren = targetProps.children;
const flyProps = _.first(ContextMenu.FlyOut.findInAllAsProps(this.props));
const flyChildren = flyProps.children;
const targetElement = _.first(findTypes(this.props, ContextMenu.Target));
const targetChildren = _.get(targetElement, 'props.children', null);

const flyoutElement = _.first(findTypes(this.props, ContextMenu.FlyOut));
const flyProps = _.get(flyoutElement, 'props', {});


return (
Expand All @@ -211,7 +219,7 @@ const ContextMenu = createClass({
top: (direction === ContextMenu.UP ? targetRect.top - flyOutHeight : targetRect.bottom)
})}
>
{flyChildren}
{flyProps.children}
</Portal>
) : null}
</span>
Expand Down
33 changes: 20 additions & 13 deletions src/components/DataTable/DataTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import _ from 'lodash';
import React from 'react';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { findAllChildComponents } from '../../util/child-component';
import { createClass, findTypes, filterTypes } from '../../util/component-types';

import Checkbox from '../Checkbox/Checkbox';
import ScrollTable from '../ScrollTable/ScrollTable';
Expand Down Expand Up @@ -108,14 +107,22 @@ const DataTable = createClass({
};
},

childProps: {
Column: {
field: string.isRequired,
title: string
},
ColumnGroup: {
title: string
}
components: {
Column: createClass({
displayName: 'DataTable.Column',
propName: 'Column',
propTypes: {
field: string.isRequired,
title: string
}
}),
ColumnGroup: createClass({
displayName: 'DataTable.ColumnGroup',
propName: 'ColumnGroup',
propTypes: {
title: string
}
})
},

statics: {
Expand Down Expand Up @@ -175,13 +182,13 @@ const DataTable = createClass({
} = this.props;


const childComponentElements = findAllChildComponents(this.props, [DataTable.Column, DataTable.ColumnGroup]);
const childComponentElements = findTypes(this.props, [DataTable.Column, DataTable.ColumnGroup]);
const flattenedColumns = _.reduce(childComponentElements, (acc, childComponentElement) => {
if (childComponentElement.type === DataTable.Column) {
return acc.concat([{props: childComponentElement.props, columnGroupProps: null}]);
}
if (childComponentElement.type === DataTable.ColumnGroup) {
return acc.concat(_.map(findAllChildComponents(childComponentElement.props, [DataTable.Column]), (columnChildComponent) => ({ props: columnChildComponent.props, columnGroupProps: childComponentElement.props })));
return acc.concat(_.map(findTypes(childComponentElement.props, DataTable.Column), (columnChildComponent) => ({ props: columnChildComponent.props, columnGroupProps: childComponentElement.props })));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

format this a little better

}
}, []);

Expand All @@ -207,7 +214,7 @@ const DataTable = createClass({
</Th>
) : (
<Th
colSpan={_.size(DataTable.Column.findInChildren(props.children))}
colSpan={_.size(filterTypes(props.children, DataTable.Column))}
{..._.omit(props, ['field', 'children', 'width', 'title'])}
key={_.get(props, 'field', index)}
>
Expand Down
18 changes: 12 additions & 6 deletions src/components/Dialog/Dialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from 'lodash';
import React from 'react';
import Overlay from '../Overlay/Overlay';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass, findTypes } from '../../util/component-types';

const boundClassNames = lucidClassNames.bind('&-Dialog');

Expand All @@ -24,9 +24,15 @@ const LARGE = 'large';
const Dialog = createClass({
displayName: 'Dialog',

childProps: {
Header: null,
Footer: null,
components: {
Header: createClass({
displayName: 'Dialog.Header',
propName: 'Header'
}),
Footer: createClass({
displayName: 'Dialog.Footer',
propName: 'Footer'
}),
},

propTypes: {
Expand Down Expand Up @@ -63,8 +69,8 @@ const Dialog = createClass({
...passThroughs
} = this.props;

const headerChildProp = _.first(Dialog.Header.findInAllAsProps(this.props));
const footerChildProp = _.first(Dialog.Footer.findInAllAsProps(this.props));
const headerChildProp = _.get(_.first(findTypes(this.props, Dialog.Header)), 'props', {});
const footerChildProp = _.get(_.first(findTypes(this.props, Dialog.Footer)), 'props', {});

return (
<Overlay
Expand Down
2 changes: 1 addition & 1 deletion src/components/DragCaptureZone/DragCaptureZone.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from 'lodash';
import React from 'react';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass } from '../../util/component-types';

const boundClassNames = lucidClassNames.bind('&-DragCaptureZone');
const {
Expand Down
47 changes: 32 additions & 15 deletions src/components/DropMenu/DropMenu.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import React from 'react';
import _ from 'lodash';
import { lucidClassNames } from '../../util/style-helpers';
import { createClass } from '../../util/component-definition';
import { createClass, findTypes, rejectTypes } from '../../util/component-types';
import { scrollParentTo } from '../../util/dom-helpers';
import { rejectNullElements } from '../../util/child-component';
import * as KEYCODE from '../../constants/key-code';
import * as reducers from './DropMenu.reducers';
import ContextMenu from '../ContextMenu/ContextMenu';
Expand Down Expand Up @@ -43,13 +42,26 @@ const DropMenu = createClass({

reducers,

childProps: {
Control: {},
OptionGroup: {},
Option: {
isDisabled: bool
},
NullOption: {}
components: {
Control: createClass({
displayName: 'DropMenu.Control',
propName: 'Control'
}),
OptionGroup: createClass({
displayName: 'DropMenu.OptionGroup',
propName: 'OptionGroup'
}),
Option: createClass({
displayName: 'DropMenu.Option',
propName: 'Option',
propTypes: {
isDisabled: bool
}
}),
NullOption: createClass({
displayName: 'DropMenu.NullOption',
propName: 'NullOption'
})
},

propTypes: {
Expand Down Expand Up @@ -179,13 +191,17 @@ const DropMenu = createClass({
Option,
NullOption
} = ParentType;
const optionGroups = OptionGroup.findInAllAsProps(props); // find all OptionGroup props
const ungroupedOptions = Option.findInAllAsProps(props); // find all ungrouped Option props
const nullOptions = NullOption ? NullOption.findInAllAsProps(props) : []; // find all NullOption props
//const optionGroups = OptionGroup.findInAllAsProps(props); // find all OptionGroup props
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove commented code

const optionGroups = _.map(findTypes(props, OptionGroup), 'props'); // find all OptionGroup props
//const ungroupedOptions = Option.findInAllAsProps(props); // find all ungrouped Option props
const ungroupedOptions = _.map(findTypes(props, Option), 'props') // find all ungrouped Option props
//const nullOptions = NullOption ? NullOption.findInAllAsProps(props) : []; // find all NullOption props
const nullOptions = NullOption ? _.map(findTypes(props, NullOption), 'props') : []; // find all NullOption props

// flatten grouped options into array of objects to associate { index, group index, and props } for each option
const groupedOptionData = _.reduce(optionGroups, (memo, optionGroupProps, optionGroupIndex) => {
const groupedOptions = Option.findInAllAsProps(optionGroupProps); // find all Option props for current group
//const groupedOptions = Option.findInAllAsProps(optionGroupProps); // find all Option props for current group
const groupedOptions = _.map(findTypes(optionGroupProps, Option), 'props'); // find all Option props for current group
return memo.concat(_.map(groupedOptions, (optionProps, localOptionIndex) => {
return {
localOptionIndex,
Expand Down Expand Up @@ -416,7 +432,8 @@ const DropMenu = createClass({
nullOptions
} = this.state;

const controlProps = _.first(DropMenu.Control.findInAllAsProps(this.props));
//const controlProps = _.first(DropMenu.Control.findInAllAsProps(this.props));
const controlProps = _.get(_.first(findTypes(this.props, DropMenu.Control)), 'props', {});

return (
<div className={boundClassNames('&', '&-base', {
Expand Down Expand Up @@ -445,7 +462,7 @@ const DropMenu = createClass({
joinArray(
// for each option group,
_.map(optionGroups, (optionGroupProps, optionGroupIndex) => {
const labelElements = rejectNullElements(optionGroupProps.children);
const labelElements = rejectTypes(optionGroupProps.children, [DropMenu.Control, DropMenu.OptionGroup, DropMenu.Option, DropMenu.NullOption]);
// render label if there is one
return (_.isEmpty(labelElements) ? [] : [
<div {...optionGroupProps} className={boundClassNames('&-label', optionGroupProps.className)}>
Expand Down
Loading