From 754416e7b37b260fcb625463afc6bcae5a41475e Mon Sep 17 00:00:00 2001 From: Alexander Fedyashov Date: Mon, 3 Apr 2017 07:47:57 +0300 Subject: [PATCH] feat(factories): add overrideProps (#1428) * feat(factories): add overrideProps * wip(factories): perform updates * wip(factories): perform updates * feat(factories): always generate keys * fix more tests, add test todos * fix(BreadcrumbDivider): use defaultProps in options * fix(FeedExampleShorthand): use unique shorthand strings * fix(mixed): wip on components update * fix(mixed): wip on components update * fix(FeedExampleExtraImagesShorthand): use unique keys * fix(FeedExampleEventsProp): use unique shorthand * fix(Dropdown): use defaultProps in create options * fix(mixed): fix broken tests * feat(mixed): update event handlers * tests(factory): add tests * tests(mixed): update tests and fixes prop handling * fix(Breadcrumb): generate key for shorthand divider * fix(ComponentExample): fix show html item active color --- .../ComponentDoc/ComponentExample.js | 2 +- .../FeedExampleExtraImagesShorthand.js | 2 +- .../views/Feed/Types/FeedExampleEventsProp.js | 4 +- .../views/Feed/Types/FeedExampleShorthand.js | 4 +- src/addons/Confirm/Confirm.js | 26 +-- src/collections/Breadcrumb/Breadcrumb.js | 24 +-- .../Breadcrumb/BreadcrumbDivider.js | 13 +- .../Breadcrumb/BreadcrumbSection.js | 2 +- src/collections/Menu/Menu.js | 23 +-- src/collections/Menu/MenuItem.js | 2 +- src/collections/Message/Message.js | 4 +- src/collections/Message/MessageItem.js | 2 +- src/collections/Table/Table.js | 2 +- src/collections/Table/TableCell.js | 2 +- src/collections/Table/TableRow.js | 4 +- src/elements/Button/Button.js | 4 +- src/elements/Input/Input.js | 16 +- src/elements/Label/Label.js | 13 +- src/elements/List/ListItem.js | 2 +- src/lib/factories.js | 69 +++++--- src/modules/Dropdown/Dropdown.js | 9 +- src/modules/Dropdown/DropdownHeader.js | 7 +- src/modules/Dropdown/DropdownItem.js | 4 +- src/modules/Modal/Modal.js | 9 +- src/modules/Progress/Progress.js | 4 +- src/modules/Search/Search.js | 4 +- .../collections/Breadcrumb/Breadcrumb-test.js | 36 +--- test/specs/collections/Menu/Menu-test.js | 30 ++-- .../commonTests/implementsCommonProps.js | 30 ++-- .../commonTests/implementsShorthandProp.js | 13 +- test/specs/docs/examples-test.js | 1 + test/specs/elements/Input/Input-test.js | 14 +- test/specs/elements/Label/Label-test.js | 17 ++ test/specs/lib/factories-test.js | 164 +++++++++++------- .../modules/Dropdown/DropdownItem-test.js | 4 +- test/tests.bundle.js | 4 + 36 files changed, 309 insertions(+), 261 deletions(-) diff --git a/docs/app/Components/ComponentDoc/ComponentExample.js b/docs/app/Components/ComponentDoc/ComponentExample.js index bed5dbd145..804a3d6594 100644 --- a/docs/app/Components/ComponentDoc/ComponentExample.js +++ b/docs/app/Components/ComponentDoc/ComponentExample.js @@ -413,7 +413,7 @@ class ComponentExample extends Component { - + diff --git a/docs/app/Examples/views/Feed/Content/FeedExampleExtraImagesShorthand.js b/docs/app/Examples/views/Feed/Content/FeedExampleExtraImagesShorthand.js index 1d0e69e19e..8525638958 100644 --- a/docs/app/Examples/views/Feed/Content/FeedExampleExtraImagesShorthand.js +++ b/docs/app/Examples/views/Feed/Content/FeedExampleExtraImagesShorthand.js @@ -6,7 +6,7 @@ const date = '3 days ago' const summary = 'Helen Troy added 2 photos' const extraImages = [ '/assets/images/wireframe/image.png', - '/assets/images/wireframe/image.png', + '/assets/images/wireframe/image-text.png', ] const FeedExampleExtraImagesShorthand = () => ( diff --git a/docs/app/Examples/views/Feed/Types/FeedExampleEventsProp.js b/docs/app/Examples/views/Feed/Types/FeedExampleEventsProp.js index 609def786c..9c2876f691 100644 --- a/docs/app/Examples/views/Feed/Types/FeedExampleEventsProp.js +++ b/docs/app/Examples/views/Feed/Types/FeedExampleEventsProp.js @@ -13,7 +13,7 @@ const events = [{ summary: 'Helen Troy added 2 new illustrations', extraImages: [ '/assets/images/wireframe/image.png', - '/assets/images/wireframe/image.png', + '/assets/images/wireframe/image-text.png', ], }, { date: '3 days ago', @@ -29,7 +29,7 @@ const events = [{ extraText: 'Look at these fun pics I found from a few years ago. Good times.', extraImages: [ '/assets/images/wireframe/image.png', - '/assets/images/wireframe/image.png', + '/assets/images/wireframe/image-text.png', ], }] diff --git a/docs/app/Examples/views/Feed/Types/FeedExampleShorthand.js b/docs/app/Examples/views/Feed/Types/FeedExampleShorthand.js index 97e6ed00d4..f71bebd43a 100644 --- a/docs/app/Examples/views/Feed/Types/FeedExampleShorthand.js +++ b/docs/app/Examples/views/Feed/Types/FeedExampleShorthand.js @@ -15,7 +15,7 @@ const events = [ summary: 'Helen Troy added 2 new illustrations', extraImages: [ '/assets/images/wireframe/image.png', - '/assets/images/wireframe/image.png', + '/assets/images/wireframe/image-text.png', ], }, { @@ -42,7 +42,7 @@ const events = [ summary: 'Justen Kitsune added 2 new photos of you', extraImages: [ '/assets/images/wireframe/image.png', - '/assets/images/wireframe/image.png', + '/assets/images/wireframe/image-text.png', ], }, ] diff --git a/src/addons/Confirm/Confirm.js b/src/addons/Confirm/Confirm.js index 9b48c752d6..efdc80e429 100644 --- a/src/addons/Confirm/Confirm.js +++ b/src/addons/Confirm/Confirm.js @@ -59,16 +59,22 @@ class Confirm extends Component { } handleCancel = e => { - const { onCancel } = this.props - - if (onCancel) onCancel(e, this.props) + _.invoke(this.props, 'onCancel', e, this.props) } - handleConfirm = e => { - const { onConfirm } = this.props + handleCancelOverrides = predefinedProps => ({ + onClick: (e, buttonProps) => { + _.invoke(predefinedProps, 'onClick', e, buttonProps) + this.handleCancel(e) + }, + }) - if (onConfirm) onConfirm(e, this.props) - } + handleConfirmOverrides = predefinedProps => ({ + onClick: (e, buttonProps) => { + _.invoke(predefinedProps, 'onClick', e, buttonProps) + _.invoke(this.props, 'onConfirm', e, this.props) + }, + }) render() { const { @@ -91,10 +97,10 @@ class Confirm extends Component { {Modal.Header.create(header)} {Modal.Content.create(content)} - {Button.create(cancelButton, { onClick: this.handleCancel })} + {Button.create(cancelButton, { overrideProps: this.handleCancelOverrides })} {Button.create(confirmButton, { - onClick: this.handleConfirm, - primary: true, + defaultProps: { primary: true }, + overrideProps: this.handleConfirmOverrides, })} diff --git a/src/collections/Breadcrumb/Breadcrumb.js b/src/collections/Breadcrumb/Breadcrumb.js index ccc5bf8971..989acd0c22 100644 --- a/src/collections/Breadcrumb/Breadcrumb.js +++ b/src/collections/Breadcrumb/Breadcrumb.js @@ -34,34 +34,18 @@ function Breadcrumb(props) { const rest = getUnhandledProps(Breadcrumb, props) const ElementType = getElementType(Breadcrumb, props) - if (!_.isNil(children)) { - return {children} - } + if (!_.isNil(children)) return {children} const childElements = [] _.each(sections, (section, index) => { // section - const breadcrumbSection = BreadcrumbSection.create(section) - childElements.push(breadcrumbSection) + const breadcrumbElement = BreadcrumbSection.create(section) + childElements.push(breadcrumbElement) // divider if (index !== sections.length - 1) { - // TODO generate a key from breadcrumbSection.props once this is merged: - // https://github.com/Semantic-Org/Semantic-UI-React/pull/645 - // - // Stringify the props of the section as the divider key. - // - // Section: { content: 'Home', link: true, onClick: handleClick } - // Divider key: content=Home|link=true|onClick=handleClick - let key - if (section.key) { - key = `${section.key}_divider` - } else { - key = _.map(breadcrumbSection.props, (v, k) => { - return `${k}=${typeof v === 'function' ? v.name || 'func' : v}` - }).join('|') - } + const key = `${breadcrumbElement.key}_divider` || JSON.stringify(section) childElements.push(BreadcrumbDivider.create({ content: divider, icon, key })) } }) diff --git a/src/collections/Breadcrumb/BreadcrumbDivider.js b/src/collections/Breadcrumb/BreadcrumbDivider.js index 712e7ce34d..5fe1cba7da 100644 --- a/src/collections/Breadcrumb/BreadcrumbDivider.js +++ b/src/collections/Breadcrumb/BreadcrumbDivider.js @@ -26,13 +26,14 @@ function BreadcrumbDivider(props) { const rest = getUnhandledProps(BreadcrumbDivider, props) const ElementType = getElementType(BreadcrumbDivider, props) - const iconElement = Icon.create(icon, { ...rest, className: classes }) - if (iconElement) return iconElement + if (!_.isNil(icon)) return Icon.create(icon, { defaultProps: { ...rest, className: classes } }) + if (!_.isNil(content)) return {content} - let breadcrumbContent = content - if (_.isNil(content)) breadcrumbContent = _.isNil(children) ? '/' : children - - return {breadcrumbContent} + return ( + + {_.isNil(children) ? '/' : children} + + ) } BreadcrumbDivider._meta = { diff --git a/src/collections/Breadcrumb/BreadcrumbSection.js b/src/collections/Breadcrumb/BreadcrumbSection.js index 13a06bc429..e9e2cdb22d 100644 --- a/src/collections/Breadcrumb/BreadcrumbSection.js +++ b/src/collections/Breadcrumb/BreadcrumbSection.js @@ -94,4 +94,4 @@ export default class BreadcrumbSection extends Component { } } -BreadcrumbSection.create = createShorthandFactory(BreadcrumbSection, content => ({ content, link: true }), true) +BreadcrumbSection.create = createShorthandFactory(BreadcrumbSection, content => ({ content, link: true })) diff --git a/src/collections/Menu/Menu.js b/src/collections/Menu/Menu.js index aaca9c9e32..7fe27edfd3 100644 --- a/src/collections/Menu/Menu.js +++ b/src/collections/Menu/Menu.js @@ -133,24 +133,27 @@ class Menu extends Component { static Item = MenuItem static Menu = MenuMenu - handleItemClick = (e, itemProps) => { - const { index } = itemProps - const { items, onItemClick } = this.props + handleItemOverrides = predefinedProps => ({ + onClick: (e, itemProps) => { + const { index } = itemProps - this.trySetState({ activeIndex: index }) + this.trySetState({ activeIndex: index }) - if (_.get(items[index], 'onClick')) items[index].onClick(e, itemProps) - if (onItemClick) onItemClick(e, itemProps) - } + _.invoke(predefinedProps, 'onClick', e, itemProps) + _.invoke(this.props, 'onItemClick', e, itemProps) + }, + }) renderItems() { const { items } = this.props const { activeIndex } = this.state return _.map(items, (item, index) => MenuItem.create(item, { - active: activeIndex === index, - index, - onClick: this.handleItemClick, + defaultProps: { + active: activeIndex === index, + index, + }, + overrideProps: this.handleItemOverrides, })) } diff --git a/src/collections/Menu/MenuItem.js b/src/collections/Menu/MenuItem.js index c523d3746c..48352002eb 100644 --- a/src/collections/Menu/MenuItem.js +++ b/src/collections/Menu/MenuItem.js @@ -136,4 +136,4 @@ export default class MenuItem extends Component { } } -MenuItem.create = createShorthandFactory(MenuItem, val => ({ content: val, name: val }), true) +MenuItem.create = createShorthandFactory(MenuItem, val => ({ content: val, name: val })) diff --git a/src/collections/Message/Message.js b/src/collections/Message/Message.js index 02e1ebf44d..55ffee8358 100644 --- a/src/collections/Message/Message.js +++ b/src/collections/Message/Message.js @@ -3,7 +3,7 @@ import _ from 'lodash' import React, { Component, PropTypes } from 'react' import { - createShorthand, + createHTMLParagraph, customPropTypes, getElementType, getUnhandledProps, @@ -181,7 +181,7 @@ export default class Message extends Component { {MessageHeader.create(header)} {MessageList.create(list)} - {createShorthand('p', val => ({ children: val }), content)} + {createHTMLParagraph(content)} )} diff --git a/src/collections/Message/MessageItem.js b/src/collections/Message/MessageItem.js index 2909d348ca..366ce51e17 100644 --- a/src/collections/Message/MessageItem.js +++ b/src/collections/Message/MessageItem.js @@ -50,6 +50,6 @@ MessageItem.defaultProps = { as: 'li', } -MessageItem.create = createShorthandFactory(MessageItem, content => ({ content }), true) +MessageItem.create = createShorthandFactory(MessageItem, content => ({ content })) export default MessageItem diff --git a/src/collections/Table/Table.js b/src/collections/Table/Table.js index 607e8391b4..9c207820d0 100644 --- a/src/collections/Table/Table.js +++ b/src/collections/Table/Table.js @@ -90,7 +90,7 @@ function Table(props) { return ( - {headerRow && {TableRow.create(headerRow, { cellAs: 'th' })}} + {headerRow && {TableRow.create(headerRow, { defaultProps: { cellAs: 'th' } })}} {renderBodyRow && _.map(tableData, (data, index) => TableRow.create(renderBodyRow(data, index)))} diff --git a/src/collections/Table/TableCell.js b/src/collections/Table/TableCell.js index 65798911ea..c9fb3565ba 100644 --- a/src/collections/Table/TableCell.js +++ b/src/collections/Table/TableCell.js @@ -132,6 +132,6 @@ TableCell.propTypes = { width: PropTypes.oneOf(SUI.WIDTHS), } -TableCell.create = createShorthandFactory(TableCell, content => ({ content }), true) +TableCell.create = createShorthandFactory(TableCell, content => ({ content })) export default TableCell diff --git a/src/collections/Table/TableRow.js b/src/collections/Table/TableRow.js index dfb38d6aed..22d31b8a96 100644 --- a/src/collections/Table/TableRow.js +++ b/src/collections/Table/TableRow.js @@ -54,7 +54,7 @@ function TableRow(props) { return ( - {_.map(cells, (cell) => TableCell.create(cell, { as: cellAs }))} + {_.map(cells, (cell) => TableCell.create(cell, { defaultProps: { as: cellAs } }))} ) } @@ -111,6 +111,6 @@ TableRow.propTypes = { warning: PropTypes.bool, } -TableRow.create = createShorthandFactory(TableRow, cells => ({ cells }), true) +TableRow.create = createShorthandFactory(TableRow, cells => ({ cells })) export default TableRow diff --git a/src/elements/Button/Button.js b/src/elements/Button/Button.js index 7c6aad7d4c..52b82a650c 100644 --- a/src/elements/Button/Button.js +++ b/src/elements/Button/Button.js @@ -255,10 +255,10 @@ class Button extends Component { ) } - const labelElement = Label.create(label, { + const labelElement = Label.create(label, { defaultProps: { basic: true, pointing: labelPosition === 'left' ? 'right' : 'left', - }) + } }) if (labelElement) { const classes = cx('ui', baseClasses, 'button', className) diff --git a/src/elements/Input/Input.js b/src/elements/Input/Input.js index dffb1fc0da..3abb1619fd 100644 --- a/src/elements/Input/Input.js +++ b/src/elements/Input/Input.js @@ -195,28 +195,22 @@ class Input extends Component { // Render Shorthand // ---------------------------------------- - const actionElement = Button.create(action, elProps => ({ - className: cx( - // all action components should have the button className - !_.includes(elProps.className, 'button') && 'button', - ), - })) + const actionElement = Button.create(action, { defaultProps: { className: 'button' } }) const iconElement = Icon.create(icon) - const labelElement = Label.create(label, elProps => ({ + const labelElement = Label.create(label, { defaultProps: { className: cx( - // all label components should have the label className - !_.includes(elProps.className, 'label') && 'label', + 'label', // add 'left|right corner' _.includes(labelPosition, 'corner') && labelPosition, ), - })) + } }) return ( {actionPosition === 'left' && actionElement} {iconPosition === 'left' && iconElement} {labelPosition !== 'right' && labelElement} - {createHTMLInput(input || type, htmlInputProps)} + {createHTMLInput(input || type, { defaultProps: htmlInputProps })} {actionPosition !== 'left' && actionElement} {iconPosition !== 'left' && iconElement} {labelPosition === 'right' && labelElement} diff --git a/src/elements/Label/Label.js b/src/elements/Label/Label.js index 074e9e19aa..f3b2284be7 100644 --- a/src/elements/Label/Label.js +++ b/src/elements/Label/Label.js @@ -133,11 +133,12 @@ export default class Label extends Component { if (onClick) onClick(e, this.props) } - handleRemove = (e) => { - const { onRemove } = this.props - - if (onRemove) onRemove(e, this.props) - } + handleIconOverrides = predefinedProps => ({ + onClick: e => { + _.invoke(predefinedProps, 'onClick', e) + _.invoke(this.props, 'onRemove', e, this.props) + }, + }) render() { const { @@ -202,7 +203,7 @@ export default class Label extends Component { {typeof image !== 'boolean' && Image.create(image)} {content} {createShorthand(LabelDetail, val => ({ content: val }), detail)} - {onRemove && Icon.create(removeIconShorthand, { onClick: this.handleRemove })} + {onRemove && Icon.create(removeIconShorthand, { overrideProps: this.handleIconOverrides })} ) } diff --git a/src/elements/List/ListItem.js b/src/elements/List/ListItem.js index 8c6de809de..6c0b9d3414 100644 --- a/src/elements/List/ListItem.js +++ b/src/elements/List/ListItem.js @@ -149,6 +149,6 @@ ListItem.propTypes = { value: PropTypes.string, } -ListItem.create = createShorthandFactory(ListItem, content => ({ content }), true) +ListItem.create = createShorthandFactory(ListItem, content => ({ content })) export default ListItem diff --git a/src/lib/factories.js b/src/lib/factories.js index 55d03e600d..586221ae77 100644 --- a/src/lib/factories.js +++ b/src/lib/factories.js @@ -12,16 +12,18 @@ import React, { cloneElement, isValidElement } from 'react' * @param {function|string} Component A ReactClass or string * @param {function} mapValueToProps A function that maps a primitive value to the Component props * @param {string|object|function} val The value to create a ReactElement from - * @param {object|function} [defaultProps={}] Default props object or function (called with regular props). - * @param {boolean} [generateKey=false] Whether or not to generate a child key, useful for collections. + * @param {Object} [options={}] + * @param {object} [options.defaultProps={}] Default props object + * @param {object|function} [options.overrideProps={}] Override props object or function (called with regular props) * @returns {object|null} */ -export function createShorthand(Component, mapValueToProps, val, defaultProps = {}, generateKey = false) { +export function createShorthand(Component, mapValueToProps, val, options = {}) { if (typeof Component !== 'function' && typeof Component !== 'string') { throw new Error('createShorthandFactory() Component must be a string or function.') } - // short circuit for disabling shorthand - if (val === null) return null + // short circuit noop values + if (_.isNil(val) || _.isBoolean(val)) return null + const valIsString = _.isString(val) const valIsNumber = _.isNumber(val) @@ -29,30 +31,45 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = const isPropsObject = _.isPlainObject(val) const isPrimitiveValue = valIsString || valIsNumber || _.isArray(val) + // unhandled type return null + /* eslint-disable no-console */ + if (!isReactElement && !isPropsObject && !isPrimitiveValue) { + if (process.env.NODE_ENV !== 'production') { + console.error([ + 'Shorthand value must be a string|number|array|object|ReactElement.', + ' Use null|undefined|boolean for none', + ` Received ${typeof val}.`, + ].join('')) + } + return null + } + /* eslint-enable no-console */ + // ---------------------------------------- // Build up props // ---------------------------------------- + const { defaultProps = {} } = options // User's props - const usersProps = isReactElement && val.props - || isPropsObject && val - || isPrimitiveValue && mapValueToProps(val) + const usersProps = isReactElement && val.props || isPropsObject && val || isPrimitiveValue && mapValueToProps(val) - // Default props - defaultProps = _.isFunction(defaultProps) ? defaultProps(usersProps) : defaultProps + // Override props + let { overrideProps = {} } = options + overrideProps = _.isFunction(overrideProps) ? overrideProps({ ...defaultProps, ...usersProps }) : overrideProps // Merge props /* eslint-disable react/prop-types */ - const props = { ...defaultProps, ...usersProps } + const props = { ...defaultProps, ...usersProps, ...overrideProps } // Merge className - if (usersProps.className && defaultProps.className) { - props.className = cx(defaultProps.className, usersProps.className) + if (defaultProps.className || overrideProps.className || usersProps.className) { + const mergedClassesNames = cx(defaultProps.className, overrideProps.className, usersProps.className) + props.className = _.uniq(mergedClassesNames.split(' ')).join(' ') } // Merge style - if (usersProps.style && defaultProps.style) { - props.style = { ...defaultProps.style, ...usersProps.style } + if (defaultProps.style || overrideProps.style || usersProps.style) { + props.style = { ...defaultProps.style, ...usersProps.style, ...overrideProps.style } } // ---------------------------------------- @@ -67,7 +84,7 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = // apply and consume the childKey props.key = typeof childKey === 'function' ? childKey(props) : childKey delete props.childKey - } else if (generateKey && (valIsString || valIsNumber)) { + } else if (valIsString || valIsNumber) { // use string/number shorthand values as the key props.key = val } @@ -83,9 +100,6 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = // Create ReactElements from built up props if (isPrimitiveValue || isPropsObject) return - - // Otherwise null - return null } // ============================================================ @@ -93,26 +107,25 @@ export function createShorthand(Component, mapValueToProps, val, defaultProps = // ============================================================ /** - * Creates a `createShorthand` function that is waiting for a value and defaultProps. + * Creates a `createShorthand` function that is waiting for a value and options. * * @param {function|string} Component A ReactClass or string * @param {function} mapValueToProps A function that maps a primitive value to the Component props - * @param {boolean} [generateKey] Whether or not to generate a child key, useful for collections. * @returns {function} A shorthand factory function waiting for `val` and `defaultProps`. */ -export function createShorthandFactory(Component, mapValueToProps, generateKey) { +export function createShorthandFactory(Component, mapValueToProps) { if (typeof Component !== 'function' && typeof Component !== 'string') { throw new Error('createShorthandFactory() Component must be a string or function.') } - return (val, defaultProps) => { - return createShorthand(Component, mapValueToProps, val, defaultProps, generateKey) - } + return (val, options) => createShorthand(Component, mapValueToProps, val, options) } // ============================================================ // HTML Factories // ============================================================ -export const createHTMLImage = createShorthandFactory('img', value => ({ src: value })) -export const createHTMLInput = createShorthandFactory('input', value => ({ type: value })) -export const createHTMLLabel = createShorthandFactory('label', value => ({ children: value })) +export const createHTMLDivision = createShorthandFactory('div', val => ({ children: val })) +export const createHTMLImage = createShorthandFactory('img', val => ({ src: val })) +export const createHTMLInput = createShorthandFactory('input', val => ({ type: val })) +export const createHTMLLabel = createShorthandFactory('label', val => ({ children: val })) +export const createHTMLParagraph = createShorthandFactory('p', val => ({ children: val })) diff --git a/src/modules/Dropdown/Dropdown.js b/src/modules/Dropdown/Dropdown.js index 0c4c243631..34d4723beb 100644 --- a/src/modules/Dropdown/Dropdown.js +++ b/src/modules/Dropdown/Dropdown.js @@ -4,7 +4,6 @@ import React, { Children, cloneElement, PropTypes } from 'react' import { AutoControlledComponent as Component, - createShorthand, customPropTypes, getElementType, getUnhandledProps, @@ -1118,7 +1117,7 @@ export default class Dropdown extends Component { // if no item could be found for a given state value the selected item will be undefined // compact the selectedItems so we only have actual objects left return _.map(_.compact(selectedItems), (item, index) => { - const defaultLabelProps = { + const defaultProps = { active: item.value === selectedLabel, as: 'a', key: item.value, @@ -1128,8 +1127,8 @@ export default class Dropdown extends Component { } return Label.create( - renderLabel(item, index, defaultLabelProps), - defaultLabelProps, + renderLabel(item, index, defaultProps), + { defaultProps } ) }) } @@ -1176,7 +1175,7 @@ export default class Dropdown extends Component { return ( - {createShorthand(DropdownHeader, val => ({ content: val }), header)} + {DropdownHeader.create(header)} {this.renderOptions()} ) diff --git a/src/modules/Dropdown/DropdownHeader.js b/src/modules/Dropdown/DropdownHeader.js index 86b326c16e..1af3548368 100644 --- a/src/modules/Dropdown/DropdownHeader.js +++ b/src/modules/Dropdown/DropdownHeader.js @@ -3,6 +3,7 @@ import _ from 'lodash' import React, { PropTypes } from 'react' import { + createShorthandFactory, customPropTypes, getElementType, getUnhandledProps, @@ -25,9 +26,7 @@ function DropdownHeader(props) { const rest = getUnhandledProps(DropdownHeader, props) const ElementType = getElementType(DropdownHeader, props) - if (!_.isNil(children)) { - return {children} - } + if (!_.isNil(children)) return {children} return ( @@ -60,4 +59,6 @@ DropdownHeader.propTypes = { icon: customPropTypes.itemShorthand, } +DropdownHeader.create = createShorthandFactory(DropdownHeader, content => ({ content })) + export default DropdownHeader diff --git a/src/modules/Dropdown/DropdownItem.js b/src/modules/Dropdown/DropdownItem.js index dc4f45629f..0fa3b958e9 100644 --- a/src/modules/Dropdown/DropdownItem.js +++ b/src/modules/Dropdown/DropdownItem.js @@ -140,13 +140,13 @@ export default class DropdownItem extends Component { 'span', val => ({ children: val }), description, - props => ({ className: 'description' }) + { defaultProps: { className: 'description' } } ) const textElement = createShorthand( 'span', val => ({ children: val }), content || text, - props => ({ className: 'text' }) + { defaultProps: { className: 'text' } } ) return ( diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index 751b2a83ed..d6a7563792 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -151,6 +151,13 @@ class Modal extends Component { this.trySetState({ open: false }) } + handleIconOverrides = predefinedProps => ({ + onClick: e => { + _.invoke(predefinedProps, 'onClick', e) + this.handleClose(e) + }, + }) + handleOpen = (e) => { debug('open()') @@ -266,7 +273,7 @@ class Modal extends Component { const closeIconName = closeIcon === true ? 'close' : closeIcon const modalJSX = ( - {Icon.create(closeIconName, { onClick: this.handleClose })} + {Icon.create(closeIconName, { overrideProps: this.handleIconOverrides })} {children} ) diff --git a/src/modules/Progress/Progress.js b/src/modules/Progress/Progress.js index 9a2979d152..63ac08047f 100644 --- a/src/modules/Progress/Progress.js +++ b/src/modules/Progress/Progress.js @@ -3,7 +3,7 @@ import _ from 'lodash' import React, { Component, PropTypes } from 'react' import { - createShorthand, + createHTMLDivision, customPropTypes, getElementType, getUnhandledProps, @@ -138,7 +138,7 @@ class Progress extends Component { const { children, label } = this.props if (!_.isNil(children)) return
{children}
- return createShorthand('div', val => ({ children: val }), label, { className: 'label' }) + return createHTMLDivision(label, { defaultProps: { className: 'label' } }) } renderProgress = percent => { diff --git a/src/modules/Search/Search.js b/src/modules/Search/Search.js index 7e27fbf9e3..b8065b41f5 100644 --- a/src/modules/Search/Search.js +++ b/src/modules/Search/Search.js @@ -517,7 +517,7 @@ export default class Search extends Component { const { icon, input } = this.props const { value } = this.state - return Input.create(input, { + return Input.create(input, { defaultProps: { ...rest, icon, input: { className: 'prompt', tabIndex: '0', autoComplete: 'off' }, @@ -526,7 +526,7 @@ export default class Search extends Component { onClick: this.handleInputClick, onFocus: this.handleFocus, value, - }) + } }) } renderNoResults = () => { diff --git a/test/specs/collections/Breadcrumb/Breadcrumb-test.js b/test/specs/collections/Breadcrumb/Breadcrumb-test.js index 1bf2f8e5f7..1f6daf118f 100644 --- a/test/specs/collections/Breadcrumb/Breadcrumb-test.js +++ b/test/specs/collections/Breadcrumb/Breadcrumb-test.js @@ -1,11 +1,13 @@ import React from 'react' import Breadcrumb from 'src/collections/Breadcrumb/Breadcrumb' +import BreadcrumbDivider from 'src/collections/Breadcrumb/BreadcrumbDivider' +import BreadcrumbSection from 'src/collections/Breadcrumb/BreadcrumbSection' import * as common from 'test/specs/commonTests' -import { sandbox } from 'test/utils' describe('Breadcrumb', () => { common.isConformant(Breadcrumb) + common.hasSubComponents(Breadcrumb, [BreadcrumbDivider, BreadcrumbSection]) common.hasUIClassName(Breadcrumb) common.rendersChildren(Breadcrumb) @@ -22,40 +24,14 @@ describe('Breadcrumb', () => { it('renders children with `sections` prop', () => { const wrapper = shallow() - wrapper.should.have.exactly(1).descendants('BreadcrumbDivider') - wrapper.should.have.exactly(2).descendants('BreadcrumbSection') + wrapper.should.have.exactly(1).descendants(BreadcrumbDivider) + wrapper.should.have.exactly(2).descendants(BreadcrumbSection) }) it('renders defined divider with `divider` prop', () => { const wrapper = mount() - const divider = wrapper.find('BreadcrumbDivider').first() + const divider = wrapper.find(BreadcrumbDivider).first() divider.should.contain.text('>') }) - - it('generates a divider key from section props', () => { - sandbox.spy(Breadcrumb.Divider, 'create') - shallow() - - Breadcrumb.Divider.create.should.have.been.calledWithMatch({ - content: undefined, - icon: undefined, - key: 'content=Home|link=true', - }) - - Breadcrumb.Divider.create.restore() - }) - - it('generates a divider key from section key', () => { - sandbox.spy(Breadcrumb.Divider, 'create') - shallow() - - Breadcrumb.Divider.create.should.have.been.calledWithMatch({ - content: undefined, - icon: undefined, - key: 'sectionA_divider', - }) - - Breadcrumb.Divider.create.restore() - }) }) diff --git a/test/specs/collections/Menu/Menu-test.js b/test/specs/collections/Menu/Menu-test.js index 56daa21fa8..45cfaa7de2 100644 --- a/test/specs/collections/Menu/Menu-test.js +++ b/test/specs/collections/Menu/Menu-test.js @@ -107,27 +107,35 @@ describe('Menu', () => { }) describe('onItemClick', () => { - const items = [ - { key: 'home', name: 'home' }, - { key: 'users', name: 'users' }, - ] - it('can be omitted', () => { - const click = () => mount().find('MenuItem').first().simulate('click') + const click = () => mount() + .find('MenuItem') + .first() + .simulate('click') expect(click).to.not.throw() }) it('is called with (e, { name, index }) when clicked', () => { - const spy = sandbox.spy() const event = { target: null } - const props = { name: 'home', index: 0 } + const itemSpy = sandbox.spy() + const menuSpy = sandbox.spy() + + const items = [ + { key: 'home', name: 'home' }, + { key: 'users', name: 'users', onClick: itemSpy }, + ] + const matchProps = { index: 1, name: 'users' } - mount().find('MenuItem').first() + mount() + .find('MenuItem') + .last() .simulate('click', event) - spy.should.have.been.calledOnce() - spy.should.have.been.calledWithMatch(event, props) + itemSpy.should.have.been.calledOnce() + itemSpy.should.have.been.calledWithMatch(event, matchProps) + menuSpy.should.have.been.calledOnce() + menuSpy.should.have.been.calledWithMatch(event, matchProps) }) }) }) diff --git a/test/specs/commonTests/implementsCommonProps.js b/test/specs/commonTests/implementsCommonProps.js index 084466d857..32e03e0116 100644 --- a/test/specs/commonTests/implementsCommonProps.js +++ b/test/specs/commonTests/implementsCommonProps.js @@ -18,15 +18,14 @@ import helpers from './commonHelpers' * @param {string|function} [options.ShorthandComponent] The component that should be rendered from the shorthand value. * @param {function} [options.mapValueToProps] A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. */ export const implementsButtonProp = (Component, options = {}) => { implementsShorthandProp(Component, { propKey: 'button', ShorthandComponent: Button, mapValueToProps: val => ({ content: val }), - requiredProps: {}, - shorthandDefaultProps: {}, ...options, }) } @@ -40,15 +39,14 @@ export const implementsButtonProp = (Component, options = {}) => { * @param {string|function} [options.ShorthandComponent] The component that should be rendered from the shorthand value. * @param {function} [options.mapValueToProps] A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. */ export const implementsHTMLInputProp = (Component, options = {}) => { implementsShorthandProp(Component, { propKey: 'input', ShorthandComponent: 'input', mapValueToProps: val => ({ type: val }), - requiredProps: {}, - shorthandDefaultProps: {}, ...options, }) } @@ -62,15 +60,14 @@ export const implementsHTMLInputProp = (Component, options = {}) => { * @param {string|function} [options.ShorthandComponent] The component that should be rendered from the shorthand value. * @param {function} [options.mapValueToProps] A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. */ export const implementsHTMLLabelProp = (Component, options = {}) => { implementsShorthandProp(Component, { propKey: 'label', ShorthandComponent: 'label', mapValueToProps: val => ({ children: val }), - requiredProps: {}, - shorthandDefaultProps: {}, ...options, }) } @@ -84,15 +81,14 @@ export const implementsHTMLLabelProp = (Component, options = {}) => { * @param {string|function} [options.ShorthandComponent] The component that should be rendered from the shorthand value. * @param {function} [options.mapValueToProps] A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. */ export const implementsIconProp = (Component, options = {}) => { implementsShorthandProp(Component, { propKey: 'icon', ShorthandComponent: Icon, mapValueToProps: val => ({ name: val }), - requiredProps: {}, - shorthandDefaultProps: {}, ...options, }) } @@ -106,15 +102,14 @@ export const implementsIconProp = (Component, options = {}) => { * @param {string|function} [options.ShorthandComponent] The component that should be rendered from the shorthand value. * @param {function} [options.mapValueToProps] A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. */ export const implementsImageProp = (Component, options = {}) => { implementsShorthandProp(Component, { propKey: 'image', ShorthandComponent: Image, mapValueToProps: val => ({ src: val }), - requiredProps: {}, - shorthandDefaultProps: {}, ...options, }) } @@ -128,15 +123,14 @@ export const implementsImageProp = (Component, options = {}) => { * @param {string|function} [options.ShorthandComponent] The component that should be rendered from the shorthand value. * @param {function} [options.mapValueToProps] A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. */ export const implementsLabelProp = (Component, options = {}) => { implementsShorthandProp(Component, { propKey: 'label', ShorthandComponent: Label, mapValueToProps: val => ({ content: val }), - requiredProps: {}, - shorthandDefaultProps: {}, ...options, }) } diff --git a/test/specs/commonTests/implementsShorthandProp.js b/test/specs/commonTests/implementsShorthandProp.js index 9e88fff43e..b783fdc4e0 100644 --- a/test/specs/commonTests/implementsShorthandProp.js +++ b/test/specs/commonTests/implementsShorthandProp.js @@ -20,8 +20,9 @@ const shorthandComponentName = ShorthandComponent => { * @param {string|function} options.ShorthandComponent The component that should be rendered from the shorthand value. * @param {function} options.mapValueToProps A function that maps a primitive value to the Component props * @param {Object} [options.requiredProps={}] Props required to render the component. - * @param {Object|function} [options.shorthandDefaultProps={}] Props required to render the shorthand component. - * @param {Object} [options.alwaysPresent] Whether or not the shorthand exists by default + * @param {Object} [options.shorthandDefaultProps] Default props for the shorthand component. + * @param {Object} [options.shorthandOverrideProps] Override props for the shorthand component. + * @param {boolean} [options.alwaysPresent] Whether or not the shorthand exists by default */ export default (Component, options = {}) => { const { @@ -30,6 +31,7 @@ export default (Component, options = {}) => { propKey, ShorthandComponent, shorthandDefaultProps = {}, + shorthandOverrideProps = {}, requiredProps = {}, } = options const { assertRequired } = helpers('implementsShorthandProp', Component) @@ -42,10 +44,13 @@ export default (Component, options = {}) => { const name = shorthandComponentName(ShorthandComponent) const assertValidShorthand = (value) => { - const renderedShorthand = createShorthand(ShorthandComponent, mapValueToProps, value, shorthandDefaultProps) + const shorthandElement = createShorthand(ShorthandComponent, mapValueToProps, value, { + defaultProps: shorthandDefaultProps, + overrideProps: shorthandOverrideProps, + }) const element = createElement(Component, { ...requiredProps, [propKey]: value }) - shallow(element).should.contain(renderedShorthand) + shallow(element).should.contain(shorthandElement) } if (alwaysPresent || Component.defaultProps && Component.defaultProps[propKey]) { diff --git a/test/specs/docs/examples-test.js b/test/specs/docs/examples-test.js index 7c63e2514d..5b96e4ae19 100644 --- a/test/specs/docs/examples-test.js +++ b/test/specs/docs/examples-test.js @@ -25,6 +25,7 @@ describe('examples', () => { const filename = path.replace(/^.*\/(\w+\.js)$/, '$1') it(`${filename} renders without console messages`, () => { + // TODO also render the example's path in a just as the docs do mount(createElement(exampleContext(path).default)) console.info.should.not.have.been.called(`Console info: ${console.info.args}`) diff --git a/test/specs/elements/Input/Input-test.js b/test/specs/elements/Input/Input-test.js index 23400ee33d..f5648fbad1 100644 --- a/test/specs/elements/Input/Input-test.js +++ b/test/specs/elements/Input/Input-test.js @@ -1,5 +1,3 @@ -import cx from 'classnames' -import _ from 'lodash' import React from 'react' import Input from 'src/elements/Input/Input' @@ -56,19 +54,11 @@ describe('Input', () => { common.rendersChildren(Input) common.implementsLabelProp(Input, { - shorthandDefaultProps: elProps => ({ - className: cx({ - label: !_.includes(elProps.className, 'label'), - }), - }), + shorthandDefaultProps: { className: 'label' }, }) common.implementsButtonProp(Input, { propKey: 'action', - shorthandDefaultProps: elProps => ({ - className: cx({ - button: !_.includes(elProps.className, 'button'), - }), - }), + shorthandDefaultProps: { className: 'button' }, }) common.implementsCreateMethod(Input) common.implementsHTMLInputProp(Input, { diff --git a/test/specs/elements/Label/Label-test.js b/test/specs/elements/Label/Label-test.js index 16a5015489..7ec728a0ae 100644 --- a/test/specs/elements/Label/Label-test.js +++ b/test/specs/elements/Label/Label-test.js @@ -70,6 +70,23 @@ describe('Label', () => { .find('Icon') .should.have.prop('data-foo', true) }) + + it('handles events on Label and Icon', () => { + const event = { target: null } + const iconSpy = sandbox.spy() + const labelSpy = sandbox.spy() + + const iconProps = { 'data-foo': true, onClick: iconSpy } + const labelProps = { onRemove: labelSpy, removeIcon: iconProps } + + mount(