From d340257061b03b8af882e7895a67070f7c3471c2 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 17 Sep 2019 12:45:18 +0200 Subject: [PATCH 1/2] [Breadcrumbs] Extend docs for items*Collapse --- docs/pages/api/breadcrumbs.md | 2 +- packages/material-ui/src/Breadcrumbs/Breadcrumbs.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/pages/api/breadcrumbs.md b/docs/pages/api/breadcrumbs.md index dcd654a1697873..390aa0f9380382 100644 --- a/docs/pages/api/breadcrumbs.md +++ b/docs/pages/api/breadcrumbs.md @@ -29,7 +29,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi | 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. | | 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 and last will be shown, with an ellipsis in between. | +| 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. | | separator | node | '/' | Custom separator node. | The `ref` is forwarded to the root element. diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js index 99f2e1c87fd832..c3ba9a1607421a 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.js @@ -155,8 +155,8 @@ Breadcrumbs.propTypes = { itemsBeforeCollapse: PropTypes.number, /** * Specifies the maximum number of breadcrumbs to display. When there are more - * than the maximum number, only the first and last will be shown, with an - * ellipsis in between. + * than the maximum number, only the first `itemsBeforeCollapse` and last `itemsAfterCollapse` + * will be shown, with an ellipsis in between. */ maxItems: PropTypes.number, /** From 09dee5ae11ff4506f3fafb1c7d6bb47df5cabf96 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 17 Sep 2019 16:18:44 +0200 Subject: [PATCH 2/2] [Breadcrumbs] Test with testing-library --- .../src/Breadcrumbs/Breadcrumbs.test.js | 109 ++++++++++-------- 1 file changed, 61 insertions(+), 48 deletions(-) diff --git a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js index f48db84d71e8e0..d381497f536ad3 100644 --- a/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js +++ b/packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js @@ -1,15 +1,15 @@ import React from 'react'; -import { assert } from 'chai'; +import { expect } from 'chai'; import { createMount, getClasses } from '@material-ui/core/test-utils'; import describeConformance from '../test-utils/describeConformance'; import Breadcrumbs from './Breadcrumbs'; -import BreadcrumbSeparator from './BreadcrumbSeparator'; -import BreadcrumbCollapsed from './BreadcrumbCollapsed'; import consoleErrorMock from 'test/utils/consoleErrorMock'; +import { cleanup, createClientRender } from 'test/utils/createClientRender'; describe('', () => { let mount; let classes; + const render = createClientRender({ strict: true }); before(() => { mount = createMount({ strict: true }); @@ -20,8 +20,8 @@ describe('', () => { ); }); - after(() => { - mount.cleanUp(); + afterEach(() => { + cleanup(); }); describeConformance(Conformance?, () => ({ @@ -30,53 +30,65 @@ describe('', () => { mount, refInstanceof: window.HTMLElement, testComponentPropWith: 'div', + after: () => mount.cleanUp(), })); - it('should render seperators', () => { - const wrapper = mount( + it('should render inaccessible seperators between each listitem', () => { + const { getAllByRole, getByRole } = render( - - + first + second , ); - assert.strictEqual(wrapper.find(BreadcrumbSeparator).length, 1); + + expect( + getAllByRole('listitem').filter(item => !item.matches('[aria-hidden="true"]')), + ).to.have.length(2); + + expect(getByRole('list')).to.have.text('first/second'); }); - it('should render an ellipse', () => { - const wrapper = mount( + it('should render an ellipse between `itemsAfterCollapse` and `itemsBeforeCollapse`', () => { + const { getAllByRole, getByRole } = render( - - - - - - - - - + first + second + third + fourth + fifth + sixth + seventh + eighth + ninth , ); - assert.strictEqual(wrapper.find(BreadcrumbSeparator).length, 2); - assert.strictEqual(wrapper.find(BreadcrumbCollapsed).length, 1); + + expect( + getAllByRole('listitem').filter(item => !item.matches('[aria-hidden="true"]')), + ).to.have.length(3); + expect(getByRole('list')).to.have.text('first//ninth'); + expect(getAllByRole('listitem')[2].querySelector('[data-mui-test="MoreHorizIcon"]')).to.be.ok; }); it('should expand when `BreadcrumbCollapsed` is clicked', () => { - const wrapper = mount( + const { getAllByRole } = render( - - - - - - - - - + first + second + third + fourth + fifth + sixth + seventh + eighth + ninth , ); - assert.strictEqual(wrapper.find(BreadcrumbSeparator).length, 2); - wrapper.find(BreadcrumbCollapsed).simulate('click'); - assert.strictEqual(wrapper.find(BreadcrumbSeparator).length, 8); + + getAllByRole('listitem')[2].click(); + + const items = getAllByRole('listitem').filter(item => !item.matches('[aria-hidden="true"]')); + expect(items).to.have.length(9); }); describe('warnings', () => { @@ -88,21 +100,22 @@ describe('', () => { consoleErrorMock.reset(); }); - it('should support invalid input', () => { - const wrapper = mount( + it('should warn about invalid input', () => { + const { getAllByRole, getByRole } = render( - - - - + first + second + third + fourth , ); - assert.strictEqual(wrapper.find(BreadcrumbSeparator).length, 3); - assert.strictEqual(wrapper.find(BreadcrumbCollapsed).length, 0); - assert.strictEqual(consoleErrorMock.callCount(), 2); - assert.include( - consoleErrorMock.args()[0][0], - 'you have provided an invalid combination of props to the', + expect( + getAllByRole('listitem').filter(item => !item.matches('[aria-hidden="true"]')), + ).to.have.length(4); + expect(getByRole('list')).to.have.text('first/second/third/fourth'); + expect(consoleErrorMock.callCount()).to.equal(2); // strict mode renders twice + expect(consoleErrorMock.args()[0][0]).to.include( + 'you have provided an invalid combination of props to the Breadcrumbs.\nitemsAfterCollapse={2} + itemsBeforeCollapse={2} >= maxItems={3}', ); }); });