fix(chips): add chip leading icon class management (#281)

Author: Matt Goo

.travis.yml

@@ -37,4 +37,4 @@ matrix:
         - docker pull mdcreact/screenshots
         - docker image ls
       script:
-        - docker run -it --rm --cap-add=SYS_ADMIN -e MDC_GCLOUD_SERVICE_ACCOUNT_KEY="${MDC_GCLOUD_SERVICE_ACCOUNT_KEY}" mdcreact/screenshots /bin/sh -c "git fetch; git checkout \"${CURRENT_BRANCH}\"; git pull; npm i; /home/pptruser/material-components-web-react/test/screenshot/start.sh; sleep 30s; npm run test:image-diff"
+        - docker run -it --rm --cap-add=SYS_ADMIN -e MDC_GCLOUD_SERVICE_ACCOUNT_KEY="${MDC_GCLOUD_SERVICE_ACCOUNT_KEY}" mdcreact/screenshots /bin/sh -c "git checkout .; git fetch; git checkout \"${CURRENT_BRANCH}\"; git pull; npm i; /home/pptruser/material-components-web-react/test/screenshot/start.sh; sleep 30s; npm run test:image-diff"

packages/chips/Chip.js

@@ -31,6 +31,7 @@ export class Chip extends Component {
   foundation_ = null;
   state = {
     classList: new Set(),
+    leadingIconClassList: new Set(),
   };
 
   componentDidMount() {
@@ -79,6 +80,16 @@ export class Chip extends Component {
       setStyleProperty: (propertyName, value) => this.chipElement_.style.setProperty(propertyName, value),
       notifyRemoval: () => this.props.handleRemove(this.props.id),
       notifyInteraction: () => this.props.handleSelect(this.props.id),
+      addClassToLeadingIcon: (className) => {
+        const leadingIconClassList = new Set(this.state.leadingIconClassList);
+        leadingIconClassList.add(className);
+        this.setState({leadingIconClassList});
+      },
+      removeClassFromLeadingIcon: (className) => {
+        const leadingIconClassList = new Set(this.state.leadingIconClassList);
+        leadingIconClassList.delete(className);
+        this.setState({leadingIconClassList});
+      },
     };
   }
 
@@ -96,9 +107,13 @@ export class Chip extends Component {
 
   handleRemoveIconClick = (e) => this.foundation_.handleTrailingIconInteraction(e);
 
-  handleTransitionEnd = (e) => this.foundation_.handleTransitionEnd(e);
+  handleTransitionEnd = (e) => {
+    this.props.onTransitionEnd(e);
+    this.foundation_.handleTransitionEnd(e);
+  };
 
   renderLeadingIcon = (leadingIcon) => {
+    const {leadingIconClassList} = this.state;
     const {
       className,
       ...otherProps
@@ -107,6 +122,7 @@ export class Chip extends Component {
     const props = {
       className: classnames(
         className,
+        Array.from(leadingIconClassList),
         'mdc-chip__icon',
         'mdc-chip__icon--leading',
       ),
@@ -148,6 +164,7 @@ export class Chip extends Component {
       handleRemove,
       onClick,
       onKeyDown,
+      onTransitionEnd,
       computeBoundingRect,
       initRipple,
       unbounded,
@@ -187,6 +204,7 @@ Chip.propTypes = {
   handleRemove: PropTypes.func,
   onClick: PropTypes.func,
   onKeyDown: PropTypes.func,
+  onTransitionEnd: PropTypes.func,
   initRipple: PropTypes.func,
   unbounded: PropTypes.bool,
   chipCheckmark: PropTypes.node,
@@ -201,6 +219,7 @@ Chip.defaultProps = {
   selected: false,
   onClick: () => {},
   onKeyDown: () => {},
+  onTransitionEnd: () => {},
   initRipple: () => {},
   handleSelect: () => {},
   handleRemove: () => {},

packages/chips/ChipSet.js

@@ -42,7 +42,9 @@ export default class ChipSet extends Component {
 
   componentDidUpdate(prevProps) {
     if (this.props.selectedChipIds !== prevProps.selectedChipIds) {
-      this.updateChipSelection();
+      const selectedChipIds = new Set(this.props.selectedChipIds);
+      this.setState({selectedChipIds});
+      this.updateChipSelection(selectedChipIds);
     }
   }
 
@@ -74,10 +76,10 @@ export default class ChipSet extends Component {
     };
   }
 
-  updateChipSelection() {
+  updateChipSelection(ids = this.state.selectedChipIds) {
     React.Children.forEach(this.props.children, (child) => {
       const {id} = child.props;
-      if (this.props.selectedChipIds.indexOf(id) > -1) {
+      if (ids.has(id)) {
         this.foundation_.select(id);
       } else {
         // remove deselect when MDC Web issue 3612 is fixed
@@ -90,6 +92,7 @@ export default class ChipSet extends Component {
     const {handleSelect, choice, filter} = this.props;
     // update when mdc web issue is fix
     // https://github.com/material-components/material-components-web/issues/3613
+    // filter || choice is duplicate logic found in MDC Web foundation code
     if (filter || choice) {
       this.foundation_.toggleSelect(chipId);
     }

test/screenshot/chips/index.js

@@ -104,9 +104,14 @@ const sizes = ['Small', 'Medium', 'Large'];
 const clothes = ['Tops', 'Bottoms', 'Shoes'];
 const contacts = ['Jane Smith', 'John Doe'];
 
-const renderChips = (list) => {
+const renderChips = (list, hasLeadingIcon = false) => {
   return list.map((label, index) => (
-    <Chip id={`${index}chip`} key={index} label={label} />
+    <Chip
+      id={`${index}chip`}
+      key={index}
+      label={label}
+      leadingIcon={hasLeadingIcon ? <MaterialIcon icon='shopping_basket' /> : null}
+    />
   ));
 };
 
@@ -123,7 +128,12 @@ const ChipsScreenshotTest = () => {
         {renderChips(sizes)}
       </ChoiceChipsTest>
 
-      Filter Chips
+      Filter Chips with Leading Icon
+      <FilterChipsTest selectedChipIds={['1chip', '2chip']}>
+        {renderChips(clothes, true)}
+      </FilterChipsTest>
+
+      Filter Chips no Leading Icon
       <FilterChipsTest selectedChipIds={['1chip', '2chip']}>
         {renderChips(clothes)}
       </FilterChipsTest>

test/screenshot/golden.json

@@ -2,7 +2,7 @@
 
   "button": "a0cf6b2a5d09400657303bb2a7b220dbe33642a8f44fd7571cdfcf5517fb3586",
   "card": "b2fd82763c383be438ff6578083bf9009711c7470333d07eb916ab690fc42d31",
-  "chips": "9cdf56c10f4badd9555433e84a52d52028f286f42392f4902251dcb8eb748e33",
+  "chips": "89fb955abe09193af4e2b0f8cb9f0df06495508d2321fee247fa1a50cfe16a61",
   "line-ripple": "56b136db2dc7e09260849447e6bde9b55a837af332a05d9f52506ab1c95e2e57",
   "fab": "db36f52195c420062d91dd5ebe5432ad87247b3c1146fd547b0a195079bbce2f",
   "floating-label": "1d4d4f2e57e1769b14fc84985d1e6f53410c49aef41c9cf4fde94f938adefe57",
@@ -12,6 +12,7 @@
   "select": "ed9c021c8347f18e6ece48d55a8a3b30bb2380ec94276b002e9c064e7674e1c8",
   "switch": "dd8a3ec00447e0c586b5bbefdc633681d29e6f04ff8b517a68209bd1f4a6a4e4",
   "tab": "0e53fa0ca9b2de4ff7941169a9b6a929a83b18e517c18404adeb40f7e644a2f1",
+  "tab-bar": "b5cc3a623212f39be0a10001bb7f7e8d33f899ba44688e62b4ea449a8c849027",
   "tab-indicator": "7ce7ce8fd50301c67d7ebfb0ba953208260ce2881bee0c7e640c46bf60dc90b6",
   "tab-scroller": "468866dd0c222b36b55485ab44a5760133a4ddfb2a6cf81e6ae4672d7e02a447",
   "text-field": "3467c006063732dbc46fad4dbf3ea806b97772be3c13f798bc74475c4ec63b21",

test/unit/chips/Chip.test.js

@@ -77,6 +77,19 @@ test('#adapter.setStyleProperty should add styles to chip', () => {
   assert.equal(chipElement.style.width, width);
 });
 
+test('#adapter.addClassToLeadingIcon adds to state.leadingIconClassList', () => {
+  const wrapper = shallow(<Chip id='123' />);
+  wrapper.instance().foundation_.adapter_.addClassToLeadingIcon('test-leading-icon-class');
+  assert.isTrue(wrapper.state().leadingIconClassList.has('test-leading-icon-class'));
+});
+
+test('#adapter.removeClassFromLeadingIcon removes from state.leadingIconClassList', () => {
+  const wrapper = shallow(<Chip id='123' />);
+  wrapper.setState({leadingIconClassList: new Set('test-leading-icon-class')});
+  wrapper.instance().foundation_.adapter_.removeClassFromLeadingIcon('test-leading-icon-class');
+  assert.isFalse(wrapper.state().leadingIconClassList.has('test-leading-icon-class'));
+});
+
 test('#adapter.notifyInteraction calls #props.handleSelect w/ chipId', () => {
   const handleSelect = td.func();
   const wrapper = shallow(<Chip id='123' handleSelect={handleSelect} />);
@@ -142,6 +155,13 @@ test('renders leading icon with base class names', () => {
   assert.isTrue(wrapper.children().first().hasClass('mdc-chip__icon--leading'));
 });
 
+test('renders leadingIcon with state.leadingIconClassList', () => {
+  const leadingIcon = <i className='leading-icon'></i>;
+  const wrapper = shallow(<Chip id='1' leadingIcon={leadingIcon} />);
+  wrapper.setState({leadingIconClassList: new Set(['test-leading-icon-class'])});
+  assert.isTrue(wrapper.children().first().hasClass('test-leading-icon-class'));
+});
+
 test('renders remove icon', () => {
   const removeIcon = <i className='remove-icon'></i>;
   const wrapper = shallow(<Chip id='1' removeIcon={removeIcon} />);
@@ -182,11 +202,20 @@ test('remove icon keydown calls #foundation.handleTrailingIconInteraction', () =
 test('calls #foundation.handleTransitionEnd on transitionend event', () => {
   const wrapper = shallow(<Chip id='1' />);
   wrapper.instance().foundation_.handleTransitionEnd = td.func();
-  const evt = {};
+  const evt = {target: {}};
   wrapper.simulate('transitionend', evt);
   td.verify(wrapper.instance().foundation_.handleTransitionEnd(evt), {times: 1});
 });
 
+
+test('calls #props.onTransitionEnd on transitionend event', () => {
+  const onTransitionEnd = td.func();
+  const wrapper = shallow(<Chip id='1' onTransitionEnd={onTransitionEnd} />);
+  const evt = {target: {classList: {contains: () => {}}}};
+  wrapper.simulate('transitionend', evt);
+  td.verify(onTransitionEnd(evt), {times: 1});
+});
+
 test('renders chip checkmark if it exists', () => {
   const wrapper = mount(<Chip id='1' chipCheckmark={<ChipCheckmark/>} />);
   assert.equal(wrapper.find('.mdc-chip__checkmark').length, 1);

test/unit/chips/ChipSet.test.js

@@ -13,12 +13,11 @@ test('creates foundation', () => {
   assert.exists(wrapper.instance().foundation_);
 });
 
-test('calls #foundation.select when the selectedChipIds change', () => {
+test('updates state.selectedChipIds when the props.selectedChipIds change', () => {
   const wrapper = shallow(<ChipSet><div id='1' /></ChipSet>);
-  wrapper.instance().updateChipSelection = td.func();
   const selectedChipIds = ['1'];
   wrapper.setProps({selectedChipIds});
-  td.verify(wrapper.instance().updateChipSelection(), {times: 1});
+  assert.isTrue(wrapper.state().selectedChipIds.has('1'));
 });
 
 test('filter classname is added if is filter variant', () => {
@@ -66,38 +65,24 @@ test('#adapter.setSelected removes selectedChipId from state', () => {
   assert.isFalse(wrapper.state().selectedChipIds.has('1'));
 });
 
-test('#foundation.select is called when #updateChipSelection is called', () => {
-  const wrapper = shallow(<ChipSet selectedChipIds={['1']}><div id='1' /></ChipSet>);
+test('#foundation.select is called when #updateChipSelection is called and ' +
+  'state.selectedChipIds has a selected Id', () => {
+  const wrapper = shallow(<ChipSet><div id='1' /></ChipSet>);
   wrapper.instance().foundation_.select = td.func();
+  const selectedChipIds = new Set(['1']);
+  wrapper.setState({selectedChipIds});
   wrapper.instance().updateChipSelection();
   td.verify(wrapper.instance().foundation_.select('1'), {times: 1});
 });
 
-test('#foundation.deselect is called when #updateChipSelection is called', () => {
+test('#foundation.deselect is called when #updateChipSelection is called and ' +
+  'state.selectedChipIds does not have selected Id', () => {
   const wrapper = shallow(<ChipSet><div id='1' /></ChipSet>);
   wrapper.instance().foundation_.deselect = td.func();
   wrapper.instance().updateChipSelection();
   td.verify(wrapper.instance().foundation_.deselect('1'), {times: 1});
 });
 
-test('#foundation.select is called when the selectedChipIds change', () => {
-  const wrapper = shallow(<ChipSet><div id='1' /></ChipSet>);
-  wrapper.instance().foundation_.select = td.func();
-  const selectedChipIds = ['1'];
-  wrapper.setProps({selectedChipIds});
-  td.verify(wrapper.instance().foundation_.select('1'), {times: 1});
-});
-
-test('#foundation.deselect is called when the selectedChipIds change', () => {
-  const wrapper = shallow(<ChipSet><div id='1' /></ChipSet>);
-  const selectedChipIds = ['1'];
-  wrapper.setProps({selectedChipIds});
-  wrapper.instance().foundation_.deselect = td.func();
-  wrapper.setProps({selectedChipIds: []});
-
-  td.verify(wrapper.instance().foundation_.deselect('1'), {times: 1});
-});
-
 test('#handleSelect calls props.handleSelect', () => {
   const handleSelect = td.func();
   const wrapper = shallow(<ChipSet handleSelect={handleSelect}></ChipSet>);