From e92f0a55aaf12167e1d0cb4b87768cb879967bb8 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Thu, 24 Jan 2019 13:05:57 -0800 Subject: [PATCH 01/10] 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/10] 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/10] 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 e8ee12121c7c076cf30bf7a7adae6d78e86e7ae8 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Thu, 24 Jan 2019 22:25:59 -0800 Subject: [PATCH 04/10] Reverting menu for easier PR --- src/Menu/Menu.Component.js | 51 ++++++++------------- src/Menu/Menu.js | 42 ++++++++--------- src/Menu/Menu.test.js | 94 +++++++++++++------------------------- 3 files changed, 67 insertions(+), 120 deletions(-) diff --git a/src/Menu/Menu.Component.js b/src/Menu/Menu.Component.js index fd46069a1..04a810c0f 100644 --- a/src/Menu/Menu.Component.js +++ b/src/Menu/Menu.Component.js @@ -1,4 +1,3 @@ -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 '../'; @@ -132,27 +131,15 @@ export const MenuComponent = () => { - - Option 1 - - - Option 2 - - - Option 3 - + Option 1 + Option 2 + Option 3 - - Option 1 - - - Option 2 - - - Option 3 - + Option 4 + Option 5 + Option 6 @@ -165,15 +152,16 @@ export const MenuComponent = () => { - - Option 1 + + Option 1 - - Option 2 + + Option 2 - - Option 3 + + Option 3 + Option 4 @@ -185,15 +173,12 @@ export const MenuComponent = () => { - - Option 1 - - - Option 2 - - - Option 3 + Option 1 + + Option 2 + Option 3 + Option 4 diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index 943e70af7..680d4b7ae 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -1,4 +1,5 @@ import classnames from 'classnames'; +import { Link } from 'react-router-dom'; import PropTypes from 'prop-types'; import React from 'react'; @@ -35,42 +36,34 @@ export const MenuList = ({ children, className, ...props }) => { }; // ---------------------------------------- Menu Item ---------------------------------------- -export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, urlProps, ...props }) => { +export const MenuItem = ({ url, link, isLink, separator, addon, children, onclick, className, addonProps, linkProps, urlProps, ...props }) => { const menuItemLinkClasses = classnames( 'fd-menu__item', { 'fd-menu__link': isLink } ); - - const renderLink = () => { - const isString = typeof children === 'string'; - if (url || onclick || isString) { - return ( - - {children} - - ); - } else if (children) { - return React.cloneElement(children, { - 'className': menuItemLinkClasses, - ...urlProps - }); - } else if (children) { - return children; - } - }; - return (
  • {addon &&
    {}
    } - {renderLink()} + {link && + + {children} + + } + {url && + + {children} + + } + {(!url && !link) && {children}}
    • {separator &&
    } @@ -82,6 +75,7 @@ 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 b3fdb7233..0930b7ae2 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,42 +23,25 @@ describe('', () => { - - Option 1 - - - - Option 2 - - - - Option 3 + Option 1 + + Option 2 + 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 @@ -69,18 +52,16 @@ describe('', () => { - - Option 1 - - - Option 2 + + Option 1 - - Option 3 + + Option 2 - - Option 4 + + Option 3 + Option 4 @@ -90,18 +71,12 @@ describe('', () => { - - Option 1 - - - Option 2 - - - Option 3 - - - Option 4 + Option 1 + + Option 2 + Option 3 + Option 4 @@ -161,18 +136,13 @@ describe('', () => { - - Option 1 - - - Option 2 - - - Option 3 - - - Option 4 + Option 1 + + Option 2 + Option 3 + Option 4 ); @@ -187,9 +157,7 @@ describe('', () => { - - - + ); From 33334780163cdb0e1ea24ea65e4e3d251b73b727 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Fri, 25 Jan 2019 08:34:49 -0800 Subject: [PATCH 05/10] Update breadcrumb documentation --- src/Breadcrumb/Breadcrumb.Component.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.Component.js b/src/Breadcrumb/Breadcrumb.Component.js index 2cc839896..83e0e0dd8 100644 --- a/src/Breadcrumb/Breadcrumb.Component.js +++ b/src/Breadcrumb/Breadcrumb.Component.js @@ -11,13 +11,13 @@ export const BreadcrumbComponent = () => { `; const breadcrumbLinkCode = ` - + Link Text - + Link Text - + Link Text `; @@ -46,16 +46,16 @@ export const BreadcrumbComponent = () => { - An example using link (routerLink) + An example using React Router's Link component - + Link Text - + Link Text - + Link Text From dd2a2f19838c14d0b98ad605753ce54683116d45 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Fri, 25 Jan 2019 08:39:16 -0800 Subject: [PATCH 06/10] Fix doc issue --- src/Breadcrumb/Breadcrumb.Component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Breadcrumb/Breadcrumb.Component.js b/src/Breadcrumb/Breadcrumb.Component.js index 83e0e0dd8..a5481d341 100644 --- a/src/Breadcrumb/Breadcrumb.Component.js +++ b/src/Breadcrumb/Breadcrumb.Component.js @@ -46,7 +46,7 @@ export const BreadcrumbComponent = () => { - An example using React Router's Link component + An example using React Router\'s Link component From 4b922cad4c71a428cfb1d53d56ce984e7ee623a0 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 10:15:12 -0800 Subject: [PATCH 07/10] PR feedback --- src/Breadcrumb/Breadcrumb.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index 0c32bfed3..3cfb4facf 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -33,14 +33,13 @@ export const BreadcrumbItem = ({ url, link, name, className, children, ...props ); return ( -
  • +
  • {renderLink()}
    • ); }; BreadcrumbItem.propTypes = { - link: PropTypes.string, name: PropTypes.string, url: PropTypes.string }; From 713ac0aba014949cbb351670b7a91adf7386a45f Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 18:00:51 -0800 Subject: [PATCH 08/10] Removing unused prop --- src/Breadcrumb/Breadcrumb.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index 3cfb4facf..1cfddfbb6 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -14,7 +14,7 @@ Breadcrumb.propDescriptions = { children: 'List item (`BreadcrumbItem`) nodes.' }; -export const BreadcrumbItem = ({ url, link, name, className, children, ...props }) => { +export const BreadcrumbItem = ({ url, name, className, children, ...props }) => { const renderLink = () => { if (!children && url) { return ( @@ -46,6 +46,5 @@ 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.' }; From dd8e8c7a5483657d56d5d1d9895f61ba9d8ec53b Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 18:08:04 -0800 Subject: [PATCH 09/10] Updating desc text --- src/Breadcrumb/Breadcrumb.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index 1cfddfbb6..345505660 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -45,6 +45,6 @@ BreadcrumbItem.propTypes = { }; BreadcrumbItem.propDescriptions = { - name: 'Localized display text of the link (for either `link` or `url`).', - 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: 'Ttext of the link (Only for `url` prop.).', + url: 'An anchor tag will be generated and set to the url prop. Name or child text must be provided.' }; From 8b83ab36f16f208355fcdf77b3672b99be6d68e3 Mon Sep 17 00:00:00 2001 From: Jeffrey Johnson Date: Mon, 28 Jan 2019 18:10:08 -0800 Subject: [PATCH 10/10] Update documentation --- src/Breadcrumb/Breadcrumb.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Breadcrumb/Breadcrumb.js b/src/Breadcrumb/Breadcrumb.js index 345505660..50f428f28 100644 --- a/src/Breadcrumb/Breadcrumb.js +++ b/src/Breadcrumb/Breadcrumb.js @@ -45,6 +45,6 @@ BreadcrumbItem.propTypes = { }; BreadcrumbItem.propDescriptions = { - name: 'Ttext of the link (Only for `url` prop.).', + 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.' };