Skip to content

Commit

Permalink
[ButtonBase] Explicit the need for a class component (mui#7656)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari authored and Sebastian Sebald committed Aug 7, 2017
1 parent 648d599 commit d6a6d77
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/Hidden/HiddenJs.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function HiddenJs(props: Props) {

warning(
Object.keys(other).length === 0,
`Material-UI: Unsupported properties received ${JSON.stringify(other)}`,
`Material-UI: unsupported properties received ${JSON.stringify(other)}`,
);

return children;
Expand Down
16 changes: 13 additions & 3 deletions src/internal/ButtonBase.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// @flow weak

import React, { Component } from 'react';
import type { Element } from 'react';
import { findDOMNode } from 'react-dom';
import warning from 'warning';
import classNames from 'classnames';
import keycode from 'keycode';
import createStyleSheet from '../styles/createStyleSheet';
Expand Down Expand Up @@ -109,6 +111,12 @@ class ButtonBase extends Component<DefaultProps, Props, State> {

componentDidMount() {
listenForFocusKeys();

warning(
this.button,
`Material-UI: please provide a class to the component property.
The keyboard focus logic needs a reference to work correctly.`,
);
}

componentWillUpdate(nextProps, nextState) {
Expand Down Expand Up @@ -208,10 +216,12 @@ class ButtonBase extends Component<DefaultProps, Props, State> {
return;
}

event.persist();
if (this.button) {
event.persist();

const keyboardFocusCallback = this.onKeyboardFocusHandler.bind(this, event);
detectKeyboardFocus(this, findDOMNode(this.button), keyboardFocusCallback);
const keyboardFocusCallback = this.onKeyboardFocusHandler.bind(this, event);
detectKeyboardFocus(this, findDOMNode(this.button), keyboardFocusCallback);
}

if (this.props.onFocus) {
this.props.onFocus(event);
Expand Down
51 changes: 43 additions & 8 deletions src/internal/ButtonBase.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import keycode from 'keycode';
import { assert } from 'chai';
import { spy, useFakeTimers } from 'sinon';
import { createShallow, createMount, getClasses } from '../test-utils';
import { focusKeyPressed } from '../utils/keyboardFocus';
import consoleErrorMock from '../../test/utils/consoleErrorMock';
import TouchRipple from './TouchRipple';
import ButtonBase, { styleSheet } from './ButtonBase';

Expand Down Expand Up @@ -315,9 +317,7 @@ describe('<ButtonBase />', () => {
</ButtonBase.Naked>,
);
instance = wrapper.instance();

button = document.getElementById('test-button');

if (!button) {
throw new Error('missing button');
}
Expand All @@ -332,13 +332,18 @@ describe('<ButtonBase />', () => {
clock.restore();
});

it('should not set keyboard focus before time has passed', () => {
assert.strictEqual(wrapper.state('keyboardFocused'), false, 'should not be keyboardFocused');
});

it('should listen for tab presses and set keyboard focus', () => {
it('should work', () => {
assert.strictEqual(
wrapper.state('keyboardFocused'),
false,
'should not set keyboard focus before time has passed',
);
clock.tick(instance.keyboardFocusCheckTime * instance.keyboardFocusMaxCheckTimes);
assert.strictEqual(wrapper.state('keyboardFocused'), true, 'should be keyboardFocused');
assert.strictEqual(
wrapper.state('keyboardFocused'),
true,
'should listen for tab presses and set keyboard focus',
);
});
});

Expand Down Expand Up @@ -367,6 +372,18 @@ describe('<ButtonBase />', () => {
});

describe('handleFocus()', () => {
let clock;

before(() => {
clock = useFakeTimers();
consoleErrorMock.spy();
});

after(() => {
clock.restore();
consoleErrorMock.reset();
});

it('when disabled should not persist event', () => {
const wrapper = mount(
<ButtonBase.Naked classes={{}} disabled>
Expand Down Expand Up @@ -394,6 +411,24 @@ describe('<ButtonBase />', () => {
assert.strictEqual(onKeyboardFocusSpy.callCount, 1);
assert.strictEqual(onKeyboardFocusSpy.calledWith(eventMock), true);
});

it('should work with a functionnal component', () => {
focusKeyPressed(true);
const MyLink = props =>
<a href="/foo" {...props}>
bar
</a>;
const wrapper = mount(
<ButtonBase.Naked classes={{}} component={MyLink}>
Hello
</ButtonBase.Naked>,
);
assert.match(consoleErrorMock.args()[0][0], /Material-UI: please provide a class/);
const instance = wrapper.instance();
instance.focusKeyPressed = true;
wrapper.simulate('focus');
clock.tick(instance.keyboardFocusCheckTime);
});
});

describe('handleKeyDown()', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/internal/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ class Modal extends Component<DefaultProps, Props, State> {
modalContent.setAttribute('tabIndex', -1);
warning(
false,
'Material-UI: The modal content node does not accept focus. ' +
'Material-UI: the modal content node does not accept focus. ' +
'For the benefit of assistive technologies, ' +
'the tabIndex of the node is being set to "-1".',
);
Expand Down
2 changes: 1 addition & 1 deletion src/styles/createGenerateClassName.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default function createGenerateClassName(): generateClassName {
warning(
ruleCounter < 1e10,
[
'Material-UI: You might have a memory leak.',
'Material-UI: you might have a memory leak.',
'The ruleCounter is not supposed to grow that much.',
].join(''),
);
Expand Down
2 changes: 1 addition & 1 deletion src/styles/withStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const withStyles = (styleSheet: Array<Object> | Object, options: Object = {}) =>
warning(
indexCounter < 0,
[
'Material-UI: You might have a memory leak.',
'Material-UI: you might have a memory leak.',
'The indexCounter is not supposed to grow that much.',
].join(' '),
);
Expand Down
2 changes: 1 addition & 1 deletion src/utils/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function createChainedFunction(...funcs: Array<any>) {
return funcs.filter(func => func != null).reduce((acc, func) => {
warning(
typeof func === 'function',
'Material-UI: Invalid Argument Type, must only provide functions, undefined, or null.',
'Material-UI: invalid Argument Type, must only provide functions, undefined, or null.',
);

if (acc === null) {
Expand Down

0 comments on commit d6a6d77

Please sign in to comment.