From bea531e9b331d8f6f4a85ed3a046fe10f9198466 Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Thu, 12 Jan 2017 19:07:35 -0800 Subject: [PATCH 1/7] feat(typings): add closeOnBlur and openOnFocus (#1151) * feat(typings): add closeOnBlur and openOnFocus * refactor(Dropdown): sort prop types --- src/modules/Dropdown/Dropdown.js | 12 +++--- src/modules/Dropdown/index.d.ts | 71 +++++++++++++++++--------------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/modules/Dropdown/Dropdown.js b/src/modules/Dropdown/Dropdown.js index 3bed4274d0..e1517f154f 100644 --- a/src/modules/Dropdown/Dropdown.js +++ b/src/modules/Dropdown/Dropdown.js @@ -81,6 +81,9 @@ export default class Dropdown extends Component { /** Additional classes. */ className: PropTypes.string, + /** Whether or not the menu should close when the dropdown is blurred. */ + closeOnBlur: PropTypes.bool, + /** A compact dropdown has no minimum width. */ compact: PropTypes.bool, @@ -230,6 +233,9 @@ export default class Dropdown extends Component { /** Controls whether or not the dropdown menu is displayed. */ open: PropTypes.bool, + /** Whether or not the menu should open when the dropdown is focused. */ + openOnFocus: PropTypes.bool, + /** Array of Dropdown.Item props e.g. `{ text: '', value: '' }` */ options: customPropTypes.every([ customPropTypes.disallow(['children']), @@ -296,12 +302,6 @@ export default class Dropdown extends Component { /** The text displayed in the dropdown, usually for the active item. */ text: PropTypes.string, - /** Whether or not the menu should open when the dropdown is focused. */ - openOnFocus: PropTypes.bool, - - /** Whether or not the menu should close when the dropdown is blurred. */ - closeOnBlur: PropTypes.bool, - /** Custom element to trigger the menu to become visible. Takes place of 'text'. */ trigger: customPropTypes.every([ customPropTypes.disallow(['selection', 'text']), diff --git a/src/modules/Dropdown/index.d.ts b/src/modules/Dropdown/index.d.ts index bcd1249447..14606368eb 100644 --- a/src/modules/Dropdown/index.d.ts +++ b/src/modules/Dropdown/index.d.ts @@ -1,23 +1,22 @@ -import { ReactFocusEvents, ReactFormEvents, ReactMouseEvents } from '../..'; +import {ReactFocusEvents, ReactFormEvents, ReactMouseEvents} from '../..'; import * as React from 'react'; export type DropdownPropPointing = 'left' | 'right' | 'top' | 'top left' | 'top right' | 'bottom' | 'bottom left' | 'bottom right' export type DropdownPropAdditionPosition = 'top' | 'bottom'; export interface DropdownProps extends ReactMouseEvents, ReactFocusEvents, ReactFormEvents { - /** Label prefixed to an option added by a user. */ additionLabel?: string; - + /** Position of the `Add: ...` option in the dropdown list ('top' or 'bottom'). */ additionPosition?: DropdownPropAdditionPosition; - + /** * Allow user additions to the list of options (boolean). * Requires the use of `selection`, `options` and `search`. */ allowAdditions?: any; - + /** An element type to render as (string or function). */ as?: any; @@ -33,20 +32,23 @@ export interface DropdownProps extends ReactMouseEvents, ReactFocus /** Additional classes. */ className?: string; + /** Whether or not the menu should close when the dropdown is blurred. */ + closeOnBlur?: boolean, + /** A compact dropdown has no minimum width. */ compact?: boolean; /** Initial value of open. */ defaultOpen?: boolean; - + /** Currently selected label in multi-select. */ - defaultSelectedLabel?:any; + defaultSelectedLabel?: any; /** Initial value or value array if multiple. */ defaultValue?: string|number|Array|Array; - + /** A disabled dropdown menu or item does not allow user interaction. */ - disabled?:boolean; + disabled?: boolean; /** An errored dropdown can alert a user to a problem. */ error?: boolean; @@ -59,7 +61,7 @@ export interface DropdownProps extends ReactMouseEvents, ReactFocus /** A dropdown menu can contain a header. */ header?: React.ReactNode; - + /** Shorthand for Icon. */ icon?: any; @@ -76,7 +78,7 @@ export interface DropdownProps extends ReactMouseEvents, ReactFocus multiple?: boolean; /** Name of the hidden input which holds the value. */ - name?:string; + name?: string; /** Message to display when there are no results. */ noResultsMessage?: string; @@ -96,25 +98,28 @@ export interface DropdownProps extends ReactMouseEvents, ReactFocus * @param {string} value - Current value of search input. */ onSearchChange?: React.FormEventHandler; - + /** Controls whether or not the dropdown menu is displayed. */ open?: boolean; + /** Whether or not the menu should open when the dropdown is focused. */ + openOnFocus?: boolean, + /** Array of Dropdown.Item props e.g. `{ text: '', value: '' }` */ options?: Array; - + /** Placeholder text. */ placeholder?: string; /** A dropdown can be formatted so that its menu is pointing. */ pointing?: boolean | DropdownPropPointing; - + /** * A function that takes (data, index, defaultLabelProps) and returns * shorthand for Label . */ - renderLabel?:any; - + renderLabel?: any; + /** A dropdown can have its menu scroll. */ scrolling?: boolean; @@ -122,8 +127,8 @@ export interface DropdownProps extends ReactMouseEvents, ReactFocus * A selection dropdown can allow a user to search through a large list of choices. * Pass a function here to replace the default search. */ - search?: ((filteredOptions:any, searchQuery:any) => void) | boolean; // TODO -add search function; - + search?: ((filteredOptions: any, searchQuery: any) => void) | boolean; // TODO -add search function; + /** Define whether the highlighted item should be selected on blur. */ selectOnBlur?: boolean; @@ -134,7 +139,7 @@ export interface DropdownProps extends ReactMouseEvents, ReactFocus simple?: boolean; /** A dropdown can receive focus. */ - tabIndex?:string; + tabIndex?: string; /** The text displayed in the dropdown, usually for the active item. */ text?: string|React.ReactNode; @@ -156,20 +161,20 @@ interface DropdownClass extends React.ComponentClass { export const Dropdown: DropdownClass; interface DropdownDividerProps { - + /** An element type to render as (string or function). */ - as?: any; + as?: any; /** Additional classes. */ - className?:string; + className?: string; } export const DropdownDivider: React.ComponentClass; interface DropdownHeaderProps { - + /** An element type to render as (string or function). */ - as?: any; + as?: any; /** Primary content. */ children?: React.ReactNode; @@ -187,12 +192,12 @@ interface DropdownHeaderProps { export const DropdownHeader: React.ComponentClass; interface DropdownItemProps extends ReactMouseEvents, ReactFocusEvents, ReactFormEvents { - + /** Style as the currently chosen item. */ active?: boolean; - + /** An element type to render as (string or function). */ - as?: any; + as?: any; /** Primary content. */ children?: React.ReactNode; @@ -207,16 +212,16 @@ interface DropdownItemProps extends ReactMouseEvents, ReactFocusEve disabled?: boolean; /** Shorthand for Flag. */ - flag?:any; + flag?: any; /** Shorthand for Icon. */ icon?: any; /** Shorthand for Image. */ - image?:any; + image?: any; /** Shorthand for Label. */ - label?:any; + label?: any; /** * The item currently selected by keyboard shortcut. @@ -234,9 +239,9 @@ interface DropdownItemProps extends ReactMouseEvents, ReactFocusEve export const DropdownItem: React.ComponentClass; interface DropdownMenuProps { - + /** An element type to render as (string or function). */ - as?: any; + as?: any; /** Primary content. */ children?: React.ReactNode; @@ -245,7 +250,7 @@ interface DropdownMenuProps { className?: string; /** A dropdown menu can scroll. */ - scrolling?:boolean; + scrolling?: boolean; } export const DropdownMenu: React.ComponentClass; From ab48267f7f8f3198ad66d57790f48a3736590640 Mon Sep 17 00:00:00 2001 From: Alexander Fedyashov Date: Fri, 13 Jan 2017 06:59:05 +0200 Subject: [PATCH 2/7] perf(props): Remove propTypes from production build (#731) * perf(props): Remove propTypes from production build perf(props): Remove propTypes from production build * perf(package): add babel react optimization plugins * wip(Rail): remove props feat(Plugin): remove props fix(Plugin): remove plugin code fix(Plugin): remove plugin code fix(Plugin): remove plugin code fix(Plugin): remove plugin code * revert(package): remove `babel-register` * chore(package): add plugin * chore(package): add plugin * feat(package): Add plugin, update tests, cleanups * fix(ComponentProps): update component (remove enums functionality), fix lint issues * fix(common): fix review comments --- .babelrc | 10 ++- .github/CONTRIBUTING.md | 24 ++++-- .../Components/ComponentDoc/ComponentProps.js | 82 ++----------------- package.json | 5 ++ src/elements/Rail/Rail.js | 11 +-- src/lib/getUnhandledProps.js | 31 +------ test/specs/commonTests.js | 36 ++++---- test/specs/lib/getUnhandledProps-test.js | 41 +++------- 8 files changed, 74 insertions(+), 166 deletions(-) diff --git a/.babelrc b/.babelrc index f408783fde..2fc567d3e3 100644 --- a/.babelrc +++ b/.babelrc @@ -5,7 +5,15 @@ "stage-1" ], "plugins": [ - "lodash" + "lodash", + "transform-react-handled-props", + ["transform-react-remove-prop-types", { + "mode": "wrap" + }], + ["transform-runtime", { + "polyfill": false, + "regenerator": false + }] ], "env": { "test": { diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index adeb8d3c5e..8819c03c32 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -126,7 +126,7 @@ class Dropdown extends Component { ### Define _meta -Every component has a static property called `_meta`. This object defines the component. The values here are used in `propTypes`, generated documentation, generated test cases, and some utilities. +Every component has a static property called `_meta`. This object defines the component. The values here are used for generated documentation, generated test cases and some utilities. Here's an example `_meta` object: @@ -136,9 +136,6 @@ import { META } from '../../lib' const _meta = { name: 'MyComponent', type: META.TYPES.MODULE, - props: { - pointing: ['bottom left', 'bottom right'], - }, } ``` @@ -162,6 +159,23 @@ class MyComponent { } ``` +### Using propTypes + +Every component must have fully described `propTypes`. + + ```js + import React, { PropTypes } from 'react' + + function MyComponent(props) { + return
{props.children}
+ } + + MyComponent.propTypes = { + children: PropTypes.node, + position: PropTypes.oneOf(['left', 'right']), + } + ``` + ### Conformance Test Review [common tests](#common-tests) below. You should now add the [`isConformant()`](#isconformant-required) common test and get it to pass. This will validate the `_meta` and help you get your component off the ground. @@ -176,7 +190,7 @@ This will also help with getting early feedback and smaller faster iterations on Review the SUI documentation for the component. Spec out the component's proposed API. The spec should demonstrate how your component's API will support all the native SUI features. You can reference this [API proposal][7] for the Input. -Once we have solidified the component spec, it's time to write some code. The following sections cover everything you'll need to spec and build your awesome component. +Once we have solidified the component spec, it's time to write some code. The following sections cover everything you'll need to spec and build your awesome component. ## API diff --git a/docs/app/Components/ComponentDoc/ComponentProps.js b/docs/app/Components/ComponentDoc/ComponentProps.js index 0f7b87307f..5077bd1222 100644 --- a/docs/app/Components/ComponentDoc/ComponentProps.js +++ b/docs/app/Components/ComponentDoc/ComponentProps.js @@ -3,11 +3,6 @@ import React, { Component, PropTypes } from 'react' import { Label, Table } from 'src' -const descriptionExtraStyle = { - fontSize: '0.95em', - color: '#777', -} - /** * Displays a table of a Component's PropTypes. */ @@ -25,26 +20,9 @@ export default class ComponentProps extends Component { meta: PropTypes.object, } - state = { - showEnumsFor: {}, - } - - toggleEnumsFor = (prop) => () => { - this.setState({ - showEnumsFor: { - ...this.state.showEnumsFor, - [prop]: !this.state.showEnumsFor[prop], - }, - }) - } - - renderName = (item) => {item.name} + renderName = item => {item.name} - requiredRenderer = (item) => { - if (!item.required) return null - - return - } + requiredRenderer = item => item.required && renderDefaultValue = (item) => { let defaultValue = _.get(item, 'defaultValue.value') @@ -62,52 +40,6 @@ export default class ComponentProps extends Component { ) } - renderEnums = (item) => { - const { showEnumsFor } = this.state - const { meta } = this.props - - if (item.type.indexOf('enum') === -1) return null - - const values = meta.props[item.name] - const truncateAt = 30 - - if (!values) return null - - // show all if there are few - if (values.length < truncateAt) { - return ( -

- Enums: - {values.join(', ')} -

- ) - } - - // add button to show more when there are many values and it is not toggled - if (!showEnumsFor[item.name]) { - return ( -

- Enums: - - Show all {values.length} - -

{values.slice(0, truncateAt - 1).join(', ')}...
-

- ) - } - - // add "show more" button when there are many - return ( -

- Enums: - - Show less - -

{values.join(', ')}
-

- ) - } - render() { const { props: propsDefinition } = this.props const content = _.sortBy(_.map(propsDefinition, (config, name) => { @@ -129,6 +61,7 @@ export default class ComponentProps extends Component { description: description && description.split('\n').map(l => ([l,
])), } }), 'name') + return ( @@ -145,14 +78,9 @@ export default class ComponentProps extends Component { {this.renderName(item)} {this.requiredRenderer(item)} - - {item.type} - + {item.type} {this.renderDefaultValue(item.defaultValue)} - - {item.description &&

{item.description}

} - {this.renderEnums(item)} - + {item.description &&

{item.description}

} ))} diff --git a/package.json b/package.json index 326ea575c9..4a8c541585 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,11 @@ "babel-loader": "^6.2.8", "babel-plugin-__coverage__": "^11.0.0", "babel-plugin-lodash": "^3.2.10", + "babel-plugin-react-transform": "^2.0.2", + "babel-plugin-transform-react-constant-elements": "^6.9.1", + "babel-plugin-transform-react-handled-props": "^0.2.1", + "babel-plugin-transform-react-remove-prop-types": "^0.2.10", + "babel-plugin-transform-runtime": "^6.15.0", "babel-preset-es2015": "^6.18.0", "babel-preset-react": "^6.5.0", "babel-preset-stage-1": "^6.5.0", diff --git a/src/elements/Rail/Rail.js b/src/elements/Rail/Rail.js index 31547c085e..ae1ce3de4d 100644 --- a/src/elements/Rail/Rail.js +++ b/src/elements/Rail/Rail.js @@ -47,11 +47,6 @@ function Rail(props) { Rail._meta = { name: 'Rail', type: META.TYPES.ELEMENT, - props: { - close: ['very'], - position: SUI.FLOATS, - size: _.without(SUI.SIZES, 'medium'), - }, } Rail.propTypes = { @@ -70,7 +65,7 @@ Rail.propTypes = { /** A rail can appear closer to the main viewport. */ close: PropTypes.oneOfType([ PropTypes.bool, - PropTypes.oneOf(Rail._meta.props.close), + PropTypes.oneOf(['very']), ]), /** A rail can create a division between itself and a container. */ @@ -80,10 +75,10 @@ Rail.propTypes = { internal: PropTypes.bool, /** A rail can be presented on the left or right side of a container. */ - position: PropTypes.oneOf(Rail._meta.props.position).isRequired, + position: PropTypes.oneOf(SUI.FLOATS).isRequired, /** A rail can have different sizes. */ - size: PropTypes.oneOf(Rail._meta.props.size), + size: PropTypes.oneOf(_.without(SUI.SIZES, 'medium')), } export default Rail diff --git a/src/lib/getUnhandledProps.js b/src/lib/getUnhandledProps.js index d93e70f435..1a4d199541 100644 --- a/src/lib/getUnhandledProps.js +++ b/src/lib/getUnhandledProps.js @@ -1,14 +1,3 @@ -/** - * Push all `source` array elements to the `target` array if they don't already exist in `target`. - * - * @param {Array} source - An array of elements to add to the `target` - * @param {Array} target - An array to receive unique elements from the `source` - * @returns {Array} Mutated `target` array - */ -const pushUnique = (source, target) => source.forEach(x => { - if (target.indexOf(x) === -1) target.push(x) -}) - /** * Returns an object consisting of props beyond the scope of the Component. * Useful for getting and spreading unknown props from the user. @@ -17,25 +6,9 @@ const pushUnique = (source, target) => source.forEach(x => { * @returns {{}} A shallow copy of the prop object */ const getUnhandledProps = (Component, props) => { - const { autoControlledProps, defaultProps, propTypes } = Component - let { handledProps } = Component - - // ---------------------------------------- - // Calculate handledProps once and cache - // ---------------------------------------- - if (!handledProps) { - handledProps = [] - - if (autoControlledProps) pushUnique(autoControlledProps, handledProps) - if (defaultProps) pushUnique(Object.keys(defaultProps), handledProps) - if (propTypes) pushUnique(Object.keys(propTypes), handledProps) - - Component.handledProps = handledProps - } + // Note that `handledProps` are generated automatically during build with `babel-plugin-transform-react-handled-props` + const { handledProps = [] } = Component - // ---------------------------------------- - // Return _unhandled_ props - // ---------------------------------------- return Object.keys(props).reduce((acc, prop) => { if (prop === 'childKey') return acc if (handledProps.indexOf(prop) === -1) acc[prop] = props[prop] diff --git a/test/specs/commonTests.js b/test/specs/commonTests.js index d8147a1584..84c1429445 100644 --- a/test/specs/commonTests.js +++ b/test/specs/commonTests.js @@ -254,6 +254,27 @@ export const isConformant = (Component, options = {}) => { }) }) + describe('handles props', () => { + it('defines handled props in Component.handledProps', () => { + Component.should.have.any.keys('handledProps') + Component.handledProps.should.be.an('array') + }) + + it('Component.handledProps includes all handled props', () => { + const computedProps = _.union( + Component.autoControlledProps, + _.keys(Component.defaultProps), + _.keys(Component.propTypes), + ) + const expectedProps = _.uniq(computedProps).sort() + + Component.handledProps.should.to.deep.equal(expectedProps, + 'It seems that not all props were defined in Component.handledProps, you need to check that they are equal ' + + 'to the union of Component.autoControlledProps and keys of Component.defaultProps and Component.propTypes' + ) + }) + }) + // ---------------------------------------- // Events // ---------------------------------------- @@ -460,15 +481,6 @@ export const rendersChildren = (Component, options = {}) => { // ---------------------------------------- // className from prop // ---------------------------------------- -const _definesPropOptions = (Component, propKey) => { - it(`defines ${propKey} options in Component._meta.props`, () => { - Component.should.have.any.keys('_meta') - Component._meta.should.have.any.keys('props') - Component._meta.props.should.have.any.keys(propKey) - Component._meta.props[propKey].should.be.an('array') - }) -} - const _noDefaultClassNameFromProp = (Component, propKey, options = {}) => { const { className = propKey, requiredProps = {} } = options // required props may include a prop that creates a className @@ -592,7 +604,6 @@ export const implementsWidthProp = (Component, options = {}) => { describe(`${propKey} (common)`, () => { assertRequired(Component, 'a `Component`') - _definesPropOptions(Component, propKey) _noDefaultClassNameFromProp(Component, propKey, options) _noClassNameFromBoolProps(Component, propKey, options) @@ -853,7 +864,6 @@ export const implementsTextAlignProp = (Component, options = {}) => { describe('aligned (common)', () => { assertRequired(Component, 'a `Component`') - _definesPropOptions(Component, 'textAlign') _noDefaultClassNameFromProp(Component, 'textAlign', options) _noClassNameFromBoolProps(Component, 'textAlign', options) @@ -889,7 +899,6 @@ export const implementsVerticalAlignProp = (Component, options = {}) => { describe('verticalAlign (common)', () => { assertRequired(Component, 'a `Component`') - _definesPropOptions(Component, 'verticalAlign') _noDefaultClassNameFromProp(Component, 'verticalAlign', options) _noClassNameFromBoolProps(Component, 'verticalAlign', options) @@ -952,7 +961,6 @@ export const propValueOnlyToClassName = (Component, propKey, options = {}) => { assertRequired(Component, 'a `Component`') assertRequired(propKey, 'a `propKey`') - _definesPropOptions(Component, propKey) _noDefaultClassNameFromProp(Component, propKey, options) _noClassNameFromBoolProps(Component, propKey, options) @@ -990,7 +998,6 @@ export const propKeyAndValueToClassName = (Component, propKey, options = {}) => assertRequired(Component, 'a `Component`') assertRequired(propKey, 'a `propKey`') - _definesPropOptions(Component, propKey) _noDefaultClassNameFromProp(Component, propKey, options) _noClassNameFromBoolProps(Component, propKey, options) _classNamePropValueBeforePropName(Component, propKey, options) @@ -1013,7 +1020,6 @@ export const propKeyOrValueAndKeyToClassName = (Component, propKey, options = {} assertRequired(Component, 'a `Component`') assertRequired(propKey, 'a `propKey`') - _definesPropOptions(Component, propKey) _noDefaultClassNameFromProp(Component, propKey, options) _classNamePropValueBeforePropName(Component, propKey, options) beforeEach(() => { diff --git a/test/specs/lib/getUnhandledProps-test.js b/test/specs/lib/getUnhandledProps-test.js index db1fcd22d8..411c379ed0 100644 --- a/test/specs/lib/getUnhandledProps-test.js +++ b/test/specs/lib/getUnhandledProps-test.js @@ -1,48 +1,27 @@ -import React, { PropTypes } from 'react' - +import React from 'react' import { getUnhandledProps } from 'src/lib' // We spread the unhandled props onto the rendered result. // Then, we can test the props of the rendered result. -// This is the inteded usage of the util. +// This is the intended usage of the util. function TestComponent(props) { return
} -TestComponent._meta = { name: 'TestComponent' } - -beforeEach(() => { - delete TestComponent.propTypes - delete TestComponent.defaultProps - delete TestComponent.autoControlledProps -}) describe('getUnhandledProps', () => { - it('removes props defined in propTypes', () => { - TestComponent.propTypes = { 'data-remove-me': PropTypes.string } - shallow() - .should.not.have.prop('data-remove-me', 'thanks') - }) it('removes the proprietary childKey prop', () => { shallow() .should.not.have.prop('childKey') }) - it('removes props defined in defaultProps', () => { - TestComponent.defaultProps = { 'data-remove-me': 'thanks' } - shallow() - .should.not.have.prop('data-remove-me', 'thanks') - }) - it('removes props defined in autoControlledProps', () => { - TestComponent.autoControlledProps = ['data-remove-me'] - shallow() - .should.not.have.prop('data-remove-me') - }) - it('removes default versions of autoControlledProps', () => { - TestComponent.autoControlledProps = ['data-remove-me'] - shallow() - .should.not.have.prop('defaultRemoveMe') - }) - it('leaves props that are not defined in propTypes', () => { + + it('leaves props that are not defined in handledProps', () => { shallow() .should.have.prop('data-leave-this') }) + + it('removes props defined in handledProps', () => { + TestComponent.handledProps = ['data-remove-me'] + shallow() + .should.not.have.prop('data-remove-me', 'thanks') + }) }) From dfe89d39f136f1f27af6910607d7024bce6585fe Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Thu, 12 Jan 2017 21:00:31 -0800 Subject: [PATCH 3/7] breaking(shorthand): only generate keys from strings and numbers (#1062) breaking(shorthand): only generate keys from strings and numbers --- .../Types/BreadcrumbExampleProps.js | 6 +- .../Menu/Types/MenuExampleProps.js | 22 +++-- src/lib/factories.js | 80 ++++++++----------- test/specs/collections/Menu/Menu-test.js | 12 +-- test/specs/lib/factories-test.js | 55 ++++++++++++- 5 files changed, 105 insertions(+), 70 deletions(-) diff --git a/docs/app/Examples/collections/Breadcrumb/Types/BreadcrumbExampleProps.js b/docs/app/Examples/collections/Breadcrumb/Types/BreadcrumbExampleProps.js index 6cca10a03c..c378857538 100644 --- a/docs/app/Examples/collections/Breadcrumb/Types/BreadcrumbExampleProps.js +++ b/docs/app/Examples/collections/Breadcrumb/Types/BreadcrumbExampleProps.js @@ -2,9 +2,9 @@ import React from 'react' import { Breadcrumb } from 'semantic-ui-react' const sections = [ - { content: 'Home', link: true }, - { content: 'Store', link: true }, - { content: 'T-Shirt', active: true }, + { key: 'Home', content: 'Home', link: true }, + { key: 'Store', content: 'Store', link: true }, + { key: 'Shirt', content: 'T-Shirt', active: true }, ] const BreadcrumbExampleProps = () => ( diff --git a/docs/app/Examples/collections/Menu/Types/MenuExampleProps.js b/docs/app/Examples/collections/Menu/Types/MenuExampleProps.js index ba7bee5f9d..13932fe8a9 100644 --- a/docs/app/Examples/collections/Menu/Types/MenuExampleProps.js +++ b/docs/app/Examples/collections/Menu/Types/MenuExampleProps.js @@ -1,16 +1,14 @@ -import React, { Component } from 'react' +import React from 'react' import { Menu } from 'semantic-ui-react' -export default class MenuExampleProps extends Component { - state = {} +const items = [ + { key: 'editorials', active: true, name: 'Editorials' }, + { key: 'review', name: 'Reviews' }, + { key: 'events', name: 'Upcoming Events' }, +] - render() { - const items = [ - { active: true, name: 'Editorials' }, - { name: 'Reviews' }, - { name: 'Upcoming Events' }, - ] +const MenuExampleProps = () => ( + +) - return - } -} +export default MenuExampleProps diff --git a/src/lib/factories.js b/src/lib/factories.js index fbd405c64e..55952abf87 100644 --- a/src/lib/factories.js +++ b/src/lib/factories.js @@ -2,47 +2,6 @@ import _ from 'lodash' import cx from 'classnames' import React, { cloneElement, isValidElement } from 'react' -// ============================================================ -// Factory Utilities -// ============================================================ -/** - * A pure function that generates a unique child key hash code from an element's props. - * - * @param {object} props A ReactElement's props object. - * @returns {number} - */ -export const getChildKey = (props) => { - const { key, childKey } = props - - // already defines a key - if (key) return key - - // defines a childKey function or value - if (childKey) return typeof childKey === 'function' ? childKey(props) : childKey - - // 1. Stringify props to a short as possible run on string of key/values. - // 2. Don't stringify entire functions, use the function name || 'f'. - // 3. Generate a short hash number from the string. - // props : { color: 'red', onClick: handleClick } - // string : 'color:"red",onClick:handleClick' - // hash : 110042245 - return Object.keys(props).map(name => { - const val = props[name] - const type = typeof val - - const valueString = type === 'string' && val - || type === 'number' && val - || type === 'boolean' && (val ? 'true' : 'false') - || type === 'function' && (val.name || 'function') - || Array.isArray(val) && ['[', val.join(','), ']'].join('') - || val === null && 'null' - || type === 'object' && ['{', Object.keys(val).map(k => [k, ':', val[k]].join('')), '}'].join('') - || val === undefined && 'undefined' - - return [name, ':', valueString].join('') - }).join(',') -} - // ============================================================ // Factories // ============================================================ @@ -63,10 +22,12 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = } // short circuit for disabling shorthand if (val === null) return null + const valIsString = _.isString(val) + const valIsNumber = _.isNumber(val) const isReactElement = isValidElement(val) const isPropsObject = _.isPlainObject(val) - const isPrimitiveValue = _.isString(val) || _.isNumber(val) || _.isArray(val) + const isPrimitiveValue = valIsString || valIsNumber || _.isArray(val) // ---------------------------------------- // Build up props @@ -80,15 +41,38 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = // Default props defaultProps = _.isFunction(defaultProps) ? defaultProps(usersProps) : defaultProps - // Merge props and className + // Merge props + /* eslint-disable react/prop-types */ const props = { ...defaultProps, ...usersProps } - if (_.has(usersProps, 'className') || _.has(defaultProps.className)) { - props.className = cx(defaultProps.className, usersProps.className) // eslint-disable-line react/prop-types + // Merge className + if (usersProps.className && defaultProps.className) { + props.className = cx(defaultProps.className, usersProps.className) } - // Generate child key - if (generateKey) props.key = getChildKey(props) // eslint-disable-line react/prop-types + // Merge style + if (usersProps.style && defaultProps.style) { + props.style = { ...defaultProps.style, ...usersProps.style } + } + + // ---------------------------------------- + // Get key + // ---------------------------------------- + + // Use key, childKey, or generate key + if (!props.key) { + const { childKey } = props + + if (childKey) { + // apply and consume the childKey + props.key = typeof childKey === 'function' ? childKey(props) : childKey + delete props.childKey + } else if (generateKey && (valIsString || valIsNumber)) { + // use string/number shorthand values as the key + props.key = val + } + } + /* eslint-enable react/prop-types */ // ---------------------------------------- // Create Element @@ -105,7 +89,7 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = } // ============================================================ -// Factories Creators +// Factory Creators // ============================================================ /** diff --git a/test/specs/collections/Menu/Menu-test.js b/test/specs/collections/Menu/Menu-test.js index c75a795cb7..9e509eac09 100644 --- a/test/specs/collections/Menu/Menu-test.js +++ b/test/specs/collections/Menu/Menu-test.js @@ -40,8 +40,8 @@ describe('Menu', () => { describe('activeIndex', () => { const items = [ - { name: 'home' }, - { name: 'users' }, + { key: 'home', name: 'home' }, + { key: 'users', name: 'users' }, ] it('is null by default', () => { @@ -68,8 +68,8 @@ describe('Menu', () => { describe('items', () => { const spy = sandbox.spy() const items = [ - { name: 'home', onClick: spy, 'data-foo': 'something' }, - { name: 'users', active: true, 'data-foo': 'something' }, + { key: 'home', name: 'home', onClick: spy, 'data-foo': 'something' }, + { key: 'users', name: 'users', active: true, 'data-foo': 'something' }, ] const children = mount().find('MenuItem') @@ -100,8 +100,8 @@ describe('Menu', () => { describe('onItemClick', () => { const items = [ - { name: 'home' }, - { name: 'users' }, + { key: 'home', name: 'home' }, + { key: 'users', name: 'users' }, ] it('can be omitted', () => { diff --git a/test/specs/lib/factories-test.js b/test/specs/lib/factories-test.js index af8fe203fc..ea208898b2 100644 --- a/test/specs/lib/factories-test.js +++ b/test/specs/lib/factories-test.js @@ -19,7 +19,8 @@ const getShorthand = ({ mapValueToProps = val => ({}), value, defaultProps, -}) => createShorthand(Component, mapValueToProps, value, defaultProps) + generateKey, +}) => createShorthand(Component, mapValueToProps, value, defaultProps, generateKey) // ---------------------------------------- // Common tests @@ -201,6 +202,58 @@ describe('factories', () => { }) }) + describe('child key', () => { + it('uses the `key` prop from an element', () => { + getShorthand({ value:
}) + .should.have.property('key', 'foo') + }) + it('uses the `key` prop as a string', () => { + getShorthand({ value: { key: 'foo' } }) + .should.have.property('key', 'foo') + }) + it('uses the `key` prop as a number', () => { + getShorthand({ value: { key: 123 } }) + .should.have.property('key', '123') + }) + it('uses the `childKey` prop as a string', () => { + getShorthand({ value: { childKey: 'foo' } }) + .should.have.property('key', 'foo') + }) + it('uses the `childKey` prop as a number', () => { + getShorthand({ value: { childKey: 123 } }) + .should.have.property('key', '123') + }) + it('calls `childKey` with the final `props` if it is a function', () => { + const props = { foo: 'foo', childKey: sandbox.spy(({ foo }) => foo) } + + const element = getShorthand({ value: props }) + + props.childKey.should.have.been.calledOnce() + props.childKey.should.have.been.calledWithExactly({ foo: 'foo', key: 'foo' }) + + element.key.should.equal('foo') + }) + it('consumes the childKey prop', () => { + getShorthand({ value: { childKey: 123 } }) + .props.should.not.have.property('childKey') + }) + it('is generated from shorthand string values', () => { + getShorthand({ value: 'foo', generateKey: true }) + .should.have.property('key', 'foo') + }) + it('is generated from shorthand number values', () => { + getShorthand({ value: 123, generateKey: true }) + .should.have.property('key', '123') + }) + it('is not generated if generateKey is false', () => { + getShorthand({ value: 'foo', generateKey: false }) + .should.have.property('key', null) + + getShorthand({ value: 123, generateKey: false }) + .should.have.property('key', null) + }) + }) + describe('from undefined', () => { itReturnsNull(undefined) itReturnsNullGivenDefaultProps(undefined) From 92d58aac5e877cf0c6f88e7a924e4d0960b5cd0e Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Thu, 12 Jan 2017 21:10:27 -0800 Subject: [PATCH 4/7] fix(typings): fix Modal mountnode -> mountNode (#1153) --- src/modules/Modal/index.d.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/modules/Modal/index.d.ts b/src/modules/Modal/index.d.ts index 05b80ad4c9..3cb47d5f55 100644 --- a/src/modules/Modal/index.d.ts +++ b/src/modules/Modal/index.d.ts @@ -2,7 +2,7 @@ import { SemanticSIZES } from '../..'; import * as React from 'react'; interface ModalProps { - + /** An element type to render as (string or function). */ as?: any; @@ -59,7 +59,7 @@ interface ModalProps { open?: boolean; /** The node where the modal should mount.. */ - mountnode?: any; + mountNode?: any; /** A modal can vary in size */ size?: SemanticSIZES; @@ -76,7 +76,7 @@ interface ModalClass extends React.ComponentClass { export const Modal: ModalClass; interface ModalHeaderProps { - + /** An element type to render as (string or function). */ as?: any; @@ -90,7 +90,7 @@ interface ModalHeaderProps { export const ModalHeader: React.ComponentClass; interface ModalContentProps { - + /** An element type to render as (string or function). */ as?: any; @@ -107,7 +107,7 @@ interface ModalContentProps { export const ModalContent: React.ComponentClass; interface ModalDescriptionProps { - + /** An element type to render as (string or function). */ as?: any; @@ -121,7 +121,7 @@ interface ModalDescriptionProps { export const ModalDescription: React.ComponentClass; interface ModalActionsProps { - + /** An element type to render as (string or function). */ as?: any; From bf8aaad9b90832778f3df937fc6deb09c9f3985a Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Thu, 12 Jan 2017 23:59:43 -0800 Subject: [PATCH 5/7] docs(CONTRIBUTING): update toc --- .github/CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8819c03c32..d18405070e 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -12,6 +12,7 @@ CONTRIBUTING - [Workflow](#workflow) - [Create a Component](#create-a-component) - [Define _meta](#define-_meta) + - [Using propTypes](#using-proptypes) - [Conformance Test](#conformance-test) - [Open A PR](#open-a-pr) - [Spec out the API](#spec-out-the-api) From 2d8dddb36e2276dad7c67935720778f2268f8028 Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Fri, 13 Jan 2017 00:02:40 -0800 Subject: [PATCH 6/7] 0.64.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4a8c541585..1d5246da17 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "semantic-ui-react", - "version": "0.63.6", + "version": "0.64.0", "description": "The official Semantic-UI-React integration.", "main": "dist/commonjs/index.js", "files": [ From b7ffa6ad157817902c0bfe332f9eccc4cdcae128 Mon Sep 17 00:00:00 2001 From: Levi Thomason Date: Fri, 13 Jan 2017 00:08:33 -0800 Subject: [PATCH 7/7] docs(changelog): update changelog [ci skip] --- CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae26681be0..99d95b4fbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Change Log +## [v0.64.0](https://github.com/Semantic-Org/Semantic-UI-React/tree/v0.64.0) (2017-01-13) +[Full Changelog](https://github.com/Semantic-Org/Semantic-UI-React/compare/v0.63.6...v0.64.0) + +**Implemented enhancements:** + +- perf\(props\): Remove propTypes from production build [\#731](https://github.com/Semantic-Org/Semantic-UI-React/pull/731) ([layershifter](https://github.com/layershifter)) + +**Fixed bugs:** + +- Typings: Modal [\#1152](https://github.com/Semantic-Org/Semantic-UI-React/issues/1152) +- fix\(Table\): Cannot convert a Symbol value to a string [\#1057](https://github.com/Semantic-Org/Semantic-UI-React/issues/1057) + +**Merged pull requests:** + +- fix\(typings\): fix Modal mountnode -\> mountNode [\#1153](https://github.com/Semantic-Org/Semantic-UI-React/pull/1153) ([levithomason](https://github.com/levithomason)) +- feat\(typings\): add closeOnBlur and openOnFocus [\#1151](https://github.com/Semantic-Org/Semantic-UI-React/pull/1151) ([levithomason](https://github.com/levithomason)) +- breaking\(shorthand\): only generate keys from strings and numbers [\#1062](https://github.com/Semantic-Org/Semantic-UI-React/pull/1062) ([levithomason](https://github.com/levithomason)) + ## [v0.63.6](https://github.com/Semantic-Org/Semantic-UI-React/tree/v0.63.6) (2017-01-11) [Full Changelog](https://github.com/Semantic-Org/Semantic-UI-React/compare/v0.63.5...v0.63.6)