feat: implementing Breadcrumb component. #460
Conversation
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import './styles.css'; | ||
|
|
There was a problem hiding this comment.
add the description to the component
Pull Request Test Coverage Report for Build 883
💛 - Coveralls |
| import Breadcrumb from './../index'; | ||
|
|
||
| describe('<Breadcrumb/>', () => { | ||
| it('should fallback to href to href="javascript:void(0);" if the prop is not provided', () => { |
There was a problem hiding this comment.
should fallback the href prop to href="javascript:void(0);" if the prop is not provided
| it('should fallback to href to href="javascript:void(0);" if the prop is not provided', () => { | ||
| const component = mount(<Breadcrumb label="index" />); | ||
|
|
||
| expect(component.find('a[href="javascript:void(0);"]').exists()).toBe(true); |
There was a problem hiding this comment.
more descriptive:
expect(component.find('a').prop('href')).toBe('javascript:void(0);')
|
|
||
| expect(component.find('a[href="javascript:void(0);"]').exists()).toBe(true); | ||
| }); | ||
| it('should fallback to href to href passed', () => { |
There was a problem hiding this comment.
should set the href passed
| it('should fallback to href to href passed', () => { | ||
| const component = mount(<Breadcrumb label="index" href="index" />); | ||
|
|
||
| expect(component.find('a[href="index"]').exists()).toBe(true); |
|
|
||
| expect(component.find('a').text()).toBe('index'); | ||
| }); | ||
| it('should set the label passed as children', () => { |
There was a problem hiding this comment.
this test is twice
src/components/Breadcrumb/index.js
Outdated
| import PropTypes from 'prop-types'; | ||
| import classnames from 'classnames'; | ||
| import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; | ||
| import { faEllipsisH } from '@fortawesome/free-solid-svg-icons'; |
There was a problem hiding this comment.
Here we need to use internals icon for the component since fontawesome is not a library dependency. Ask for icons to @TahimiLeonBravo.
src/components/Breadcrumb/index.js
Outdated
| label, | ||
| onClick, | ||
| disabled, | ||
| overflowMenu, |
There was a problem hiding this comment.
In the api doc I don't see this prop. This is not needed, if we need this then pass a ButtonMenu direct as children of the Breadcrumbs, but this case is not used commonly.
| @@ -0,0 +1,64 @@ | |||
| ##### breadcrumb | |||
There was a problem hiding this comment.
You put more examples in here and I think that is more needed in Breadcrumbs. It is like Tabset and Tab.
Also remove the example with the overflow menu.
| } | ||
|
|
||
| .rainbow-breadcrumb-disabled > a { | ||
| color: #d2d4de !important; |
There was a problem hiding this comment.
let's try to not use !important
| import Breadcrumb from './../../Breadcrumb/index'; | ||
|
|
||
| describe('<Breadcrumbs/>', () => { | ||
| it('should have a aria-label attribute for accessibility', () => { |
There was a problem hiding this comment.
add test for:
role="navigation" in nav element
Also add a11y test
| } = props; | ||
|
|
||
| return ( | ||
| <nav aria-label="Breadcrumbs" style={style} className={className}> |
There was a problem hiding this comment.
here you left:
role="navigation"
src/components/Breadcrumbs/index.js
Outdated
| * This prop that should not be visible in the documentation. | ||
| * @ignore | ||
| */ | ||
| children: PropTypes.node, |
There was a problem hiding this comment.
children: PropTypes.oneOfType([
PropTypes.arrayOf(PropTypes.node),
PropTypes.object,
]),
src/components/Breadcrumbs/index.js
Outdated
| Breadcrumbs.defaultProps = { | ||
| className: undefined, | ||
| style: undefined, | ||
| children: null, |
| @@ -0,0 +1,94 @@ | |||
| /* eslint-disable import/no-extraneous-dependencies,no-script-url */ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
export the component in the root index
| @@ -0,0 +1,42 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
export the component in the root index
src/components/Breadcrumb/index.js
Outdated
|
|
||
| Breadcrumb.propTypes = { | ||
| /** The text label for the breadcrumb. */ | ||
| label: PropTypes.string.isRequired, |
There was a problem hiding this comment.
remember that the label must be a string or node
| it('should be accessible', async () => { | ||
| expect.assertions(1); | ||
| const html = ReactDOMServer.renderToString( | ||
| <Breadcrumbs activeTabName="tab-1"> |
There was a problem hiding this comment.
remove activeTabName
… into breadcrumb-component
fixes #430