From e92f0a55aaf12167e1d0cb4b87768cb879967bb8 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Thu, 24 Jan 2019 13:05:57 -0800 Subject: [PATCH 01/11] Saving state --- src/Breadcrumb/Breadcrumb.Component.js | 33 ++++++--- src/Breadcrumb/Breadcrumb.js | 27 +++++--- src/Breadcrumb/Breadcrumb.test.js | 12 ++-- src/Menu/Menu.Component.js | 51 +++++++++----- src/Menu/Menu.js | 44 +++++++----- src/Menu/Menu.test.js | 94 +++++++++++++++++--------- src/Shellbar/Shellbar.test.js | 1 + 7 files changed, 173 insertions(+), 89 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.Component.js b/src/Breadcrumb/Breadcrumb.Component.js index 006d4c58c..ebb8cb304 100644 --- a/src/Breadcrumb/Breadcrumb.Component.js +++ b/src/Breadcrumb/Breadcrumb.Component.js @@ -1,18 +1,25 @@ +import { Link } from 'react-router-dom'; import React from 'react'; import { Breadcrumb, BreadcrumbItem } from '../'; import { Description, DocsText, DocsTile, Header, Import, Separator } from '../_playground'; export const BreadcrumbComponent = () => { const breadcrumbHrefCode = ` - - - + + + `; - const breadcrumbLinkCode = ` - - - + const breadcrumbLinkCode = ` + + Link Text + + + Link Text + + + Link Text + `; return ( @@ -42,9 +49,15 @@ export const BreadcrumbComponent = () => { An example using link (routerLink) - - - + + Link Text + + + Link Text + + + Link Text + {breadcrumbLinkCode} diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index d1e083b4d..145fb0733 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -1,6 +1,5 @@ import PropTypes from 'prop-types'; import React from 'react'; -import { BrowserRouter, Link } from 'react-router-dom'; export const Breadcrumb = ({ children, ...props }) => { return
    {children}
; @@ -10,19 +9,29 @@ Breadcrumb.propTypes = { children: PropTypes.node }; -export const BreadcrumbItem = ({ url, link, name, className, ...props }) => { +export const BreadcrumbItem = ({ url, link, name, className, children, ...props }) => { + const renderLink = () => { + if (!children && url) { + console.warn('It is suggested to use an anchor link or other link to provide a uniform API'); //TODO: We block these via lint and need a warning util + return ( + {name} + ); + } else if (children) { + return React.cloneElement(children, { + 'className': 'fd-breadcrumb__link' + }); + } + }; + return ( - -
  • - {link && {name}} - {url && {name}} -
    • -     +
  • + {renderLink()} +
    • ); }; BreadcrumbItem.propTypes = { - name: PropTypes.string.isRequired, link: PropTypes.string, + name: PropTypes.string, url: PropTypes.string }; diff --git a/src/Breadcrumb/Breadcrumb.test.js b/src/Breadcrumb/Breadcrumb.test.js index 9cf4ca606..ce40c7efe 100644 --- a/src/Breadcrumb/Breadcrumb.test.js +++ b/src/Breadcrumb/Breadcrumb.test.js @@ -1,4 +1,4 @@ -import { MemoryRouter } from 'react-router-dom'; +import { Link, MemoryRouter } from 'react-router-dom'; import { mount } from 'enzyme'; import React from 'react'; import renderer from 'react-test-renderer'; @@ -16,9 +16,13 @@ describe('', () => { const breadCrumbRouterLink = ( - - - + + + Link Text + + + Link Text + ); diff --git a/src/Menu/Menu.Component.js b/src/Menu/Menu.Component.js index 33a52587c..ffba3ed7a 100644 --- a/src/Menu/Menu.Component.js +++ b/src/Menu/Menu.Component.js @@ -1,3 +1,4 @@ +import { Link } from 'react-router-dom'; import React from 'react'; import { Description, DocsText, DocsTile, Header, Import, Properties, Separator } from '../_playground'; import { Menu, MenuGroup, MenuItem, MenuList } from '../'; @@ -131,15 +132,27 @@ export const MenuComponent = () => { - Option 1 - Option 2 - Option 3 + + Option 1 + + + Option 2 + + + Option 3 + - Option 4 - Option 5 - Option 6 + + Option 1 + + + Option 2 + + + Option 3 + @@ -152,16 +165,15 @@ export const MenuComponent = () => { - - Option 1 + + Option 1 - - Option 2 + + Option 2 - - Option 3 + + Option 3 - Option 4 @@ -173,12 +185,15 @@ export const MenuComponent = () => { - Option 1 - - Option 2 + + Option 1 + + + Option 2 + + + Option 3 - Option 3 - Option 4 diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index 6fb72dae6..885ff02b4 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -1,4 +1,3 @@ -import { Link } from 'react-router-dom'; import PropTypes from 'prop-types'; import React from 'react'; @@ -22,27 +21,39 @@ export const MenuList = ({ children, className, ...props }) => { }; // ---------------------------------------- Menu Item ---------------------------------------- -export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, linkProps, urlProps, ...props }) => { +export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, urlProps, ...props }) => { + const renderLink = () => { + const isString = React.Children.map(children, (child) => { + if (typeof child === 'string') { + return true; + } + }); + + if (url) { + return ( + + {children} + + ); + } else if (children && !isString) { + return React.cloneElement(children, { + 'className': `fd-menu__item${isLink ? ' fd-menu__link' : ''}`, + ...urlProps + }); + } else if(children) { + return children; + } + }; + return (
  • {addon &&
    {}
    } - {link && - - {children} - - } - {url && - - {children} - - } - {(!url && !link) && {children}} + {renderLink()}
    • {separator &&
    }     @@ -54,7 +65,6 @@ MenuItem.propTypes = { addonProps: PropTypes.object, className: PropTypes.string, isLink: PropTypes.bool, - linkProps: PropTypes.object, separator: PropTypes.bool, url: PropTypes.string, urlProps: PropTypes.object diff --git a/src/Menu/Menu.test.js b/src/Menu/Menu.test.js index 0930b7ae2..b3fdb7233 100644 --- a/src/Menu/Menu.test.js +++ b/src/Menu/Menu.test.js @@ -1,7 +1,7 @@ -import { MemoryRouter } from 'react-router-dom'; import { mount } from 'enzyme'; import React from 'react'; import renderer from 'react-test-renderer'; +import { Link, MemoryRouter } from 'react-router-dom'; import { Menu, MenuGroup, MenuItem, MenuList } from './Menu'; describe('', () => { @@ -23,25 +23,42 @@ describe('', () => { - Option 1 - - Option 2 + + Option 1 + + + + Option 2 + + + + Option 3 - Option 3 - Option 4 - Option 5 - Option 6 + + Option 4 + + + Option 5 + + + Option 6 + - Option 7 - Option 8 - Option 9 + + Option 7 + + + Option 8 + + + Option 9 + @@ -52,16 +69,18 @@ describe('', () => { - - Option 1 + + Option 1 + + + Option 2 - - Option 2 + + Option 3 - - Option 3 + + Option 4 - Option 4 @@ -71,12 +90,18 @@ describe('', () => { - Option 1 - - Option 2 + + Option 1 + + + Option 2 + + + Option 3 + + + Option 4 - Option 3 - Option 4 @@ -136,13 +161,18 @@ describe('', () => { - Option 1 - - Option 2 + + Option 1 + + + Option 2 + + + Option 3 + + + Option 4 - Option 3 - Option 4 ); @@ -157,7 +187,9 @@ describe('', () => { - + + + ); diff --git a/src/Shellbar/Shellbar.test.js b/src/Shellbar/Shellbar.test.js index b6cba4403..8e6dc46aa 100644 --- a/src/Shellbar/Shellbar.test.js +++ b/src/Shellbar/Shellbar.test.js @@ -1,3 +1,4 @@ +import { Link } from 'react-router-dom'; import { mount } from 'enzyme'; import React from 'react'; import renderer from 'react-test-renderer'; From b0a9b5af8805f6bae7828d448ff170883a9c4645 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Thu, 24 Jan 2019 22:03:09 -0800 Subject: [PATCH 02/11] Updating menu to factor out react-router --- src/Menu/Menu.js | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index 28f2efc43..d890bafcc 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -35,7 +35,7 @@ export const MenuList = ({ children, className, ...props }) => { }; // ---------------------------------------- Menu Item ---------------------------------------- -export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, linkProps, urlProps, ...props }) => { +export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, urlProps, ...props }) => { const menuItemLinkClasses = classnames( 'fd-menu__item', { @@ -44,27 +44,21 @@ export const MenuItem = ({ url, link, isLink, separator, addon, children, onclic ); const renderLink = () => { - const isString = React.Children.map(children, (child) => { - if (typeof child === 'string') { - return true; - } - }); - - if (url) { + const isString = typeof children === 'string'; + if(url || onclick || isString) { return ( + className={menuItemLinkClasses} + href={url} + onClick={onclick}> {children} ); - } else if (children && !isString) { + } else if (children) { return React.cloneElement(children, { - 'className': `fd-menu__item${isLink ? ' fd-menu__link' : ''}`, + 'className': menuItemLinkClasses, ...urlProps }); - } else if(children) { - return children; } }; From 10d15f206f06f48934db40178ef5809420a31505 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Thu, 24 Jan 2019 22:14:13 -0800 Subject: [PATCH 03/11] Update linting --- src/Breadcrumb/Breadcrumb.js | 1 - src/Breadcrumb/Breadcrumb.test.js | 2 +- src/Menu/Menu.js | 6 ++++-- src/Shellbar/Shellbar.test.js | 1 - 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index b5c29ead2..72e13b451 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -13,7 +13,6 @@ Breadcrumb.propTypes = { export const BreadcrumbItem = ({ url, link, name, className, children, ...props }) => { const renderLink = () => { if (!children && url) { - console.warn('It is suggested to use an anchor link or other link to provide a uniform API'); //TODO: We block these via lint and need a warning util return ( {name} ); diff --git a/src/Breadcrumb/Breadcrumb.test.js b/src/Breadcrumb/Breadcrumb.test.js index ce40c7efe..d0517dea4 100644 --- a/src/Breadcrumb/Breadcrumb.test.js +++ b/src/Breadcrumb/Breadcrumb.test.js @@ -1,8 +1,8 @@ -import { Link, MemoryRouter } from 'react-router-dom'; import { mount } from 'enzyme'; import React from 'react'; import renderer from 'react-test-renderer'; import { Breadcrumb, BreadcrumbItem } from './Breadcrumb'; +import { Link, MemoryRouter } from 'react-router-dom'; describe('', () => { const defaultBreadCrumb = ( diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index d890bafcc..943e70af7 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -45,7 +45,7 @@ export const MenuItem = ({ url, link, isLink, separator, addon, children, onclic const renderLink = () => { const isString = typeof children === 'string'; - if(url || onclick || isString) { + if (url || onclick || isString) { return (
  • diff --git a/src/Shellbar/Shellbar.test.js b/src/Shellbar/Shellbar.test.js index 8e6dc46aa..b6cba4403 100644 --- a/src/Shellbar/Shellbar.test.js +++ b/src/Shellbar/Shellbar.test.js @@ -1,4 +1,3 @@ -import { Link } from 'react-router-dom'; import { mount } from 'enzyme'; import React from 'react'; import renderer from 'react-test-renderer'; From 1d3c2619cf5f3688a68ea535bc060cb8e81d2a4b Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Thu, 24 Jan 2019 22:28:07 -0800 Subject: [PATCH 04/11] Reverting breadcrumb for easier Menu PR --- src/Breadcrumb/Breadcrumb.Component.js | 33 ++++++++------------------ src/Breadcrumb/Breadcrumb.js | 26 +++++++------------- src/Breadcrumb/Breadcrumb.test.js | 12 ++++------ 3 files changed, 23 insertions(+), 48 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.Component.js b/src/Breadcrumb/Breadcrumb.Component.js index 2cc839896..2d1e46829 100644 --- a/src/Breadcrumb/Breadcrumb.Component.js +++ b/src/Breadcrumb/Breadcrumb.Component.js @@ -1,25 +1,18 @@ -import { Link } from 'react-router-dom'; import React from 'react'; import { Breadcrumb, BreadcrumbItem } from '../'; import { Description, DocsText, DocsTile, Header, Import, Separator } from '../_playground'; export const BreadcrumbComponent = () => { const breadcrumbHrefCode = ` - - - + + + `; - const breadcrumbLinkCode = ` - - Link Text - - - Link Text - - - Link Text - + const breadcrumbLinkCode = ` + + + `; return ( @@ -49,15 +42,9 @@ export const BreadcrumbComponent = () => { An example using link (routerLink) - - Link Text - - - Link Text - - - Link Text - + + + {breadcrumbLinkCode} diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index 72e13b451..924d608e8 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -1,6 +1,7 @@ import classnames from 'classnames'; import PropTypes from 'prop-types'; import React from 'react'; +import { BrowserRouter, Link } from 'react-router-dom'; export const Breadcrumb = ({ children, ...props }) => { return
      {children}
    ; @@ -10,33 +11,24 @@ Breadcrumb.propTypes = { children: PropTypes.node }; -export const BreadcrumbItem = ({ url, link, name, className, children, ...props }) => { - const renderLink = () => { - if (!children && url) { - return ( -     {name} - ); - } else if (children) { - return React.cloneElement(children, { - 'className': 'fd-breadcrumb__link' - }); - } - }; - +export const BreadcrumbItem = ({ url, link, name, className, ...props }) => { const breadcrumbItemClasses = classnames( 'fd-breadcrumb__item', className ); return ( -
  • - {renderLink()} -
    • + +
  • + {link && {name}} + {url && {name}} +
    • +     ); }; BreadcrumbItem.propTypes = { + name: PropTypes.string.isRequired, link: PropTypes.string, - name: PropTypes.string, url: PropTypes.string }; diff --git a/src/Breadcrumb/Breadcrumb.test.js b/src/Breadcrumb/Breadcrumb.test.js index d0517dea4..9cf4ca606 100644 --- a/src/Breadcrumb/Breadcrumb.test.js +++ b/src/Breadcrumb/Breadcrumb.test.js @@ -1,8 +1,8 @@ +import { MemoryRouter } from 'react-router-dom'; import { mount } from 'enzyme'; import React from 'react'; import renderer from 'react-test-renderer'; import { Breadcrumb, BreadcrumbItem } from './Breadcrumb'; -import { Link, MemoryRouter } from 'react-router-dom'; describe('', () => { const defaultBreadCrumb = ( @@ -16,13 +16,9 @@ describe('', () => { const breadCrumbRouterLink = ( - - - Link Text - - - Link Text - + + + ); From d6efe1b10ef091614659fac1cc2800cdcf93c42b Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Fri, 25 Jan 2019 07:58:30 -0800 Subject: [PATCH 05/11] Updating Menu Component Documentation --- src/Menu/Menu.Component.js | 74 +++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/src/Menu/Menu.Component.js b/src/Menu/Menu.Component.js index fd46069a1..8ee6eeacc 100644 --- a/src/Menu/Menu.Component.js +++ b/src/Menu/Menu.Component.js @@ -15,40 +15,62 @@ export const MenuComponent = () => { const menuGroupCode = ` - Option 1 - Option 2 - Option 3 + + Option 1 + + + Option 2 + + + Option 3 + - Option 4 - Option 5 - Option 6 + + Option 4 + + + Option 5 + + + Option 6 + `; const menuSeparatorCode = ` - - Option 1 + + Option 1 + + + Option 2 - - Option 2 + + Option 3 - - Option 3 + + Option 4 - Option 4 `; const menuAddonBeforeCode = ` - Option 1 - Option 2 - Option 3 - Option 4 + + Option 1 + + + Option 2 + + + Option 3 + + + Option 4 + `; @@ -83,11 +105,7 @@ export const MenuComponent = () => { name: 'addonProps', description: 'object - additional props to be spread to the addon section' }, - { name: 'link', description: 'string - a router link. Use either \'url\' or \'link\'' }, - { - name: 'linkProps', - description: 'object - additional props to be spread to the Menu Item links' - }, + { name: 'children', description: 'component - can be used to pass React Router or any other component which emits an .' }, { name: 'separator', description: 'bool - when set to true, adds a horizontal line (separator).' }, { name: 'url', description: 'string - href attribute of tag. Use either \'url\' or \'link\'' }, { @@ -145,13 +163,13 @@ export const MenuComponent = () => { - Option 1 + Option 4 - Option 2 + Option 5 - Option 3 + Option 6 @@ -174,6 +192,9 @@ export const MenuComponent = () => { Option 3 + + Option 4 +     @@ -194,6 +215,9 @@ export const MenuComponent = () => { Option 3 + + Option 4 +     From 1856fcc463e5128816964aad8c2a9d13a32a8227 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Fri, 25 Jan 2019 12:10:14 -0800 Subject: [PATCH 06/11] Remove unneeded MemoryRouter component from test --- src/Menu/Menu.test.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Menu/Menu.test.js b/src/Menu/Menu.test.js index b3fdb7233..f9424cf08 100644 --- a/src/Menu/Menu.test.js +++ b/src/Menu/Menu.test.js @@ -201,13 +201,11 @@ describe('', () => { test('should allow props to be spread to the MenuItem component\'s a element', () => { const element = mount( - - - - - - - + + + + + ); expect( From 1461e9de4c2a2e25f6227513dee411ae2c21dd01 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 10:11:21 -0800 Subject: [PATCH 07/11] Removing stray prop --- src/Menu/Menu.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index e0521f91a..809a1cce3 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -44,7 +44,7 @@ MenuList.propTypes = { }; // ---------------------------------------- Menu Item ---------------------------------------- -export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, urlProps, ...props }) => { +export const MenuItem = ({ url, isLink, separator, addon, children, onclick, className, addonProps, urlProps, ...props }) => { const menuItemLinkClasses = classnames( 'fd-menu__item', { From 8f39c9e711c079ad18dd9ba5f6a9f4b3a59e8726 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 10:48:24 -0800 Subject: [PATCH 08/11] Second commit From bf5ff44e5730efe0121ba5828291b380cec99a35 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 10:51:25 -0800 Subject: [PATCH 09/11] Second commit From aae916c3cdd50003f3719556e2fcc2413345b73c Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 17:47:00 -0800 Subject: [PATCH 10/11] Updating Menu PR to spread Classnames --- src/Menu/Menu.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index 809a1cce3..648d2ea47 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -53,8 +53,7 @@ export const MenuItem = ({ url, isLink, separator, addon, children, onclick, cla ); const renderLink = () => { - const isString = typeof children === 'string'; - if (url || onclick || isString) { + if (url) { return ( ); - } else if (children) { + } else if (children && React.isValidElement(children)) { + const childrenClassnames = classnames( + menuItemLinkClasses, + children.props.className + ); + return React.cloneElement(children, { - 'className': menuItemLinkClasses, + 'className': childrenClassnames, ...urlProps }); } else if (children) { - return children; + return ({children}); } }; From 9daed1069c164736e2d1e834acd0d7e87b2b3a31 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 18:09:32 -0800 Subject: [PATCH 11/11] Updating description --- src/Breadcrumb/Breadcrumb.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index 61144fdfe..99c1e3da1 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -38,7 +38,6 @@ BreadcrumbItem.propTypes = { }; BreadcrumbItem.propDescriptions = { - name: 'Localized display text of the link (for either `link` or `url`).', - link: 'Enables use of react-router `Link` component. Path name to be applied to Link\'s `to` prop. Should use either `link` or `url`, but not both.', - url: 'Enables use of `` element. Value to be applied to the anchor\'s `href` attribute. Should use either `link` or `url`, but not both.' + name: 'Text for the internal anchor tag.', + url: 'An anchor tag will be generated and set to the url prop. Name or child text must be provided.' };