From b6330e2441fbbf095613553f045af430bcbc8b72 Mon Sep 17 00:00:00 2001 From: Alexander Fedyashov Date: Mon, 13 Feb 2017 22:09:05 +0200 Subject: [PATCH] style(Popup): fix tests, update typings and propTypes usage --- src/index.d.ts | 1 - src/modules/Popup/Popup.js | 76 +++++++++---------- src/modules/Popup/PopupContent.js | 11 ++- src/modules/Popup/PopupHeader.js | 15 ++-- src/modules/Popup/index.d.ts | 55 +++++++++----- test/specs/modules/Popup/Popup-test.js | 32 ++++---- test/specs/modules/Popup/PopupContent-test.js | 2 + test/specs/modules/Popup/PopupHeader-test.js | 2 + 8 files changed, 107 insertions(+), 87 deletions(-) diff --git a/src/index.d.ts b/src/index.d.ts index b46b739648..28a551e460 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -11,7 +11,6 @@ export type SemanticSIZES = 'mini' | 'tiny' | 'small' | 'medium' | 'large' | 'bi export type SemanticTEXTALIGNMENTS = 'left' | 'center' | 'right' | 'justified'; export type SemanticVERTICALALIGNMENTS = 'top' | 'middle' | 'bottom'; export type SemanticCOUNTRY = 'ad' | 'andorra' | 'ae' | 'united arab emirates' | 'uae' | 'af' | 'afghanistan' | 'ag' | 'antigua' | 'ai' | 'anguilla' | 'al' | 'albania' | 'am' | 'armenia' | 'an' | 'netherlands antilles' | 'ao' | 'angola' | 'ar' | 'argentina' | 'as' | 'american samoa' | 'at' | 'austria' | 'au' | 'australia' | 'aw' | 'aruba' | 'ax' | 'aland islands' | 'az' | 'azerbaijan' | 'ba' | 'bosnia' | 'bb' | 'barbados' | 'bd' | 'bangladesh' | 'be' | 'belgium' | 'bf' | 'burkina faso' | 'bg' | 'bulgaria' | 'bh' | 'bahrain' | 'bi' | 'burundi' | 'bj' | 'benin' | 'bm' | 'bermuda' | 'bn' | 'brunei' | 'bo' | 'bolivia' | 'br' | 'brazil' | 'bs' | 'bahamas' | 'bt' | 'bhutan' | 'bv' | 'bouvet island' | 'bw' | 'botswana' | 'by' | 'belarus' | 'bz' | 'belize' | 'ca' | 'canada' | 'cc' | 'cocos islands' | 'cd' | 'congo' | 'cf' | 'central african republic' | 'cg' | 'congo brazzaville' | 'ch' | 'switzerland' | 'ci' | 'cote divoire' | 'ck' | 'cook islands' | 'cl' | 'chile' | 'cm' | 'cameroon' | 'cn' | 'china' | 'co' | 'colombia' | 'cr' | 'costa rica' | 'cs' | 'serbia' | 'cu' | 'cuba' | 'cv' | 'cape verde' | 'cx' | 'christmas island' | 'cy' | 'cyprus' | 'cz' | 'czech republic' | 'de' | 'germany' | 'dj' | 'djibouti' | 'dk' | 'denmark' | 'dm' | 'dominica' | 'do' | 'dominican republic' | 'dz' | 'algeria' | 'ec' | 'ecuador' | 'ee' | 'estonia' | 'eg' | 'egypt' | 'eh' | 'western sahara' | 'er' | 'eritrea' | 'es' | 'spain' | 'et' | 'ethiopia' | 'eu' | 'european union' | 'fi' | 'finland' | 'fj' | 'fiji' | 'fk' | 'falkland islands' | 'fm' | 'micronesia' | 'fo' | 'faroe islands' | 'fr' | 'france' | 'ga' | 'gabon' | 'gb' | 'united kingdom' | 'gd' | 'grenada' | 'ge' | 'georgia' | 'gf' | 'french guiana' | 'gh' | 'ghana' | 'gi' | 'gibraltar' | 'gl' | 'greenland' | 'gm' | 'gambia' | 'gn' | 'guinea' | 'gp' | 'guadeloupe' | 'gq' | 'equatorial guinea' | 'gr' | 'greece' | 'gs' | 'sandwich islands' | 'gt' | 'guatemala' | 'gu' | 'guam' | 'gw' | 'guinea-bissau' | 'gy' | 'guyana' | 'hk' | 'hong kong' | 'hm' | 'heard island' | 'hn' | 'honduras' | 'hr' | 'croatia' | 'ht' | 'haiti' | 'hu' | 'hungary' | 'id' | 'indonesia' | 'ie' | 'ireland' | 'il' | 'israel' | 'in' | 'india' | 'io' | 'indian ocean territory' | 'iq' | 'iraq' | 'ir' | 'iran' | 'is' | 'iceland' | 'it' | 'italy' | 'jm' | 'jamaica' | 'jo' | 'jordan' | 'jp' | 'japan' | 'ke' | 'kenya' | 'kg' | 'kyrgyzstan' | 'kh' | 'cambodia' | 'ki' | 'kiribati' | 'km' | 'comoros' | 'kn' | 'saint kitts and nevis' | 'kp' | 'north korea' | 'kr' | 'south korea' | 'kw' | 'kuwait' | 'ky' | 'cayman islands' | 'kz' | 'kazakhstan' | 'la' | 'laos' | 'lb' | 'lebanon' | 'lc' | 'saint lucia' | 'li' | 'liechtenstein' | 'lk' | 'sri lanka' | 'lr' | 'liberia' | 'ls' | 'lesotho' | 'lt' | 'lithuania' | 'lu' | 'luxembourg' | 'lv' | 'latvia' | 'ly' | 'libya' | 'ma' | 'morocco' | 'mc' | 'monaco' | 'md' | 'moldova' | 'me' | 'montenegro' | 'mg' | 'madagascar' | 'mh' | 'marshall islands' | 'mk' | 'macedonia' | 'ml' | 'mali' | 'mm' | 'myanmar' | 'burma' | 'mn' | 'mongolia' | 'mo' | 'macau' | 'mp' | 'northern mariana islands' | 'mq' | 'martinique' | 'mr' | 'mauritania' | 'ms' | 'montserrat' | 'mt' | 'malta' | 'mu' | 'mauritius' | 'mv' | 'maldives' | 'mw' | 'malawi' | 'mx' | 'mexico' | 'my' | 'malaysia' | 'mz' | 'mozambique' | 'na' | 'namibia' | 'nc' | 'new caledonia' | 'ne' | 'niger' | 'nf' | 'norfolk island' | 'ng' | 'nigeria' | 'ni' | 'nicaragua' | 'nl' | 'netherlands' | 'no' | 'norway' | 'np' | 'nepal' | 'nr' | 'nauru' | 'nu' | 'niue' | 'nz' | 'new zealand' | 'om' | 'oman' | 'pa' | 'panama' | 'pe' | 'peru' | 'pf' | 'french polynesia' | 'pg' | 'new guinea' | 'ph' | 'philippines' | 'pk' | 'pakistan' | 'pl' | 'poland' | 'pm' | 'saint pierre' | 'pn' | 'pitcairn islands' | 'pr' | 'puerto rico' | 'ps' | 'palestine' | 'pt' | 'portugal' | 'pw' | 'palau' | 'py' | 'paraguay' | 'qa' | 'qatar' | 're' | 'reunion' | 'ro' | 'romania' | 'rs' | 'serbia' | 'ru' | 'russia' | 'rw' | 'rwanda' | 'sa' | 'saudi arabia' | 'sb' | 'solomon islands' | 'sc' | 'seychelles' | 'gb sct' | 'scotland' | 'sd' | 'sudan' | 'se' | 'sweden' | 'sg' | 'singapore' | 'sh' | 'saint helena' | 'si' | 'slovenia' | 'sj' | 'svalbard' | 'jan mayen' | 'sk' | 'slovakia' | 'sl' | 'sierra leone' | 'sm' | 'san marino' | 'sn' | 'senegal' | 'so' | 'somalia' | 'sr' | 'suriname' | 'st' | 'sao tome' | 'sv' | 'el salvador' | 'sy' | 'syria' | 'sz' | 'swaziland' | 'tc' | 'caicos islands' | 'td' | 'chad' | 'tf' | 'french territories' | 'tg' | 'togo' | 'th' | 'thailand' | 'tj' | 'tajikistan' | 'tk' | 'tokelau' | 'tl' | 'timorleste' | 'tm' | 'turkmenistan' | 'tn' | 'tunisia' | 'to' | 'tonga' | 'tr' | 'turkey' | 'tt' | 'trinidad' | 'tv' | 'tuvalu' | 'tw' | 'taiwan' | 'tz' | 'tanzania' | 'ua' | 'ukraine' | 'ug' | 'uganda' | 'um' | 'us minor islands' | 'us' | 'america' | 'united states' | 'uy' | 'uruguay' | 'uz' | 'uzbekistan' | 'va' | 'vatican city' | 'vc' | 'saint vincent' | 've' | 'venezuela' | 'vg' | 'british virgin islands' | 'vi' | 'us virgin islands' | 'vn' | 'vietnam' | 'vu' | 'vanuatu' | 'gb wls' | 'wales' | 'wf' | 'wallis and futuna' | 'ws' | 'samoa' | 'ye' | 'yemen' | 'yt' | 'mayotte' | 'za' | 'south africa' | 'zm' | 'zambia' | 'zw' | 'zimbabwe'; -export type SemanticPOSITIONING = 'top left' | 'top right' | 'bottom right' | 'bottom left' | 'right center' | 'left center' | 'top center' | 'bottom center'; // ====================================================== // Widths diff --git a/src/modules/Popup/Popup.js b/src/modules/Popup/Popup.js index 294995dad5..9f63ac4163 100644 --- a/src/modules/Popup/Popup.js +++ b/src/modules/Popup/Popup.js @@ -1,7 +1,9 @@ -import React, { Component, PropTypes } from 'react' import cx from 'classnames' import _ from 'lodash' +import React, { Component, PropTypes } from 'react' + import { + customPropTypes, getElementType, getUnhandledProps, isBrowser, @@ -17,25 +19,16 @@ import PopupHeader from './PopupHeader' const debug = makeDebugger('popup') -const _meta = { - name: 'Popup', - type: META.TYPES.MODULE, - props: { - on: ['hover', 'click', 'focus'], - positioning: [ - 'top left', - 'top right', - 'bottom right', - 'bottom left', - 'right center', - 'left center', - 'top center', - 'bottom center', - ], - size: _.without(SUI.SIZES, 'medium', 'big', 'massive'), - wide: [true, false, 'very'], - }, -} +export const POSITIONS = [ + 'top left', + 'top right', + 'bottom right', + 'bottom left', + 'right center', + 'left center', + 'top center', + 'bottom center', +] /** * A Popup displays additional information on top of a page. @@ -45,14 +38,14 @@ export default class Popup extends Component { /** Display the popup without the pointing arrow. */ basic: PropTypes.bool, - /** You may pass a content as children of the Popup. */ + /** Primary content. */ children: PropTypes.node, - /** Classes to add to the Popup className. */ + /** Additional classes. */ className: PropTypes.string, - /** Simple text content for the popover */ - content: PropTypes.node, + /** Simple text content for the popover. */ + content: customPropTypes.itemShorthand, /** A flowing Popup has no maximum width and continues to flow to fit its content. */ flowing: PropTypes.bool, @@ -62,7 +55,10 @@ export default class Popup extends Component { // fluid: PropTypes.bool, /** Header displayed above the content in bold. */ - header: PropTypes.string, + header: customPropTypes.itemShorthand, + + /** Hide the Popup when scrolling the window. */ + hideOnScroll: PropTypes.bool, /** Whether the popup should not close on hover. */ hoverable: PropTypes.bool, @@ -70,14 +66,11 @@ export default class Popup extends Component { /** Invert the colors of the Popup. */ inverted: PropTypes.bool, - /** Hide the Popup when scrolling the window. */ - hideOnScroll: PropTypes.bool, - /** Horizontal offset in pixels to be applied to the Popup. */ offset: PropTypes.number, - /** Event triggering the popup */ - on: PropTypes.oneOf(_meta.props.on), + /** Event triggering the popup. */ + on: PropTypes.oneOf(['hover', 'click', 'focus']), /** * Called when a close event happens. @@ -111,11 +104,11 @@ export default class Popup extends Component { */ onUnmount: PropTypes.func, - /** Positioning for the popover */ - positioning: PropTypes.oneOf(_meta.props.positioning), + /** Positioning for the popover. */ + positioning: PropTypes.oneOf(POSITIONS), /** Popup size. */ - size: PropTypes.oneOf(_meta.props.size), + size: PropTypes.oneOf(_.without(SUI.SIZES, 'medium', 'big', 'massive')), /** Custom Popup style. */ style: PropTypes.object, @@ -124,7 +117,10 @@ export default class Popup extends Component { trigger: PropTypes.node, /** Popup width. */ - wide: PropTypes.oneOf(_meta.props.wide), + wide: PropTypes.oneOfType( + PropTypes.bool, + PropTypes.oneOf(['very']), + ), } static defaultProps = { @@ -132,7 +128,11 @@ export default class Popup extends Component { on: 'hover', } - static _meta = _meta + static _meta = { + name: 'Popup', + type: META.TYPES.MODULE, + } + static Content = PopupContent static Header = PopupHeader @@ -228,7 +228,7 @@ export default class Popup extends Component { // Lets detect if the popup is out of the viewport and adjust // the position accordingly - const positions = _.without(_meta.props.positioning, positioning) + const positions = _.without(POSITIONS, positioning) for (let i = 0; !this.isStyleInViewport(style) && i < positions.length; i++) { style = this.computePopupStyle(positions[i]) positioning = positions[i] @@ -349,8 +349,8 @@ export default class Popup extends Component { const popupJSX = ( {children} - {!children && PopupHeader.create(header)} - {!children && PopupContent.create(content)} + {_.isNil(children) && PopupHeader.create(header)} + {_.isNil(children) && PopupContent.create(content)} ) diff --git a/src/modules/Popup/PopupContent.js b/src/modules/Popup/PopupContent.js index 940cf7bfcb..a3bc76934a 100644 --- a/src/modules/Popup/PopupContent.js +++ b/src/modules/Popup/PopupContent.js @@ -1,7 +1,9 @@ -import React, { PropTypes } from 'react' import cx from 'classnames' +import React, { PropTypes } from 'react' + import { createShorthandFactory, + customPropTypes, getElementType, getUnhandledProps, META, @@ -19,9 +21,10 @@ export default function PopupContent(props) { return {children} } -PopupContent.create = createShorthandFactory(PopupContent, value => ({ children: value })) - PopupContent.propTypes = { + /** An element type to render as (string or function). */ + as: customPropTypes.as, + /** The content of the Popup */ children: PropTypes.node, @@ -34,3 +37,5 @@ PopupContent._meta = { type: META.TYPES.MODULE, parent: 'Popup', } + +PopupContent.create = createShorthandFactory(PopupContent, children => ({ children })) diff --git a/src/modules/Popup/PopupHeader.js b/src/modules/Popup/PopupHeader.js index 2a5662c477..5426a07155 100644 --- a/src/modules/Popup/PopupHeader.js +++ b/src/modules/Popup/PopupHeader.js @@ -1,7 +1,9 @@ -import React, { PropTypes } from 'react' import cx from 'classnames' +import React, { PropTypes } from 'react' + import { createShorthandFactory, + customPropTypes, getElementType, getUnhandledProps, META, @@ -19,13 +21,14 @@ export default function PopupHeader(props) { return {children} } -PopupHeader.create = createShorthandFactory(PopupHeader, value => ({ children: value })) - PopupHeader.propTypes = { - /** The header of the Popup */ + /** An element type to render as (string or function). */ + as: customPropTypes.as, + + /** Primary content. */ children: PropTypes.node, - /** Classes to add to the Popup header className. */ + /** Additional classes. */ className: PropTypes.string, } @@ -34,3 +37,5 @@ PopupHeader._meta = { type: META.TYPES.MODULE, parent: 'Popup', } + +PopupHeader.create = createShorthandFactory(PopupHeader, children => ({ children })) diff --git a/src/modules/Popup/index.d.ts b/src/modules/Popup/index.d.ts index ba9d9868b8..6d5089715d 100644 --- a/src/modules/Popup/index.d.ts +++ b/src/modules/Popup/index.d.ts @@ -1,9 +1,8 @@ -import { SemanticPOSITIONING, SemanticSIZES } from '../..'; +import * as React from 'react'; import { PortalProps } from '../../addons/Portal'; -export type PopupPropOn = 'hover' | 'click' | 'focus'; - interface PopupProps extends PortalProps { + [key: string]: any; /** Display the popup without the pointing arrow */ basic?: boolean; @@ -11,40 +10,46 @@ interface PopupProps extends PortalProps { /** Primary content. */ children?: React.ReactNode; - /** Shorthand for primary content. */ - content?: any; + /** Additional classes. */ + className?: string; + + /** Simple text content for the popover. */ + content?: React.ReactNode; - /** A Flowing popup have no maximum width and continue to flow to fit its content */ + /** A Flowing popup have no maximum width and continue to flow to fit its content. */ flowing?: boolean; - /** Header displayed above the content in bold */ - header?: string; + /** Header displayed above the content in bold. */ + header?: any; - /** The node where the popup should mount.. */ + /** The node where the popup should mount. */ hideOnScroll?: boolean; - /** Whether the popup should not close on hover */ + /** Whether the popup should not close on hover. */ hoverable?: boolean; /** Invert the colors of the popup */ inverted?: boolean; - /** Horizontal offset in pixels to be applied to the popup */ + /** Horizontal offset in pixels to be applied to the popup. */ offset?: number; - /** Event triggering the popup */ - on?: PopupPropOn; + /** Event triggering the popup. */ + on?: 'hover' | 'click' | 'focus'; - /** Positioning for the popover */ - positioning?: SemanticPOSITIONING; + /** Positioning for the popover. */ + positioning?: 'top left' | 'top right' | + 'bottom right' | 'bottom left' | + 'right center' | 'left center' | + 'top center' | 'bottom center'; - /** Popup size */ - size?: SemanticSIZES; + /** Popup size. */ + size?: 'mini' | 'tiny' | 'small' | 'large' | 'huge'; - /** custom popup style */ + /** Custom Popup style. */ style?: Object; - /** Popup width */ + /** Popup width. */ wide?: boolean | 'very'; } @@ -56,6 +61,10 @@ interface PopupClass extends React.ComponentClass { export const Popup: PopupClass; interface PopupContentProps { + [key: string]: any; + + /** An element type to render as (string or function). */ + as?: any; /** Primary content. */ children?: React.ReactNode; @@ -64,9 +73,13 @@ interface PopupContentProps { className?: string; } -export const PopupContent: React.ComponentClass; +export const PopupContent: React.StatelessComponent; interface PopupHeaderProps { + [key: string]: any; + + /** An element type to render as (string or function). */ + as?: any; /** Primary content. */ children?: React.ReactNode; @@ -75,4 +88,4 @@ interface PopupHeaderProps { className?: string; } -export const PopupHeader: React.ComponentClass; +export const PopupHeader: React.StatelessComponent; diff --git a/test/specs/modules/Popup/Popup-test.js b/test/specs/modules/Popup/Popup-test.js index 3d18d1c978..ebd84fb0f6 100644 --- a/test/specs/modules/Popup/Popup-test.js +++ b/test/specs/modules/Popup/Popup-test.js @@ -1,13 +1,13 @@ import _ from 'lodash' import React from 'react' -import Popup from 'src/modules/Popup/Popup' +import Portal from 'src/addons/Portal/Portal' +import { SUI } from 'src/lib' +import Popup, { POSITIONS } from 'src/modules/Popup/Popup' import PopupHeader from 'src/modules/Popup/PopupHeader' import PopupContent from 'src/modules/Popup/PopupContent' -import Portal from 'src/addons/Portal/Portal' - -import { domEvent, sandbox } from 'test/utils' import * as common from 'test/specs/commonTests' +import { domEvent, sandbox } from 'test/utils' // ---------------------------------------- // Wrapper @@ -107,23 +107,20 @@ describe('Popup', () => { }) describe('positioning', () => { - it('is always within the viewport', () => { - _.each(Popup._meta.props.positions, position => { + POSITIONS.forEach(position => { + it('is always within the viewport', () => { wrapperMount( foo} on='click' /> ) wrapper.find('button').simulate('click') - const { - top, - right, - bottom, - left, - } = document.querySelector('.popup.ui').getBoundingClientRect() + + const rect = document.querySelector('.popup.ui').getBoundingClientRect() + const { top, right, bottom, left } = rect expect(top).to.be.at.least(0) expect(left).to.be.at.least(0) @@ -265,13 +262,10 @@ describe('Popup', () => { }) describe('size', () => { - it('defines prop options in _meta', () => { - Popup._meta.props.should.have.any.keys('size') - Popup._meta.props.size.should.be.an('array') - }) + const sizes = _.without(SUI.SIZES, 'medium', 'big', 'massive') - it('adds the size to the popup className', () => { - Popup._meta.props.size.forEach(size => { + sizes.forEach(size => { + it(`adds the ${size} to the popup className`, () => { wrapperMount() assertInBody(`.ui.${size}.popup`) }) diff --git a/test/specs/modules/Popup/PopupContent-test.js b/test/specs/modules/Popup/PopupContent-test.js index 414e3688fe..01d8393650 100644 --- a/test/specs/modules/Popup/PopupContent-test.js +++ b/test/specs/modules/Popup/PopupContent-test.js @@ -4,4 +4,6 @@ import * as common from 'test/specs/commonTests' describe('PopupContent', () => { common.isConformant(PopupContent) common.rendersChildren(PopupContent) + + common.implementsCreateMethod(PopupContent) }) diff --git a/test/specs/modules/Popup/PopupHeader-test.js b/test/specs/modules/Popup/PopupHeader-test.js index d2307011c9..910c1e13ed 100644 --- a/test/specs/modules/Popup/PopupHeader-test.js +++ b/test/specs/modules/Popup/PopupHeader-test.js @@ -4,4 +4,6 @@ import * as common from 'test/specs/commonTests' describe('PopupHeader', () => { common.isConformant(PopupHeader) common.rendersChildren(PopupHeader) + + common.implementsCreateMethod(PopupHeader) })