From 03482748b68ebba7de24491e957f705f48e32bd5 Mon Sep 17 00:00:00 2001 From: AlexKVal Date: Fri, 25 Sep 2015 21:47:00 +0300 Subject: [PATCH] [fixed] Breadcrumb and BreadcrumbItem components --- docs/examples/Breadcrumb.js | 2 +- docs/src/ComponentsPage.js | 9 ++- src/Breadcrumb.js | 24 ++++---- src/BreadcrumbItem.js | 42 ++++++++++--- src/styleMaps.js | 1 - test/BreadcrumbItemSpec.js | 117 ++++++++++++++++++------------------ test/BreadcrumbSpec.js | 86 +++----------------------- 7 files changed, 120 insertions(+), 161 deletions(-) diff --git a/docs/examples/Breadcrumb.js b/docs/examples/Breadcrumb.js index d8e9c4cc35..d881f5f6f6 100644 --- a/docs/examples/Breadcrumb.js +++ b/docs/examples/Breadcrumb.js @@ -6,7 +6,7 @@ const breadcrumbInstance = ( Library - + Data diff --git a/docs/src/ComponentsPage.js b/docs/src/ComponentsPage.js index 79a1014f0a..9ba1aaf324 100644 --- a/docs/src/ComponentsPage.js +++ b/docs/src/ComponentsPage.js @@ -544,15 +544,14 @@ const ComponentsPage = React.createClass({ {/* Breadcrumb */}

Breadcrumbs Breadcrumb, BreadcrumbItems

-

Breadcrumbs are used to indicate the current page's location. An active class is added to a BreadcrumbItem if there's no href property for it.

+

Breadcrumbs are used to indicate the current page's location. Add active attribute to active BreadcrumbItem.

+

Do not set both active and href attributes. active overrides href and span element is rendered instead of a.

Breadcrumbs Example

Props

- -

Breadcrumb

- +

Breadcrumb component itself doesn't have any specific public properties

BreadcrumbItem

@@ -964,7 +963,7 @@ const ComponentsPage = React.createClass({ Progress bars Navs Navbars - Breadcrumbs + Breadcrumbs Tabs Pager Pagination diff --git a/src/Breadcrumb.js b/src/Breadcrumb.js index 0f7e172b71..1f45d11f8f 100644 --- a/src/Breadcrumb.js +++ b/src/Breadcrumb.js @@ -1,10 +1,15 @@ import React, { cloneElement } from 'react'; import classNames from 'classnames'; -import BootstrapMixin from './BootstrapMixin'; import ValidComponentChildren from './utils/ValidComponentChildren'; const Breadcrumb = React.createClass({ - mixins: [BootstrapMixin], + propTypes: { + /** + * bootstrap className + * @private + */ + bsClass: React.PropTypes.string + }, getDefaultProps() { return { @@ -13,24 +18,21 @@ const Breadcrumb = React.createClass({ }, render() { - const classes = this.getBsClassSet(); const { className, ...props } = this.props; return ( -
    +
      {ValidComponentChildren.map(this.props.children, this.renderBreadcrumbItem)}
    ); }, renderBreadcrumbItem(child, index) { - return cloneElement( - child, - { - key: child.key ? child.key : index, - navItem: true - } - ); + return cloneElement( child, { key: child.key ? child.key : index } ); } }); diff --git a/src/BreadcrumbItem.js b/src/BreadcrumbItem.js index 823be75e57..ed3ee1d9f6 100644 --- a/src/BreadcrumbItem.js +++ b/src/BreadcrumbItem.js @@ -1,18 +1,39 @@ import React from 'react'; import classNames from 'classnames'; -import BootstrapMixin from './BootstrapMixin'; import SafeAnchor from './SafeAnchor'; import warning from 'react/lib/warning'; const BreadcrumbItem = React.createClass({ - mixins: [BootstrapMixin], - propTypes: { - id: React.PropTypes.string, + /** + * If set to true, renders `span` instead of `a` + */ active: React.PropTypes.bool, - linkId: React.PropTypes.string, + /** + * HTML id for the wrapper `li` element + */ + id: React.PropTypes.oneOfType([ + React.PropTypes.string, + React.PropTypes.number + ]), + /** + * HTML id for the inner `a` element + */ + linkId: React.PropTypes.oneOfType([ + React.PropTypes.string, + React.PropTypes.number + ]), + /** + * `href` attribute for the inner `a` element + */ href: React.PropTypes.string, + /** + * `title` attribute for the inner `a` element + */ title: React.PropTypes.node, + /** + * `target` attribute for the inner `a` element + */ target: React.PropTypes.string }, @@ -23,18 +44,19 @@ const BreadcrumbItem = React.createClass({ }, render() { - warning(!(this.props.href && this.props.active), '[react-bootstrap] href and active properties cannot be set at the same time'); - const { - id, active, + className, + id, linkId, children, href, title, target, ...props } = this.props; - const classes = { active }; + + warning(!(href && active), '[react-bootstrap] `href` and `active` properties cannot be set at the same time'); + const linkProps = { href, title, @@ -43,7 +65,7 @@ const BreadcrumbItem = React.createClass({ }; return ( -
  1. +
  2. { active ? diff --git a/src/styleMaps.js b/src/styleMaps.js index 01f9abc6fc..4360d1a675 100644 --- a/src/styleMaps.js +++ b/src/styleMaps.js @@ -1,7 +1,6 @@ const styleMaps = { CLASSES: { 'alert': 'alert', - 'breadcrumb': 'breadcrumb', 'button': 'btn', 'button-group': 'btn-group', 'button-toolbar': 'btn-toolbar', diff --git a/test/BreadcrumbItemSpec.js b/test/BreadcrumbItemSpec.js index 9f98531de2..0ce019fa01 100644 --- a/test/BreadcrumbItemSpec.js +++ b/test/BreadcrumbItemSpec.js @@ -1,137 +1,140 @@ import React from 'react'; import ReactTestUtils from 'react/lib/ReactTestUtils'; import BreadcrumbItem from '../src/BreadcrumbItem'; +import { shouldWarn } from './helpers'; -describe('BreadcrumbItem', function () { - it('Should add active class', function () { - let instance = ReactTestUtils.renderIntoDocument( - - Active Crumb +describe('BreadcrumbItem', () => { + it('Should warn if `active` and `href` attributes set', () => { + ReactTestUtils.renderIntoDocument( + + Crumb ); - assert.ok(ReactTestUtils.findRenderedDOMComponentWithClass(instance, 'active')); + shouldWarn('[react-bootstrap] `href` and `active` properties cannot be set at the same time'); }); - it('Should not add active class', function () { - let instance = ReactTestUtils.renderIntoDocument( - + it('Should render `a` as inner element when is not active', () => { + const instance = ReactTestUtils.renderIntoDocument( + Crumb ); - let liNode = React.findDOMNode(instance); - assert.notInclude(liNode.className, 'active'); + assert.ok(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); + assert.notInclude(React.findDOMNode(instance).className, 'active'); }); - it('Should add custom classes', function () { - let instance = ReactTestUtils.renderIntoDocument( - + it('Should add `active` class with `active` attribute set.', () => { + const instance = ReactTestUtils.renderIntoDocument( + Active Crumb ); - let liNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithClass(instance, 'active')); + assert.include(React.findDOMNode(instance).className, 'active'); + }); - let classes = liNode.className; - assert.include(classes, 'active'); - assert.include(classes, 'custom-one'); - assert.include(classes, 'custom-two'); + it('Should render `span` as inner element when is active', () => { + const instance = ReactTestUtils.renderIntoDocument( + + Crumb + + ); + + assert.ok(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'span')); }); - it('Should spread props onto an active item', function() { - let instance = ReactTestUtils.renderIntoDocument( - + it('Should add custom classes onto `li` wrapper element', () => { + const instance = ReactTestUtils.renderIntoDocument( + Active Crumb ); - let spanNode = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'span'); - - spanNode.props.herpa.should.equal('derpa'); + const classes = React.findDOMNode(instance).className; + assert.include(classes, 'custom-one'); + assert.include(classes, 'custom-two'); }); - it('Should spread props onto anchor', function(done) { + it('Should spread additional props onto inner element', (done) => { const handleClick = () => { done(); }; - let instance = ReactTestUtils.renderIntoDocument( - - Crumb 1 + const instance = ReactTestUtils.renderIntoDocument( + + Crumb ); - let anchorNode = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a'); + const anchorNode = ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a'); ReactTestUtils.Simulate.click(anchorNode); - - anchorNode.props.herpa.should.equal('derpa'); }); - it('Should add id for li element', function() { - let instance = ReactTestUtils.renderIntoDocument( + it('Should apply id onto `li` wrapper element via `id` property', () => { + const instance = ReactTestUtils.renderIntoDocument( - Crumb 1 + Crumb ); - let liNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'li')); - assert.equal(liNode.id, 'test-li-id'); + assert.equal(React.findDOMNode(instance).id, 'test-li-id'); }); - it('Should add linkId', function() { - let instance = ReactTestUtils.renderIntoDocument( + it('Should apply id onto `a` inner alement via `linkId` property', () => { + const instance = ReactTestUtils.renderIntoDocument( - Crumb 1 + Crumb ); - let linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); + const linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); assert.equal(linkNode.id, 'test-link-id'); }); - it('Should add href', function() { - let instance = ReactTestUtils.renderIntoDocument( + it('Should apply `href` property onto `a` inner element', () => { + const instance = ReactTestUtils.renderIntoDocument( - Crumb 1 + Crumb ); - let linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); + const linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); assert.equal(linkNode.href, 'http://getbootstrap.com/components/#breadcrumbs'); }); - it('Should have a title', function() { - let instance = ReactTestUtils.renderIntoDocument( + it('Should apply `title` property onto `a` inner element', () => { + const instance = ReactTestUtils.renderIntoDocument( - Crumb 1 + Crumb ); - let linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); + const linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); assert.equal(linkNode.title, 'test-title'); }); - it('Should not add anchor properties to li', function() { - let instance = ReactTestUtils.renderIntoDocument( + it('Should not apply properties for inner `anchor` onto `li` wrapper element', () => { + const instance = ReactTestUtils.renderIntoDocument( - Crumb 1 + Crumb ); - let liNode = React.findDOMNode(instance); + const liNode = React.findDOMNode(instance); assert.notOk(liNode.hasAttribute('href')); assert.notOk(liNode.hasAttribute('title')); }); - it('Should set target attribute on anchor', function () { - let instance = ReactTestUtils.renderIntoDocument( + it('Should set `target` attribute on `anchor`', () => { + const instance = ReactTestUtils.renderIntoDocument( - Crumb 1 + Crumb ); - let linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); + const linkNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'a')); assert.equal(linkNode.target, '_blank'); }); }); diff --git a/test/BreadcrumbSpec.js b/test/BreadcrumbSpec.js index 9abbc24634..eb74c1c2ac 100644 --- a/test/BreadcrumbSpec.js +++ b/test/BreadcrumbSpec.js @@ -1,32 +1,9 @@ import React from 'react'; import ReactTestUtils from 'react/lib/ReactTestUtils'; import Breadcrumb from '../src/Breadcrumb'; -import BreadcrumbItem from '../src/BreadcrumbItem'; -describe('Breadcrumb', function () { - it('Should able to wrap react component', function () { - let instance = ReactTestUtils.renderIntoDocument( - - - Crumb 1 - - -
    Active Crumb Component
    -     -     - ); - - let items = ReactTestUtils.scryRenderedDOMComponentsWithTag(instance, 'li'); - - let linkNode = React.findDOMNode(items[0]).childNodes[0]; - let spanNode = linkNode.childNodes[0]; - assert.equal(spanNode.className, 'custom-span-class'); - - let divNode = React.findDOMNode(items[1]).childNodes[0].childNodes[0]; - assert.equal(divNode.className, 'custom-div-class'); - }); - - it('Should apply id to the wrapper ol element', function () { +describe('Breadcrumb', () => { + it('Should apply id to the wrapper ol element', () => { let instance = ReactTestUtils.renderIntoDocument( ); @@ -35,40 +12,18 @@ describe('Breadcrumb', function () { assert.equal(olNode.id, 'custom-id'); }); - it('Should have breadcrumb class', function () { + it('Should have breadcrumb class', () => { let instance = ReactTestUtils.renderIntoDocument( - - - Crumb 1 - - - Crumb 2 - - - Active Crumb - - + ); - assert.ok(ReactTestUtils.findRenderedDOMComponentWithClass(instance, 'breadcrumb')); - let olNode = React.findDOMNode(ReactTestUtils.findRenderedDOMComponentWithTag(instance, 'ol')); assert.include(olNode.className, 'breadcrumb'); }); - it('Should have custom classes', function () { + it('Should have custom classes', () => { let instance = ReactTestUtils.renderIntoDocument( - - - Crumb 1 - - - Crumb 2 - - - Active Crumb - - + ); let olNode = React.findDOMNode(ReactTestUtils.findRenderedComponentWithType(instance, Breadcrumb)); @@ -79,39 +34,18 @@ describe('Breadcrumb', function () { assert.include(classes, 'custom-two'); }); - it('Should have a navigation role in ol', function () { + it('Should have a navigation role', () => { let instance = ReactTestUtils.renderIntoDocument( - - - Crumb 1 - - - Crumb 2 - - - Active Crumb - - + ); - let olNode = React.findDOMNode(ReactTestUtils.findRenderedComponentWithType(instance, Breadcrumb)); assert.equal(olNode.getAttribute('role'), 'navigation'); }); - it('Should have a aria-label in ol', function () { + it('Should have an aria-label in ol', () => { let instance = ReactTestUtils.renderIntoDocument( - - - Crumb 1 - - - Crumb 2 - - - Active Crumb - - + ); let olNode = React.findDOMNode(ReactTestUtils.findRenderedComponentWithType(instance, Breadcrumb));