From 2ea39ce3ffac816183a5363b1bcc1d826abc775d Mon Sep 17 00:00:00 2001 From: Serhii Date: Sat, 15 Feb 2020 21:49:15 +0100 Subject: [PATCH 01/11] add logic --- .../src/Breadcrumbs/BreadcrumbCollapsed.js | 15 +++++++++++++-- .../material-ui/src/Breadcrumbs/Breadcrumbs.js | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index 75a5984c198fac..e2cb5f01e90099 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -31,9 +31,20 @@ const styles = theme => ({ * @ignore - internal component. */ function BreadcrumbCollapsed(props) { - const { classes, ...other } = props; + const { classes, onClick, ...other } = props; + + const handleKeyDown = e => { + e.preventDefault(); + switch (e.keyCode) { + case 32 /** Space */: + case 13 /** Enter */: { + onClick(); + } + } + }; + return ( -
  • +
    • ); diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js index 5e8075f5611bc1..6a5d9287def95d 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js @@ -110,6 +110,7 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) { return ( Date: Sat, 15 Feb 2020 22:42:45 +0100 Subject: [PATCH 02/11] add tests --- .../src/Breadcrumbs/BreadcrumbCollapsed.js | 18 +++++++++++------ .../Breadcrumbs/BreadcrumbCollapsed.test.js | 20 +++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index e2cb5f01e90099..fa22d418721cac 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -34,17 +34,23 @@ function BreadcrumbCollapsed(props) { const { classes, onClick, ...other } = props; const handleKeyDown = e => { - e.preventDefault(); - switch (e.keyCode) { - case 32 /** Space */: - case 13 /** Enter */: { + e.preventDefault(); + switch (e.key) { + case 'Space': + case 'Enter': { onClick(); } } }; - + return ( -
  • +
    • ); diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js index 6a19bd311b0c5f..13406f99cd786b 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js @@ -1,5 +1,6 @@ import React from 'react'; import { assert } from 'chai'; +import { spy } from 'sinon'; import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils'; import BreadcrumbCollapsed from './BreadcrumbCollapsed'; import MoreHorizIcon from '../internal/svg-icons/MoreHoriz'; @@ -26,6 +27,25 @@ describe('', () => { assert.strictEqual(wrapper.hasClass(classes.root), true); }); + it('should be focusable', () => { + const wrapper = shallow(); + const listElement = wrapper.find('li'); + assert.strictEqual(listElement.prop('tabIndex'), 0); + }); + + describe('prop: onClick', () => { + ['Space', 'Enter'].forEach(key => { + it(`should be called on key press - ${key}`, () => { + const handleClick = spy(); + const wrapper = mount(); + const listElement = wrapper.find('li'); + listElement.simulate('focus'); + listElement.simulate('keydown', { key }); + assert.strictEqual(handleClick.callCount, 1); + }); + }); + }); + it('should mount', () => { mount(); }); From 0fd97983feaca656c4d39a8cf4d03427f9ceb183 Mon Sep 17 00:00:00 2001 From: Serhii Date: Sun, 16 Feb 2020 14:51:52 +0100 Subject: [PATCH 03/11] remove aria-label --- packages/material-ui/src/Breadcrumbs/Breadcrumbs.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js index 6a5d9287def95d..5e8075f5611bc1 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js @@ -110,7 +110,6 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) { return ( Date: Sun, 16 Feb 2020 15:02:56 +0100 Subject: [PATCH 04/11] Add ButtonBase --- .../src/Breadcrumbs/BreadcrumbCollapsed.js | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index fa22d418721cac..7fb3628525828f 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import withStyles from '../styles/withStyles'; import { emphasize } from '../styles/colorManipulator'; import MoreHorizIcon from '../internal/svg-icons/MoreHoriz'; +import ButtonBase from '../ButtonBase'; const styles = theme => ({ root: { @@ -31,27 +32,13 @@ const styles = theme => ({ * @ignore - internal component. */ function BreadcrumbCollapsed(props) { - const { classes, onClick, ...other } = props; - - const handleKeyDown = e => { - e.preventDefault(); - switch (e.key) { - case 'Space': - case 'Enter': { - onClick(); - } - } - }; + const { classes, ...other } = props; return ( -
  • - +
  • + + +
    • ); } From 8dee6f3228ec6f09b68143fe2a182789b689cf3b Mon Sep 17 00:00:00 2001 From: Serhii Date: Sun, 16 Feb 2020 15:04:19 +0100 Subject: [PATCH 05/11] Add label --- packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index 7fb3628525828f..3e530d0b90f39a 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -36,7 +36,7 @@ function BreadcrumbCollapsed(props) { return (
  • - +
    • From b4a71778cf302141c6a962fbb6bde534705e9e09 Mon Sep 17 00:00:00 2001 From: Serhii Date: Tue, 18 Feb 2020 19:09:09 +0100 Subject: [PATCH 06/11] CR update --- .../src/Breadcrumbs/BreadcrumbCollapsed.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index 3e530d0b90f39a..fe353cd35e44b8 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -33,13 +33,18 @@ const styles = theme => ({ */ function BreadcrumbCollapsed(props) { const { classes, ...other } = props; + const handleKeyDown = e => e.preventDefault(); return ( -
  • - - - -
    • + + + ); } From 36b925bd3773ac8f96a11cbae6291ad5dee26694 Mon Sep 17 00:00:00 2001 From: Serhii Date: Tue, 18 Feb 2020 20:03:13 +0100 Subject: [PATCH 07/11] update unit tests --- .../src/Breadcrumbs/BreadcrumbCollapsed.js | 10 ++++++++-- .../src/Breadcrumbs/BreadcrumbCollapsed.test.js | 17 ++++++----------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index fe353cd35e44b8..c445454849556f 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -32,8 +32,14 @@ const styles = theme => ({ * @ignore - internal component. */ function BreadcrumbCollapsed(props) { - const { classes, ...other } = props; - const handleKeyDown = e => e.preventDefault(); + const { classes, onKeyDown, ...other } = props; + + const handleKeyDown = e => { + e.preventDefault(); + if (onKeyDown) { + onKeyDown(e); + } + }; return ( ', () => { assert.strictEqual(wrapper.hasClass(classes.root), true); }); - it('should be focusable', () => { - const wrapper = shallow(); - const listElement = wrapper.find('li'); - assert.strictEqual(listElement.prop('tabIndex'), 0); - }); - - describe('prop: onClick', () => { + describe('prop: onKeyDown', () => { ['Space', 'Enter'].forEach(key => { it(`should be called on key press - ${key}`, () => { - const handleClick = spy(); - const wrapper = mount(); + const handleKeyDown = spy(); + const wrapper = mount(); const listElement = wrapper.find('li'); listElement.simulate('focus'); - listElement.simulate('keydown', { key }); - assert.strictEqual(handleClick.callCount, 1); + listElement.simulate('keydown', { key }); + + assert.strictEqual(handleKeyDown.callCount, 1); }); }); }); From db54d6bace72530655bef3bd92a2ab0ebb5d1b15 Mon Sep 17 00:00:00 2001 From: Serhii Date: Tue, 18 Feb 2020 21:34:20 +0100 Subject: [PATCH 08/11] update unit tests --- .../material-ui/src/Breadcrumbs/Breadcrumbs.test.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js index f142d2ff06564c..eea261f96a174d 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js @@ -57,13 +57,14 @@ describe('', () => { ); const listitems = getAllByRole('listitem', { hidden: false }); - expect(listitems).to.have.length(3); + + expect(listitems).to.have.length(2); expect(getByRole('list')).to.have.text('first//ninth'); - expect(listitems[1].querySelector('[data-mui-test="MoreHorizIcon"]')).to.be.ok; + expect(getByRole('button').querySelector('[data-mui-test="MoreHorizIcon"]')).to.be.ok; }); it('should expand when `BreadcrumbCollapsed` is clicked', () => { - const { getAllByRole } = render( + const { getAllByRole, getByRole } = render( first second @@ -77,9 +78,9 @@ describe('', () => { , ); - getAllByRole('listitem', { hidden: false })[2].click(); - - expect(getAllByRole('listitem', { hidden: false })).to.have.length(3); + getByRole('button').click(); + + expect(getAllByRole('listitem', { hidden: false })).to.have.length(9); }); describe('warnings', () => { From 26f845c46bb205188a956ac3906716246ceb2390 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Tue, 18 Feb 2020 21:17:48 +0100 Subject: [PATCH 09/11] use testing-library --- docs/pages/api/breadcrumbs.md | 1 + .../src/Breadcrumbs/BreadcrumbCollapsed.js | 10 +--- .../Breadcrumbs/BreadcrumbCollapsed.test.js | 51 +++++++++---------- .../src/Breadcrumbs/Breadcrumbs.js | 9 +++- packages/material-ui/src/locale/index.js | 3 ++ 5 files changed, 36 insertions(+), 38 deletions(-) diff --git a/docs/pages/api/breadcrumbs.md b/docs/pages/api/breadcrumbs.md index fd9848a5234ef3..5c02c2381ceac6 100644 --- a/docs/pages/api/breadcrumbs.md +++ b/docs/pages/api/breadcrumbs.md @@ -27,6 +27,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi | children * | node | | The breadcrumb children. | | classes | object | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. | | component | elementType | 'nav' | The component used for the root node. Either a string to use a DOM element or a component. By default, it maps the variant to a good default headline component. | +| expandText | string | 'Show path' | Override the default label for the expand button.
    For localization purposes, you can use the provided [translations](/guides/localization/). | | itemsAfterCollapse | number | 1 | If max items is exceeded, the number of items to show after the ellipsis. | | itemsBeforeCollapse | number | 1 | If max items is exceeded, the number of items to show before the ellipsis. | | maxItems | number | 8 | Specifies the maximum number of breadcrumbs to display. When there are more than the maximum number, only the first `itemsBeforeCollapse` and last `itemsAfterCollapse` will be shown, with an ellipsis in between. | diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index c445454849556f..e9da8c9f151dd4 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -32,20 +32,12 @@ const styles = theme => ({ * @ignore - internal component. */ function BreadcrumbCollapsed(props) { - const { classes, onKeyDown, ...other } = props; - - const handleKeyDown = e => { - e.preventDefault(); - if (onKeyDown) { - onKeyDown(e); - } - }; + const { classes, ...other } = props; return ( diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js index 57375f6b9e65b0..d90897d017872c 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js @@ -1,47 +1,42 @@ import React from 'react'; -import { assert } from 'chai'; +import { expect } from 'chai'; import { spy } from 'sinon'; -import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils'; +import { getClasses } from '@material-ui/core/test-utils'; import BreadcrumbCollapsed from './BreadcrumbCollapsed'; -import MoreHorizIcon from '../internal/svg-icons/MoreHoriz'; +import { fireEvent, createClientRender } from 'test/utils/createClientRender'; describe('', () => { - let mount; - let shallow; let classes; + const render = createClientRender(); before(() => { - shallow = createShallow({ dive: true }); - mount = createMount({ strict: true }); classes = getClasses(); }); - after(() => { - mount.cleanUp(); - }); - - it('should render an ', () => { - const wrapper = shallow(); + it('should render an icon', () => { + const { container } = render(); - assert.strictEqual(wrapper.find(MoreHorizIcon).length, 1); - assert.strictEqual(wrapper.hasClass(classes.root), true); + expect(container.querySelectorAll('svg').length).to.equal(1); + expect(container.firstChild).to.have.class(classes.root); }); describe('prop: onKeyDown', () => { - ['Space', 'Enter'].forEach(key => { - it(`should be called on key press - ${key}`, () => { - const handleKeyDown = spy(); - const wrapper = mount(); - const listElement = wrapper.find('li'); - listElement.simulate('focus'); - listElement.simulate('keydown', { key }); - - assert.strictEqual(handleKeyDown.callCount, 1); - }); + it(`should be called on key down - Enter`, () => { + const handleClick = spy(); + const { container } = render(); + const expand = container.firstChild; + expand.focus(); + fireEvent.keyDown(expand, { key: 'Enter' }); + expect(handleClick.callCount).to.equal(1); }); - }); - it('should mount', () => { - mount(); + it(`should be called on key up - Space`, () => { + const handleClick = spy(); + const { container } = render(); + const expand = container.firstChild; + expand.focus(); + fireEvent.keyUp(expand, { key: ' ' }); + expect(handleClick.callCount).to.equal(1); + }); }); }); diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js index 5e8075f5611bc1..4ff1e7c046f440 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js @@ -52,6 +52,7 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) { classes, className, component: Component = 'nav', + expandText = 'Show path', itemsAfterCollapse = 1, itemsBeforeCollapse = 1, maxItems = 8, @@ -82,7 +83,7 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) { return [ ...allItems.slice(0, itemsBeforeCollapse), - , + , ...allItems.slice(allItems.length - itemsAfterCollapse, allItems.length), ]; }; @@ -149,6 +150,12 @@ Breadcrumbs.propTypes = { * By default, it maps the variant to a good default headline component. */ component: PropTypes.elementType, + /** + * Override the default label for the expand button. + * + * For localization purposes, you can use the provided [translations](/guides/localization/). + */ + expandText: PropTypes.string, /** * If max items is exceeded, the number of items to show after the ellipsis. */ diff --git a/packages/material-ui/src/locale/index.js b/packages/material-ui/src/locale/index.js index aa4162fe6f1210..c820f717d96af3 100644 --- a/packages/material-ui/src/locale/index.js +++ b/packages/material-ui/src/locale/index.js @@ -291,6 +291,9 @@ export const fiFI = { export const frFR = { props: { + MuiBreadcrumbs: { + expandText: 'Montrer le chemin', + }, MuiTablePagination: { backIconButtonText: 'Page précédente', labelRowsPerPage: 'Lignes par page :', From c77ae7f2e5baa151c2923c1666d4bc338e7f398a Mon Sep 17 00:00:00 2001 From: Serhii Date: Tue, 18 Feb 2020 21:39:56 +0100 Subject: [PATCH 10/11] prettier --- .../material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js | 7 +------ packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index e9da8c9f151dd4..7238c1f6c92436 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -35,12 +35,7 @@ function BreadcrumbCollapsed(props) { const { classes, ...other } = props; return ( - + ); diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js index eea261f96a174d..c2caad2659e5df 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js @@ -79,7 +79,7 @@ describe('', () => { ); getByRole('button').click(); - + expect(getAllByRole('listitem', { hidden: false })).to.have.length(9); }); From 42b97a61f5e78515068138f611d7a57eac3030ba Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Thu, 20 Feb 2020 11:35:52 +0100 Subject: [PATCH 11/11] fix ripple style --- .../src/Breadcrumbs/BreadcrumbCollapsed.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js index 7238c1f6c92436..9d365e5653e557 100644 --- a/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js +++ b/packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js @@ -8,15 +8,11 @@ import ButtonBase from '../ButtonBase'; const styles = theme => ({ root: { display: 'flex', - }, - icon: { - width: 24, - height: 16, + marginLeft: theme.spacing(0.5), + marginRight: theme.spacing(0.5), backgroundColor: theme.palette.grey[100], color: theme.palette.grey[700], borderRadius: 2, - marginLeft: theme.spacing(0.5), - marginRight: theme.spacing(0.5), cursor: 'pointer', '&:hover, &:focus': { backgroundColor: theme.palette.grey[200], @@ -26,6 +22,10 @@ const styles = theme => ({ backgroundColor: emphasize(theme.palette.grey[200], 0.12), }, }, + icon: { + width: 24, + height: 16, + }, }); /**