Skip to content

Commit

Permalink
[Chip] Fix Firefox issue with the svg icon (#7327)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Jul 3, 2017
1 parent fd3b9f9 commit 9c59b56
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
4 changes: 3 additions & 1 deletion docs/src/components/MarkdownElement.js
Expand Up @@ -42,9 +42,11 @@ marked.setOptions({
const anchorLinkStyle = theme => ({
'& .anchor-link-style': {
opacity: 0,
display: 'inline',
// To prevent the link to get the focus.
display: 'none',
},
'&:hover .anchor-link-style': {
display: 'inline',
opacity: 1,
fontSize: '0.8em',
lineHeight: '1',
Expand Down
15 changes: 11 additions & 4 deletions src/Chip/Chip.js
Expand Up @@ -120,7 +120,7 @@ class Chip extends Component {
onClick,
onKeyDown,
onRequestDelete,
tabIndex,
tabIndex: tabIndexProp,
...other
} = this.props;

Expand All @@ -146,10 +146,17 @@ class Chip extends Component {
});
}

let tabIndex = tabIndexProp;

if (!tabIndex) {
tabIndex = onClick || onRequestDelete ? 0 : -1;
}

return (
<button
<div
role="button"
className={className}
tabIndex={onClick || onRequestDelete ? tabIndex : -1}
tabIndex={tabIndex}
onClick={onClick}
onKeyDown={this.handleKeyDown}
ref={node => {
Expand All @@ -162,7 +169,7 @@ class Chip extends Component {
{label}
</span>
{deleteIcon}
</button>
</div>
);
}
}
Expand Down
30 changes: 15 additions & 15 deletions src/Chip/Chip.spec.js
Expand Up @@ -28,8 +28,8 @@ describe('<Chip />', () => {
);
});

it('should render a button containing a span', () => {
assert.strictEqual(wrapper.name(), 'button');
it('should render a div containing a span', () => {
assert.strictEqual(wrapper.name(), 'div');
assert.strictEqual(wrapper.childAt(0).is('span'), true, 'should be a span');
});

Expand Down Expand Up @@ -57,8 +57,8 @@ describe('<Chip />', () => {
);
});

it('should render a button containing a span', () => {
assert.strictEqual(wrapper.name(), 'button');
it('should render a div containing a span', () => {
assert.strictEqual(wrapper.name(), 'div');
assert.strictEqual(wrapper.childAt(0).is('span'), true, 'should be a span');
});

Expand All @@ -69,8 +69,8 @@ describe('<Chip />', () => {
assert.strictEqual(wrapper.props().onClick, handleClick);
});

it('should not have a tabIndex prop', () => {
assert.strictEqual(wrapper.props().tabIndex, undefined);
it('should have a tabIndex prop', () => {
assert.strictEqual(wrapper.props().tabIndex, 0);
});

it('should apply user value of tabIndex', () => {
Expand Down Expand Up @@ -105,8 +105,8 @@ describe('<Chip />', () => {
);
});

it('should render a button containing an Avatar, span and svg', () => {
assert.strictEqual(wrapper.name(), 'button');
it('should render a div containing an Avatar, span and svg', () => {
assert.strictEqual(wrapper.name(), 'div');
assert.strictEqual(wrapper.childAt(0).is(Avatar), true, 'should have an Avatar');
assert.strictEqual(wrapper.childAt(1).is('span'), true, 'should have a span');
assert.strictEqual(wrapper.childAt(2).is('pure(Cancel)'), true, 'should be an svg icon');
Expand All @@ -124,8 +124,8 @@ describe('<Chip />', () => {
assert.strictEqual(wrapper.childAt(0).prop('data-my-prop'), 'woof');
});

it('should not have a tabIndex prop', () => {
assert.strictEqual(wrapper.props().tabIndex, undefined);
it('should have a tabIndex prop', () => {
assert.strictEqual(wrapper.props().tabIndex, 0);
});

it('should fire the function given in onDeleteRequest', () => {
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('<Chip />', () => {
const anyKeydownEvent = {
keyCode: keycode('p'),
};
wrapper.find('button').simulate('keydown', anyKeydownEvent);
wrapper.find('div').simulate('keydown', anyKeydownEvent);
assert.strictEqual(onKeyDownSpy.callCount, 1, 'should have called onKeyDown');
assert(onKeyDownSpy.calledWith(anyKeydownEvent));
});
Expand All @@ -193,7 +193,7 @@ describe('<Chip />', () => {
const wrapper2 = mount(<Chip.Naked classes={{}}>Text Chip</Chip.Naked>);
const handleBlur = spy();
wrapper2.instance().chipRef.blur = handleBlur;
wrapper2.find('button').simulate('keydown', {
wrapper2.find('div').simulate('keydown', {
preventDefault: () => {},
keyCode: keycode('esc'),
});
Expand All @@ -218,7 +218,7 @@ describe('<Chip />', () => {
preventDefault: preventDefaultSpy,
keyCode: keycode('space'),
};
wrapper.find('button').simulate('keydown', spaceKeydownEvent);
wrapper.find('div').simulate('keydown', spaceKeydownEvent);
assert.strictEqual(preventDefaultSpy.callCount, 1, 'should have stopped event propagation');
assert.strictEqual(onClickSpy.callCount, 1, 'should have called onClick');
assert(onClickSpy.calledWith(spaceKeydownEvent));
Expand All @@ -230,7 +230,7 @@ describe('<Chip />', () => {
preventDefault: preventDefaultSpy,
keyCode: keycode('enter'),
};
wrapper.find('button').simulate('keydown', enterKeydownEvent);
wrapper.find('div').simulate('keydown', enterKeydownEvent);
assert.strictEqual(preventDefaultSpy.callCount, 1, 'should have stopped event propagation');
assert.strictEqual(onClickSpy.callCount, 1, 'should have called onClick');
assert(onClickSpy.calledWith(enterKeydownEvent));
Expand All @@ -247,7 +247,7 @@ describe('<Chip />', () => {
preventDefault: preventDefaultSpy,
keyCode: keycode('backspace'),
};
wrapper.find('button').simulate('keydown', backspaceKeydownEvent);
wrapper.find('div').simulate('keydown', backspaceKeydownEvent);

assert.strictEqual(preventDefaultSpy.callCount, 1, 'should have stopped event propagation');
assert.strictEqual(onRequestDeleteSpy.callCount, 1, 'should have called onClick');
Expand Down

0 comments on commit 9c59b56

Please sign in to comment.