From bf0945703f232a0e2760f15e01c980dc04f7ec8b Mon Sep 17 00:00:00 2001 From: Johanna Altmann Date: Mon, 3 Apr 2017 11:39:59 -0400 Subject: [PATCH] feat(List): handler for (#1530) feat(List): handler for --- src/elements/List/List.d.ts | 10 +- src/elements/List/List.js | 220 ++++++++------- src/elements/List/ListContent.js | 4 +- src/elements/List/ListItem.d.ts | 8 + src/elements/List/ListItem.js | 250 +++++++++--------- test/specs/elements/List/List-test.js | 35 ++- test/specs/elements/List/ListContent-test.js | 2 + .../elements/List/ListDescription-test.js | 2 + test/specs/elements/List/ListHeader-test.js | 2 + test/specs/elements/List/ListItem-test.js | 23 +- 10 files changed, 328 insertions(+), 228 deletions(-) diff --git a/src/elements/List/List.d.ts b/src/elements/List/List.d.ts index 05d1f3b2a4..e261450f79 100644 --- a/src/elements/List/List.d.ts +++ b/src/elements/List/List.d.ts @@ -9,7 +9,7 @@ import { default as ListContent } from './ListContent'; import { default as ListDescription } from './ListDescription'; import { default as ListHeader } from './ListHeader'; import { default as ListIcon } from './ListIcon'; -import { default as ListItem } from './ListItem'; +import { default as ListItem, ListItemProps } from './ListItem'; import { default as ListList } from './ListList'; export interface ListProps { @@ -51,6 +51,14 @@ export interface ListProps { /** A list can be specially formatted for navigation links. */ link?: boolean; + /** + * onClick handler for ListItem. Mutually exclusive with children. + * + * @param {SyntheticEvent} event - React's original SyntheticEvent. + * @param {object} data - All item props. + */ + onItemClick?: (event: React.MouseEvent, data: ListItemProps) => void; + /** A list can be ordered numerically. */ ordered?: boolean; diff --git a/src/elements/List/List.js b/src/elements/List/List.js index 31dc725c05..0b03d560df 100644 --- a/src/elements/List/List.js +++ b/src/elements/List/List.js @@ -1,6 +1,6 @@ import cx from 'classnames' import _ from 'lodash' -import React, { PropTypes } from 'react' +import React, { Component, PropTypes } from 'react' import { customPropTypes, @@ -23,124 +23,144 @@ import ListList from './ListList' /** * A list groups related content. */ -function List(props) { - const { - animated, - bulleted, - celled, - children, - className, - divided, - floated, - horizontal, - inverted, - items, - link, - ordered, - relaxed, - selection, - size, - verticalAlign, - } = props - - const classes = cx( - 'ui', - size, - useKeyOnly(animated, 'animated'), - useKeyOnly(bulleted, 'bulleted'), - useKeyOnly(celled, 'celled'), - useKeyOnly(divided, 'divided'), - useKeyOnly(horizontal, 'horizontal'), - useKeyOnly(inverted, 'inverted'), - useKeyOnly(link, 'link'), - useKeyOnly(ordered, 'ordered'), - useKeyOnly(selection, 'selection'), - useKeyOrValueAndKey(relaxed, 'relaxed'), - useValueAndKey(floated, 'floated'), - useVerticalAlignProp(verticalAlign), - 'list', - className, - ) - const rest = getUnhandledProps(List, props) - const ElementType = getElementType(List, props) - - if (!_.isNil(children)) { - return {children} - } +class List extends Component { + static propTypes = { + /** An element type to render as (string or function). */ + as: customPropTypes.as, - return ( - - {_.map(items, (item) => ListItem.create(item))} - - ) -} + /** A list can animate to set the current item apart from the list. */ + animated: PropTypes.bool, -List._meta = { - name: 'List', - type: META.TYPES.ELEMENT, -} + /** A list can mark items with a bullet. */ + bulleted: PropTypes.bool, -List.propTypes = { - /** An element type to render as (string or function). */ - as: customPropTypes.as, + /** A list can divide its items into cells. */ + celled: PropTypes.bool, - /** A list can animate to set the current item apart from the list. */ - animated: PropTypes.bool, + /** Primary content. */ + children: PropTypes.node, - /** A list can mark items with a bullet. */ - bulleted: PropTypes.bool, + /** Additional classes. */ + className: PropTypes.string, - /** A list can divide its items into cells. */ - celled: PropTypes.bool, + /** A list can show divisions between content. */ + divided: PropTypes.bool, - /** Primary content. */ - children: PropTypes.node, + /** An list can be floated left or right. */ + floated: PropTypes.oneOf(SUI.FLOATS), - /** Additional classes. */ - className: PropTypes.string, + /** A list can be formatted to have items appear horizontally. */ + horizontal: PropTypes.bool, - /** A list can show divisions between content. */ - divided: PropTypes.bool, + /** A list can be inverted to appear on a dark background. */ + inverted: PropTypes.bool, - /** An list can be floated left or right. */ - floated: PropTypes.oneOf(SUI.FLOATS), + /** Shorthand array of props for ListItem. */ + items: customPropTypes.collectionShorthand, - /** A list can be formatted to have items appear horizontally. */ - horizontal: PropTypes.bool, + /** A list can be specially formatted for navigation links. */ + link: PropTypes.bool, - /** A list can be inverted to appear on a dark background. */ - inverted: PropTypes.bool, + /** + * onClick handler for ListItem. Mutually exclusive with children. + * + * @param {SyntheticEvent} event - React's original SyntheticEvent. + * @param {object} data - All item props. + */ + onItemClick: customPropTypes.every([ + customPropTypes.disallow(['children']), + PropTypes.func, + ]), - /** Shorthand array of props for ListItem. */ - items: customPropTypes.collectionShorthand, + /** A list can be ordered numerically. */ + ordered: PropTypes.bool, - /** A list can be specially formatted for navigation links. */ - link: PropTypes.bool, + /** A list can relax its padding to provide more negative space. */ + relaxed: PropTypes.oneOfType([ + PropTypes.bool, + PropTypes.oneOf(['very']), + ]), - /** A list can be ordered numerically. */ - ordered: PropTypes.bool, + /** A selection list formats list items as possible choices. */ + selection: PropTypes.bool, - /** A list can relax its padding to provide more negative space. */ - relaxed: PropTypes.oneOfType([ - PropTypes.bool, - PropTypes.oneOf(['very']), - ]), + /** A list can vary in size. */ + size: PropTypes.oneOf(SUI.SIZES), - /** A selection list formats list items as possible choices. */ - selection: PropTypes.bool, + /** An element inside a list can be vertically aligned. */ + verticalAlign: PropTypes.oneOf(SUI.VERTICAL_ALIGNMENTS), + } - /** A list can vary in size. */ - size: PropTypes.oneOf(SUI.SIZES), + static _meta = { + name: 'List', + type: META.TYPES.ELEMENT, + } - /** An element inside a list can be vertically aligned. */ - verticalAlign: PropTypes.oneOf(SUI.VERTICAL_ALIGNMENTS), + static Content = ListContent + static Description = ListDescription + static Header = ListHeader + static Icon = ListIcon + static Item = ListItem + static List = ListList + + handleItemOverrides = predefinedProps => ({ + onClick: (e, itemProps) => { + _.invoke(predefinedProps, 'onClick', e, itemProps) + _.invoke(this.props, 'onItemClick', e, itemProps) + }, + }) + + render() { + const { + animated, + bulleted, + celled, + children, + className, + divided, + floated, + horizontal, + inverted, + items, + link, + ordered, + relaxed, + selection, + size, + verticalAlign, + } = this.props + + const classes = cx( + 'ui', + size, + useKeyOnly(animated, 'animated'), + useKeyOnly(bulleted, 'bulleted'), + useKeyOnly(celled, 'celled'), + useKeyOnly(divided, 'divided'), + useKeyOnly(horizontal, 'horizontal'), + useKeyOnly(inverted, 'inverted'), + useKeyOnly(link, 'link'), + useKeyOnly(ordered, 'ordered'), + useKeyOnly(selection, 'selection'), + useKeyOrValueAndKey(relaxed, 'relaxed'), + useValueAndKey(floated, 'floated'), + useVerticalAlignProp(verticalAlign), + 'list', + className, + ) + const rest = getUnhandledProps(List, this.props) + const ElementType = getElementType(List, this.props) + + if (!_.isNil(children)) { + return {children} + } + + return ( + + {_.map(items, item => ListItem.create(item, { overrideProps: this.handleItemOverrides }))} + + ) + } } -List.Content = ListContent -List.Description = ListDescription -List.Header = ListHeader -List.Icon = ListIcon -List.Item = ListItem -List.List = ListList - export default List diff --git a/src/elements/List/ListContent.js b/src/elements/List/ListContent.js index 256f9ba768..7eb291887d 100644 --- a/src/elements/List/ListContent.js +++ b/src/elements/List/ListContent.js @@ -38,9 +38,7 @@ function ListContent(props) { const rest = getUnhandledProps(ListContent, props) const ElementType = getElementType(ListContent, props) - if (!_.isNil(children)) { - return {children} - } + if (!_.isNil(children)) return {children} return ( diff --git a/src/elements/List/ListItem.d.ts b/src/elements/List/ListItem.d.ts index 4f1d8f47e7..da372ddd58 100644 --- a/src/elements/List/ListItem.d.ts +++ b/src/elements/List/ListItem.d.ts @@ -33,6 +33,14 @@ export interface ListItemProps { /** Shorthand for Image. */ image?: any; + /** + * Called on click. + * + * @param {SyntheticEvent} event - React's original SyntheticEvent. + * @param {object} data - All props. + */ + onClick?: (event: React.MouseEvent, data: ListItemProps) => void; + /** A value for an ordered list. */ value?: string; } diff --git a/src/elements/List/ListItem.js b/src/elements/List/ListItem.js index 6c0b9d3414..a1b9b78277 100644 --- a/src/elements/List/ListItem.js +++ b/src/elements/List/ListItem.js @@ -1,6 +1,6 @@ import cx from 'classnames' import _ from 'lodash' -import React, { isValidElement, PropTypes } from 'react' +import React, { Component, isValidElement, PropTypes } from 'react' import { createShorthandFactory, @@ -20,133 +20,147 @@ import ListIcon from './ListIcon' /** * A list item can contain a set of items. */ -function ListItem(props) { - const { - active, - children, - className, - content, - description, - disabled, - header, - icon, - image, - value, - } = props - - const ElementType = getElementType(ListItem, props) - const classes = cx( - useKeyOnly(active, 'active'), - useKeyOnly(disabled, 'disabled'), - useKeyOnly(ElementType !== 'li', 'item'), - className, - ) - const rest = getUnhandledProps(ListItem, props) - const valueProp = ElementType === 'li' ? { value } : { 'data-value': value } - - if (!_.isNil(children)) { - return {children} +class ListItem extends Component { + static propTypes = { + /** An element type to render as (string or function). */ + as: customPropTypes.as, + + /** A list item can active. */ + active: PropTypes.bool, + + /** Primary content. */ + children: PropTypes.node, + + /** Additional classes. */ + className: PropTypes.string, + + /** + * Shorthand for primary content. + * + * Heads up! + * + * This is handled slightly differently than the typical `content` prop since + * the wrapping ListContent is not used when there's no icon or image. + * + * If you pass content as: + * - an element/literal, it's treated as the sibling node to + * header/description (whether wrapped in Item.Content or not). + * - a props object, it forces the presence of Item.Content and passes those + * props to it. If you pass a content prop within that props object, it + * will be treated as the sibling node to header/description. + */ + content: customPropTypes.itemShorthand, + + /** Shorthand for ListDescription. */ + description: customPropTypes.itemShorthand, + + /** A list item can disabled. */ + disabled: PropTypes.bool, + + /** Shorthand for ListHeader. */ + header: customPropTypes.itemShorthand, + + /** Shorthand for ListIcon. */ + icon: customPropTypes.every([ + customPropTypes.disallow(['image']), + customPropTypes.itemShorthand, + ]), + + /** Shorthand for Image. */ + image: customPropTypes.every([ + customPropTypes.disallow(['icon']), + customPropTypes.itemShorthand, + ]), + + /** A ListItem can be clicked */ + onClick: PropTypes.func, + + /** A value for an ordered list. */ + value: PropTypes.string, } - const iconElement = ListIcon.create(icon) - const imageElement = Image.create(image) + static _meta = { + name: 'ListItem', + parent: 'List', + type: META.TYPES.ELEMENT, + } - // See description of `content` prop for explanation about why this is necessary. - if (!isValidElement(content) && _.isPlainObject(content)) { - return ( - - {iconElement || imageElement} - {ListContent.create(content, { header, description })} - - ) + handleClick = (e) => { + const { onClick } = this.props + + if (onClick) onClick(e, this.props) } - const headerElement = ListHeader.create(header) - const descriptionElement = ListDescription.create(description) + render() { + const { + active, + children, + className, + content, + description, + disabled, + header, + icon, + image, + value, + } = this.props + + const ElementType = getElementType(ListItem, this.props) + const classes = cx( + useKeyOnly(active, 'active'), + useKeyOnly(disabled, 'disabled'), + useKeyOnly(ElementType !== 'li', 'item'), + className, + ) + const rest = getUnhandledProps(ListItem, this.props) + const valueProp = ElementType === 'li' ? { value } : { 'data-value': value } + + if (!_.isNil(children)) { + return ( + + {children} + + ) + } + + const iconElement = ListIcon.create(icon) + const imageElement = Image.create(image) + + // See description of `content` prop for explanation about why this is necessary. + if (!isValidElement(content) && _.isPlainObject(content)) { + return ( + + {iconElement || imageElement} + {ListContent.create(content, { header, description })} + + ) + } + + const headerElement = ListHeader.create(header) + const descriptionElement = ListDescription.create(description) + if (iconElement || imageElement) { + return ( + + {iconElement || imageElement} + {(content || headerElement || descriptionElement) && ( + + {headerElement} + {descriptionElement} + {content} + + )} + + ) + } - if (iconElement || imageElement) { return ( - - {iconElement || imageElement} - {(content || headerElement || descriptionElement) && ( - - {headerElement} - {descriptionElement} - {content} - - )} + + {headerElement} + {descriptionElement} + {content} ) } - - return ( - - {headerElement} - {descriptionElement} - {content} - - ) -} - -ListItem._meta = { - name: 'ListItem', - parent: 'List', - type: META.TYPES.ELEMENT, -} - -ListItem.propTypes = { - /** An element type to render as (string or function). */ - as: customPropTypes.as, - - /** A list item can active. */ - active: PropTypes.bool, - - /** Primary content. */ - children: PropTypes.node, - - /** Additional classes. */ - className: PropTypes.string, - - /** - * Shorthand for primary content. - * - * Heads up! - * - * This is handled slightly differently than the typical `content` prop since - * the wrapping ListContent is not used when there's no icon or image. - * - * If you pass content as: - * - an element/literal, it's treated as the sibling node to - * header/description (whether wrapped in Item.Content or not). - * - a props object, it forces the presence of Item.Content and passes those - * props to it. If you pass a content prop within that props object, it - * will be treated as the sibling node to header/description. - */ - content: customPropTypes.itemShorthand, - - /** Shorthand for ListDescription. */ - description: customPropTypes.itemShorthand, - - /** A list item can disabled. */ - disabled: PropTypes.bool, - - /** Shorthand for ListHeader. */ - header: customPropTypes.itemShorthand, - - /** Shorthand for ListIcon. */ - icon: customPropTypes.every([ - customPropTypes.disallow(['image']), - customPropTypes.itemShorthand, - ]), - - /** Shorthand for Image. */ - image: customPropTypes.every([ - customPropTypes.disallow(['icon']), - customPropTypes.itemShorthand, - ]), - - /** A value for an ordered list. */ - value: PropTypes.string, } ListItem.create = createShorthandFactory(ListItem, content => ({ content })) diff --git a/test/specs/elements/List/List-test.js b/test/specs/elements/List/List-test.js index 07f5772abe..5d3d6980bf 100644 --- a/test/specs/elements/List/List-test.js +++ b/test/specs/elements/List/List-test.js @@ -9,6 +9,7 @@ import ListItem from 'src/elements/List/ListItem' import ListList from 'src/elements/List/ListList' import { SUI } from 'src/lib' import * as common from 'test/specs/commonTests' +import { sandbox } from 'test/utils' describe('List', () => { common.isConformant(List) @@ -34,9 +35,37 @@ describe('List', () => { common.propValueOnlyToClassName(List, 'size', SUI.SIZES) - describe('role', () => { - const items = ['Name', 'Status', 'Notes'] + const items = ['Name', 'Status', 'Notes'] + + describe('onItemClick', () => { + it('can be omitted', () => { + const click = () => shallow().simulate('click') + expect(click).to.not.throw() + }) + + it('is called with (e, itemProps) when clicked', () => { + const onClick = sandbox.spy() + const onItemClick = sandbox.spy() + const event = { target: null } + + const callbackData = { content: 'Notes', 'data-foo': 'bar' } + const itemProps = { key: 'notes', content: 'Notes', 'data-foo': 'bar', onClick } + + shallow() + .find('ListItem') + .first() + .shallow() + .simulate('click', event) + onClick.should.have.been.calledOnce() + onClick.should.have.been.calledWithMatch(event, callbackData) + + onItemClick.should.have.been.calledOnce() + onItemClick.should.have.been.calledWithMatch(event, callbackData) + }) + }) + + describe('role', () => { it('is accessibile with no items', () => { const wrapper = shallow() @@ -51,8 +80,6 @@ describe('List', () => { }) describe('shorthand', () => { - const items = ['Name', 'Status', 'Notes'] - it('renders empty tr with no shorthand', () => { const wrapper = shallow() diff --git a/test/specs/elements/List/ListContent-test.js b/test/specs/elements/List/ListContent-test.js index 5f1b39f8f9..d4ea3b03be 100644 --- a/test/specs/elements/List/ListContent-test.js +++ b/test/specs/elements/List/ListContent-test.js @@ -9,6 +9,8 @@ describe('ListContent', () => { common.isConformant(ListContent) common.rendersChildren(ListContent) + common.implementsCreateMethod(ListContent) + common.implementsVerticalAlignProp(ListContent) common.propKeyAndValueToClassName(ListContent, 'floated', SUI.FLOATS) diff --git a/test/specs/elements/List/ListDescription-test.js b/test/specs/elements/List/ListDescription-test.js index 9482219915..b16f79392d 100644 --- a/test/specs/elements/List/ListDescription-test.js +++ b/test/specs/elements/List/ListDescription-test.js @@ -4,4 +4,6 @@ import * as common from 'test/specs/commonTests' describe('ListDescription', () => { common.isConformant(ListDescription) common.rendersChildren(ListDescription) + + common.implementsCreateMethod(ListDescription) }) diff --git a/test/specs/elements/List/ListHeader-test.js b/test/specs/elements/List/ListHeader-test.js index f44973962d..49dd3ccd53 100644 --- a/test/specs/elements/List/ListHeader-test.js +++ b/test/specs/elements/List/ListHeader-test.js @@ -4,4 +4,6 @@ import * as common from 'test/specs/commonTests' describe('ListHeader', () => { common.isConformant(ListHeader) common.rendersChildren(ListHeader) + + common.implementsCreateMethod(ListHeader) }) diff --git a/test/specs/elements/List/ListItem-test.js b/test/specs/elements/List/ListItem-test.js index 65cd331cac..7cba839991 100644 --- a/test/specs/elements/List/ListItem-test.js +++ b/test/specs/elements/List/ListItem-test.js @@ -14,13 +14,32 @@ describe('ListItem', () => { common.propKeyOnlyToClassName(ListItem, 'active') common.propKeyOnlyToClassName(ListItem, 'disabled') - describe('list', () => { - it('omitted when rendered as `li`', () => { + describe('as', () => { + it('omits className `list` when rendered as `li`', () => { shallow() .should.not.have.className('item') }) }) + describe('onClick', () => { + it('can be omitted', () => { + const click = () => shallow().simulate('click') + expect(click).to.not.throw() + }) + + it('is called with (e, props) when clicked', () => { + const onClick = sandbox.spy() + const event = { target: null } + const props = { onClick, 'data-foo': 'bar' } + + shallow() + .simulate('click', event) + + onClick.should.have.been.calledOnce() + onClick.should.have.been.calledWithExactly(event, props) + }) + }) + describe('value', () => { it('adds data attribute by default', () => { const value = faker.hacker.phrase()