From 42696bc401b0e2e3c26644c77addd3ae1ebfd63f Mon Sep 17 00:00:00 2001 From: Tarun047 <32017154+Tarun047@users.noreply.github.com> Date: Thu, 10 Oct 2019 21:44:53 +0530 Subject: [PATCH 1/4] Update Chip to reflect repl effect --- packages/material-ui/src/Chip/Chip.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js index 5a7d605d727e81..161d08c16bccc1 100644 --- a/packages/material-ui/src/Chip/Chip.js +++ b/packages/material-ui/src/Chip/Chip.js @@ -7,6 +7,7 @@ import { emphasize, fade } from '../styles/colorManipulator'; import useForkRef from '../utils/useForkRef'; import unsupportedProp from '../utils/unsupportedProp'; import capitalize from '../utils/capitalize'; +import ButtonBase from '../ButtonBase'; import '../Avatar'; // So we don't have any override priority issue. export const styles = theme => { @@ -272,7 +273,7 @@ const Chip = React.forwardRef(function Chip(props, ref) { className, clickable: clickableProp, color = 'default', - component: Component = 'div', + component: Component = ButtonBase, deleteIcon: deleteIconProp, disabled = false, icon: iconProp, From b1fc87b25c7d1ee2fdf1105f4548957ea38fbce1 Mon Sep 17 00:00:00 2001 From: Tarun047 <32017154+Tarun047@users.noreply.github.com> Date: Tue, 15 Oct 2019 10:02:09 +0530 Subject: [PATCH 2/4] Update Chip.js --- packages/material-ui/src/Chip/Chip.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js index 161d08c16bccc1..403eede9e0f4d0 100644 --- a/packages/material-ui/src/Chip/Chip.js +++ b/packages/material-ui/src/Chip/Chip.js @@ -273,15 +273,16 @@ const Chip = React.forwardRef(function Chip(props, ref) { className, clickable: clickableProp, color = 'default', - component: Component = ButtonBase, - deleteIcon: deleteIconProp, - disabled = false, - icon: iconProp, - label, onClick, onDelete, onKeyDown, onKeyUp, + component: Component = (onClick && !onDelete) ? ButtonBase : 'div', + deleteIcon: deleteIconProp, + disabled = false, + icon: iconProp, + label, + size = 'medium', variant = 'default', ...other From 6362fc2a9f924713d028ae5078f470f64181ff64 Mon Sep 17 00:00:00 2001 From: Tarun047 <32017154+Tarun047@users.noreply.github.com> Date: Tue, 15 Oct 2019 12:12:21 +0530 Subject: [PATCH 3/4] Add Repl Effect to Chip --- packages/material-ui/src/Chip/Chip.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js index 403eede9e0f4d0..2a5e77fdb295ab 100644 --- a/packages/material-ui/src/Chip/Chip.js +++ b/packages/material-ui/src/Chip/Chip.js @@ -7,7 +7,7 @@ import { emphasize, fade } from '../styles/colorManipulator'; import useForkRef from '../utils/useForkRef'; import unsupportedProp from '../utils/unsupportedProp'; import capitalize from '../utils/capitalize'; -import ButtonBase from '../ButtonBase'; +import ButtonBase from '../ButtonBase' import '../Avatar'; // So we don't have any override priority issue. export const styles = theme => { @@ -273,16 +273,15 @@ const Chip = React.forwardRef(function Chip(props, ref) { className, clickable: clickableProp, color = 'default', - onClick, - onDelete, - onKeyDown, - onKeyUp, - component: Component = (onClick && !onDelete) ? ButtonBase : 'div', + component: Component = ButtonBase, deleteIcon: deleteIconProp, disabled = false, icon: iconProp, label, - + onClick, + onDelete, + onKeyDown, + onKeyUp, size = 'medium', variant = 'default', ...other @@ -409,6 +408,7 @@ const Chip = React.forwardRef(function Chip(props, ref) { }, className, )} + component='div' tabIndex={clickable || onDelete ? 0 : undefined} onClick={onClick} onKeyDown={handleKeyDown} From 5e4abf3c6f15d632091289ac1cb765cf1c8c7963 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Thu, 17 Oct 2019 13:40:23 +0200 Subject: [PATCH 4/4] review --- docs/pages/api/chip.md | 2 +- packages/material-ui/src/Chip/Chip.js | 20 +++++++------------- packages/material-ui/src/Chip/Chip.test.js | 8 ++++---- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/docs/pages/api/chip.md b/docs/pages/api/chip.md index 665a07ae8dd48d..862557ce82bd69 100644 --- a/docs/pages/api/chip.md +++ b/docs/pages/api/chip.md @@ -29,7 +29,7 @@ Chips represent complex entities in small blocks, such as a contact. | classes | object | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. | | clickable | bool | | If true, the chip will appear clickable, and will raise when pressed, even if the onClick prop is not defined. If false, the chip will not be clickable, even if onClick prop is defined. This can be used, for example, along with the component prop to indicate an anchor Chip is clickable. | | color | 'default'
| 'primary'
| 'secondary' | 'default' | The color of the component. It supports those theme colors that make sense for this component. | -| component | elementType | 'div' | The component used for the root node. Either a string to use a DOM element or a component. | +| component | elementType | | The component used for the root node. Either a string to use a DOM element or a component. | | deleteIcon | element | | Override the default delete icon element. Shown only if `onDelete` is set. | | disabled | bool | false | If `true`, the chip should be displayed in a disabled state. | | icon | element | | Icon element. | diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js index 2a5e77fdb295ab..88e00badd7aa41 100644 --- a/packages/material-ui/src/Chip/Chip.js +++ b/packages/material-ui/src/Chip/Chip.js @@ -7,7 +7,7 @@ import { emphasize, fade } from '../styles/colorManipulator'; import useForkRef from '../utils/useForkRef'; import unsupportedProp from '../utils/unsupportedProp'; import capitalize from '../utils/capitalize'; -import ButtonBase from '../ButtonBase' +import ButtonBase from '../ButtonBase'; import '../Avatar'; // So we don't have any override priority issue. export const styles = theme => { @@ -68,7 +68,6 @@ export const styles = theme => { }, '&:active': { boxShadow: theme.shadows[1], - backgroundColor: emphasize(backgroundColor, 0.12), }, }, /* Styles applied to the root element if `onClick` and `color="primary"` is defined or `clickable={true}`. */ @@ -76,18 +75,12 @@ export const styles = theme => { '&:hover, &:focus': { backgroundColor: emphasize(theme.palette.primary.main, 0.08), }, - '&:active': { - backgroundColor: emphasize(theme.palette.primary.main, 0.12), - }, }, /* Styles applied to the root element if `onClick` and `color="secondary"` is defined or `clickable={true}`. */ clickableColorSecondary: { '&:hover, &:focus': { backgroundColor: emphasize(theme.palette.secondary.main, 0.08), }, - '&:active': { - backgroundColor: emphasize(theme.palette.secondary.main, 0.12), - }, }, /* Styles applied to the root element if `onDelete` is defined. */ deletable: { @@ -273,7 +266,7 @@ const Chip = React.forwardRef(function Chip(props, ref) { className, clickable: clickableProp, color = 'default', - component: Component = ButtonBase, + component: ComponentProp, deleteIcon: deleteIconProp, disabled = false, icon: iconProp, @@ -324,9 +317,7 @@ const Chip = React.forwardRef(function Chip(props, ref) { } const key = event.key; - if (onClick && (key === ' ' || key === 'Enter')) { - onClick(event); - } else if (onDelete && (key === 'Backspace' || key === 'Delete')) { + if (onDelete && (key === 'Backspace' || key === 'Delete')) { onDelete(event); } else if (key === 'Escape' && chipRef.current) { chipRef.current.blur(); @@ -336,6 +327,9 @@ const Chip = React.forwardRef(function Chip(props, ref) { const clickable = clickableProp !== false && onClick ? true : clickableProp; const small = size === 'small'; + const Component = ComponentProp || (clickable ? ButtonBase : 'div'); + const moreProps = Component === ButtonBase ? { component: 'div' } : {}; + let deleteIcon = null; if (onDelete) { const customClasses = clsx({ @@ -408,12 +402,12 @@ const Chip = React.forwardRef(function Chip(props, ref) { }, className, )} - component='div' tabIndex={clickable || onDelete ? 0 : undefined} onClick={onClick} onKeyDown={handleKeyDown} onKeyUp={handleKeyUp} ref={handleRef} + {...moreProps} {...other} > {avatar || icon} diff --git a/packages/material-ui/src/Chip/Chip.test.js b/packages/material-ui/src/Chip/Chip.test.js index 033e7bda588162..833f7f4e702169 100644 --- a/packages/material-ui/src/Chip/Chip.test.js +++ b/packages/material-ui/src/Chip/Chip.test.js @@ -423,8 +423,8 @@ describe('', () => { key: ' ', }; wrapper.find('div').simulate('keyDown', spaceKeyDown); - assert.strictEqual(preventDefaultSpy.callCount, 1); - assert.strictEqual(onClickSpy.callCount, 0); + assert.strictEqual(preventDefaultSpy.callCount, 2); + assert.strictEqual(onClickSpy.callCount, 1); const spaceKeyUp = { key: ' ', @@ -441,8 +441,8 @@ describe('', () => { key: 'Enter', }; wrapper.find('div').simulate('keyDown', enterKeyDown); - assert.strictEqual(preventDefaultSpy.callCount, 1); - assert.strictEqual(onClickSpy.callCount, 0); + assert.strictEqual(preventDefaultSpy.callCount, 2); + assert.strictEqual(onClickSpy.callCount, 1); const enterKeyUp = { key: 'Enter',