Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IconButton] Add disable ripple props #15864

Merged
merged 4 commits into from May 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/material-ui/src/IconButton/IconButton.d.ts
Expand Up @@ -4,8 +4,9 @@ import { SimplifiedPropsOf } from '../OverridableComponent';

declare const IconButton: ExtendButtonBase<{
props: {
edge?: 'start' | 'end' | false;
color?: PropTypes.Color;
disableFocusRipple?: boolean;
edge?: 'start' | 'end' | false;
size?: 'small' | 'medium';
};
defaultComponent: 'button';
Expand Down
12 changes: 11 additions & 1 deletion packages/material-ui/src/IconButton/IconButton.js
Expand Up @@ -100,6 +100,7 @@ const IconButton = React.forwardRef(function IconButton(props, ref) {
className,
color = 'default',
disabled = false,
disableFocusRipple = false,
size = 'medium',
...other
} = props;
Expand All @@ -118,7 +119,7 @@ const IconButton = React.forwardRef(function IconButton(props, ref) {
className,
)}
centerRipple
focusRipple
focusRipple={!disableFocusRipple}
disabled={disabled}
ref={ref}
{...other}
Expand Down Expand Up @@ -168,6 +169,15 @@ IconButton.propTypes = {
* If `true`, the button will be disabled.
*/
disabled: PropTypes.bool,
/**
* If `true`, the keyboard focus ripple will be disabled.
* `disableRipple` must also be true.
*/
disableFocusRipple: PropTypes.bool,
/**
* If `true`, the ripple effect will be disabled.
*/
disableRipple: PropTypes.bool,
/**
* If given, uses a negative margin to counteract the padding on one
* side (this is often helpful for aligning the left or right
Expand Down
10 changes: 10 additions & 0 deletions packages/material-ui/src/IconButton/IconButton.test.js
Expand Up @@ -77,6 +77,16 @@ describe('<IconButton />', () => {
assert.strictEqual(wrapper.props().centerRipple, true);
});

it('should have a focusRipple by default', () => {
const wrapper = shallow(<IconButton>book</IconButton>);
assert.strictEqual(wrapper.props().focusRipple, true);
});

it('should pass disableFocusRipple to ButtonBase', () => {
const wrapper = shallow(<IconButton disableFocusRipple>book</IconButton>);
assert.strictEqual(wrapper.props().focusRipple, false);
});

describe('prop: size', () => {
it('should render the right class', () => {
let wrapper;
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/Tab/Tab.d.ts
Expand Up @@ -4,15 +4,16 @@ import { SimplifiedPropsOf } from '../OverridableComponent';

declare const Tab: ExtendButtonBase<{
props: {
disableFocusRipple?: boolean;
fullWidth?: boolean;
icon?: string | React.ReactElement;
value?: any;
label?: React.ReactNode;
onChange?: (event: React.ChangeEvent<{ checked: boolean }>, value: any) => void;
onClick?: React.EventHandler<any>;
selected?: boolean;
style?: React.CSSProperties;
textColor?: string | 'secondary' | 'primary' | 'inherit';
value?: any;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary for the ripple?

Copy link
Member

@leMaik leMaik May 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was just moved down (sorted alphabetically), it's the value prop the Tab had before.

wrapped?: boolean;
};
defaultComponent: 'div';
Expand Down
12 changes: 11 additions & 1 deletion packages/material-ui/src/Tab/Tab.js
Expand Up @@ -98,6 +98,7 @@ const Tab = React.forwardRef(function Tab(props, ref) {
classes,
className,
disabled = false,
disableFocusRipple = false,
fullWidth,
icon,
indicator,
Expand All @@ -123,7 +124,7 @@ const Tab = React.forwardRef(function Tab(props, ref) {

return (
<ButtonBase
focusRipple
focusRipple={!disableFocusRipple}
className={clsx(
classes.root,
classes[`textColor${capitalize(textColor)}`],
Expand Down Expand Up @@ -171,6 +172,15 @@ Tab.propTypes = {
* If `true`, the tab will be disabled.
*/
disabled: PropTypes.bool,
/**
* If `true`, the keyboard focus ripple will be disabled.
* `disableRipple` must also be true.
*/
disableFocusRipple: PropTypes.bool,
/**
* If `true`, the ripple effect will be disabled.
*/
disableRipple: PropTypes.bool,
/**
* @ignore
*/
Expand Down
20 changes: 20 additions & 0 deletions packages/material-ui/src/Tab/Tab.test.js
Expand Up @@ -37,6 +37,26 @@ describe('<Tab />', () => {
skip: ['componentProp'],
}));

it('should have a ripple by default', () => {
const wrapper = shallow(<Tab />);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are moving away from shallow tests. Can we use the mount API?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert.strictEqual(wrapper.props().disableRipple, undefined);
});

it('should pass disableRipple to ButtonBase', () => {
const wrapper = shallow(<Tab disableRipple />);
assert.strictEqual(wrapper.props().disableRipple, true);
});

it('should have a focusRipple by default', () => {
const wrapper = shallow(<Tab />);
assert.strictEqual(wrapper.props().focusRipple, true);
});

it('should pass disableFocusRipple to ButtonBase', () => {
const wrapper = shallow(<Tab disableFocusRipple />);
assert.strictEqual(wrapper.props().focusRipple, false);
});

describe('prop: selected', () => {
it('should render with the selected and root classes', () => {
const wrapper = mount(<Tab selected textColor="secondary" />);
Expand Down
2 changes: 2 additions & 0 deletions pages/api/icon-button.md
Expand Up @@ -23,6 +23,8 @@ regarding the available icon options.
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">color</span> | <span class="prop-type">enum:&nbsp;'default'&nbsp;&#124;<br>&nbsp;'inherit'&nbsp;&#124;<br>&nbsp;'primary'&nbsp;&#124;<br>&nbsp;'secondary'<br></span> | <span class="prop-default">'default'</span> | The color of the component. It supports those theme colors that make sense for this component. |
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the button will be disabled. |
| <span class="prop-name">disableFocusRipple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the keyboard focus ripple will be disabled. `disableRipple` must also be true. |
| <span class="prop-name">disableRipple</span> | <span class="prop-type">bool</span> | | If `true`, the ripple effect will be disabled. |
| <span class="prop-name">edge</span> | <span class="prop-type">enum:&nbsp;'start'&nbsp;&#124;<br>&nbsp;'end'&nbsp;&#124;<br>&nbsp;false<br></span> | <span class="prop-default">false</span> | If given, uses a negative margin to counteract the padding on one side (this is often helpful for aligning the left or right side of the icon with content above or below, without ruining the border size and shape). |
| <span class="prop-name">size</span> | <span class="prop-type">enum:&nbsp;'small'&nbsp;&#124;<br>&nbsp;'medium'<br></span> | <span class="prop-default">'medium'</span> | The size of the button. `small` is equivalent to the dense button styling. |

Expand Down
2 changes: 2 additions & 0 deletions pages/api/tab.md
Expand Up @@ -21,6 +21,8 @@ import Tab from '@material-ui/core/Tab';
| <span class="prop-name">children</span> | <span class="prop-type">unsupportedProp</span> | | This property isn't supported. Use the `component` property if you need to change the children structure. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the tab will be disabled. |
| <span class="prop-name">disableFocusRipple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the keyboard focus ripple will be disabled. `disableRipple` must also be true. |
| <span class="prop-name">disableRipple</span> | <span class="prop-type">bool</span> | | If `true`, the ripple effect will be disabled. |
| <span class="prop-name">icon</span> | <span class="prop-type">node</span> | | The icon element. |
| <span class="prop-name">label</span> | <span class="prop-type">node</span> | | The label element. |
| <span class="prop-name">value</span> | <span class="prop-type">any</span> | | You can provide your own value. Otherwise, we fallback to the child position index. |
Expand Down