Skip to content

Commit

Permalink
feat(chips): implements missing adapter method notifyTrailingIconInte…
Browse files Browse the repository at this point in the history
…raction (#653)

BREAKING CHANGE: renamed chip.props.removeIcon --> chips.props.trailingIcon
  • Loading branch information
mgr34 authored and Matt Goo committed Feb 19, 2019
1 parent e0b0946 commit c5e87a6
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 46 deletions.
43 changes: 30 additions & 13 deletions packages/chips/Chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ export interface ChipProps extends Ripple.InjectedProps<HTMLDivElement> {
handleSelect?: (id: string, selected: boolean) => void;
handleRemove?: (id: string) => void;
handleInteraction?: (id: string) => void;
handleTrailingIconInteraction?: (id: string) => void;
onClick?: React.MouseEventHandler<HTMLDivElement>;
onKeyDown?: React.KeyboardEventHandler<HTMLDivElement>;
onTransitionEnd?: React.TransitionEventHandler<HTMLDivElement>;
chipCheckmark?: React.ReactElement<HTMLElement>;
leadingIcon?: React.ReactElement<HTMLElement>;
removeIcon?: React.ReactElement<HTMLElement>;
shouldRemoveOnTrailingIconClick?: boolean;
trailingIcon?: React.ReactElement<HTMLElement>;
initRipple: (surface: HTMLElement | null) => void;
};

Expand All @@ -63,6 +65,8 @@ export class Chip extends React.Component<ChipProps, ChipState> {
handleSelect: () => {},
handleRemove: () => {},
handleInteraction: () => {},
handleTrailingIconInteraction: () => {},
shouldRemoveOnTrailingIconClick: true,
};

state = {
Expand All @@ -71,14 +75,24 @@ export class Chip extends React.Component<ChipProps, ChipState> {
};

componentDidMount() {
const {selected, shouldRemoveOnTrailingIconClick} = this.props;
this.foundation = new MDCChipFoundation(this.adapter);
this.foundation.init();
this.foundation.setSelected(this.props.selected);
this.foundation.setSelected(selected);
if (shouldRemoveOnTrailingIconClick !== this.foundation.getShouldRemoveOnTrailingIconClick()) {
this.foundation.setShouldRemoveOnTrailingIconClick(shouldRemoveOnTrailingIconClick);
}
}

componentDidUpdate(prevProps: ChipProps) {
if (this.props.selected !== prevProps.selected) {
this.foundation.setSelected(this.props.selected);
const {selected, shouldRemoveOnTrailingIconClick} = this.props;

if (selected !== prevProps.selected) {
this.foundation.setSelected(selected);
}

if (shouldRemoveOnTrailingIconClick !== prevProps.shouldRemoveOnTrailingIconClick) {
this.foundation.setShouldRemoveOnTrailingIconClick(shouldRemoveOnTrailingIconClick);
}
}

Expand Down Expand Up @@ -126,6 +140,7 @@ export class Chip extends React.Component<ChipProps, ChipState> {
notifyInteraction: () => this.props.handleInteraction!(this.props.id!),
notifySelection: (selected: boolean) =>
this.props.handleSelect!(this.props.id!, selected),
notifyTrailingIconInteraction: () => this.props.handleTrailingIconInteraction!(this.props.id!),
addClassToLeadingIcon: (className: string) => {
const leadingIconClassList = new Set(this.state.leadingIconClassList);
leadingIconClassList.add(className);
Expand All @@ -149,7 +164,7 @@ export class Chip extends React.Component<ChipProps, ChipState> {
this.foundation.handleInteraction(e);
};

handleRemoveIconClick = (e: React.MouseEvent) => this.foundation.handleTrailingIconInteraction(e);
handleTrailingIconClick = (e: React.MouseEvent) => this.foundation.handleTrailingIconInteraction(e);

handleTransitionEnd = (e: React.TransitionEvent<HTMLDivElement>) => {
this.props.onTransitionEnd!(e);
Expand All @@ -171,21 +186,21 @@ export class Chip extends React.Component<ChipProps, ChipState> {
return React.cloneElement(leadingIcon, props);
};

renderRemoveIcon = (removeIcon: React.ReactElement<HTMLElement>) => {
const {className, ...otherProps} = removeIcon.props;
renderTrailingIcon = (trailingIcon: React.ReactElement<HTMLElement>) => {
const {className, ...otherProps} = trailingIcon.props;
const props = {
className: classnames(
className,
'mdc-chip__icon',
'mdc-chip__icon--trailing'
),
onClick: this.handleRemoveIconClick,
onKeyDown: this.handleRemoveIconClick,
onClick: this.handleTrailingIconClick,
onKeyDown: this.handleTrailingIconClick,
tabIndex: 0,
role: 'button',
...otherProps,
};
return React.cloneElement(removeIcon, props);
return React.cloneElement(trailingIcon, props);
};

render() {
Expand All @@ -197,16 +212,18 @@ export class Chip extends React.Component<ChipProps, ChipState> {
handleSelect,
handleInteraction,
handleRemove,
handleTrailingIconInteraction,
onClick,
onKeyDown,
onTransitionEnd,
computeBoundingRect,
initRipple,
unbounded,
shouldRemoveOnTrailingIconClick,
/* eslint-enable no-unused-vars */
chipCheckmark,
leadingIcon,
removeIcon,
trailingIcon,
label,
...otherProps
} = this.props;
Expand All @@ -220,10 +237,10 @@ export class Chip extends React.Component<ChipProps, ChipState> {
ref={this.init}
{...otherProps}
>
{leadingIcon ? this.renderLeadingIcon(leadingIcon) : null}
{leadingIcon && this.renderLeadingIcon(leadingIcon)}
{chipCheckmark}
<div className='mdc-chip__text'>{label}</div>
{removeIcon ? this.renderRemoveIcon(removeIcon) : null}
{trailingIcon && this.renderTrailingIcon(trailingIcon)}
</div>
);
}
Expand Down
11 changes: 7 additions & 4 deletions packages/chips/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class MyInputChips extends React.Component {
<Chip
key={chip.id} // The chip's key cannot be its index, because its index may change.
label={chip.label}
removeIcon={<MaterialIcon icon='cancel' />}
trailingIcon={<MaterialIcon icon='cancel' />}
/>
)}
</ChipSet>
Expand Down Expand Up @@ -170,12 +170,15 @@ className | String | Classes to be applied to the chip element
id | Number | Required. Unique identifier for the chip
label | String | Text to be shown on the chip
leadingIcon | Element | An icon element that appears as the leading icon.
removeIcon | Element | An icon element that appears as the remove icon. Clicking on it should remove the chip.
trailingIcon | Element | An icon element that appears as the remove icon. Clicking on it should remove the chip.
selected | Boolean | Indicates whether the chip is selected
handleSelect | Function(id: string, selected: boolean) => void | Callback for selecting the chip with the given id
handleRemove | Function(id: string) => void | Callback for removing the chip with the given id
handleInteraction | Function(id: string) => void | Callback for interaction of chip (`onClick` | `onKeyDown`)

handleTrailingIconInteraction | Function(id: string) => void | Callback for interaction with trailing icon
shouldRemoveOnTrailingIconClick | Boolean | indicates if interaction with trailing icon should remove chip. defaults to `true`
> Note: `handleTrailingIconInteraction` will execute before `handleRemove`.
> Without explicitly setting shouldRemoveOnTrailingIconClick to false both
> callbacks will fire on trailingIcon interaction
## Sass Mixins

Expand Down
13 changes: 7 additions & 6 deletions test/screenshot/chips/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ type InputChipsTestState = {

class InputChipsTest extends React.Component<InputChipsTestProps, InputChipsTestState> {
state = {
chips: this.props.labels.map((label) => {
return {label: label, id: uuidv1()};
}),
chips: this.props.labels.reduce((a, label) => (
[...a, {label, id: uuidv1()}]
), [{label: 'Name Chips (dont remove)', id: uuidv1()}] ),
};

addChip(label: string) {
Expand All @@ -90,13 +90,14 @@ class InputChipsTest extends React.Component<InputChipsTestProps, InputChipsTest
<div>
<input type='text' onKeyDown={this.handleKeyDown} />
<ChipSet input updateChips={this.updateChips}>
{this.state.chips.map((chip) => (
{this.state.chips.map((chip, i: number ) => (
<Chip
id={chip.id}
key={chip.id} // The chip s key cannot be its index, because its index may change
label={chip.label}
leadingIcon={<MaterialIcon icon='face' />}
removeIcon={<MaterialIcon icon='cancel' />}
leadingIcon={i === 0 ? undefined : <MaterialIcon icon='face' />}
trailingIcon={<MaterialIcon icon={i === 0 ? 'announcement' : 'cancel'} />}
shouldRemoveOnTrailingIconClick={i !== 0}
/>
))}
</ChipSet>
Expand Down
2 changes: 1 addition & 1 deletion test/screenshot/golden.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"button": "57cb8769600669ec4d44dd23fcf2f6ded528ce8eec612d832c2b9e0271a640c3",
"card": "b2fd82763c383be438ff6578083bf9009711c7470333d07eb916ab690fc42d31",
"checkbox": "9c61177f0f927e178e7c6687d74cdfa08abc15ea8fc3c381f570b7c7d1f46d2a",
"chips": "e100a23df0b92c37920127c62d7d694ce3fe40c101c0ed05d535f5cafee62b27",
"chips": "f5973cc5f1961464cbbbe152ca25b9a989e7e5a54b6d64cb28f0c25788f7df44",
"fab": "db36f52195c420062d91dd5ebe5432ad87247b3c1146fd547b0a195079bbce2f",
"floating-label": "1d4d4f2e57e1769b14fc84985d1e6f53410c49aef41c9cf4fde94f938adefe57",
"icon-button": "5ffb1f7fbd06d2c0533f6ba8d4d9ea170cec1a248a61de1cc1bb626cb58ebcd2",
Expand Down
76 changes: 61 additions & 15 deletions test/unit/chips/Chip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ test('creates foundation', () => {
assert.exists(wrapper.instance().foundation);
});

test('#componentWillUnmount destroys foundation', () => {
const wrapper = shallow<Chip>(<Chip id='2' />);
const foundation = wrapper.instance().foundation;
foundation.destroy = td.func();
wrapper.unmount();
td.verify(foundation.destroy(), {times: 1});
});

test('calls setSelected if props.selected is true (#foundation.setSelected)', () => {
const wrapper = mount<Chip>(
<Chip id='1' selected>
Expand All @@ -22,6 +30,36 @@ test('calls setSelected if props.selected is true (#foundation.setSelected)', ()
assert.isTrue(wrapper.state().classList.has('mdc-chip--selected'));
});


test('renders a Chip with foundation.shouldRemoveOnTrailingIconClick set to true', () => {
const wrapper = shallow<Chip>(<Chip id='3' />);
assert.isTrue(wrapper.instance().foundation.getShouldRemoveOnTrailingIconClick());
});

test('#componentDidMount sets #foundation.shouldRemoveOnTrailingIconClick to false if prop false', () => {
const wrapper = shallow<Chip>(<Chip id='4' shouldRemoveOnTrailingIconClick={false} />);
assert.isFalse(wrapper.instance().foundation.getShouldRemoveOnTrailingIconClick());
});

test('when props.shouldRemoveOnTrailingIconClick updates to false, ' +
' #foundation.setShouldRemoveOnTrailingIconClick is called ', () => {
const wrapper = shallow<Chip>(<Chip id='5' shouldRemoveOnTrailingIconClick />);
assert.isTrue(wrapper.instance().foundation.getShouldRemoveOnTrailingIconClick());
wrapper.instance().foundation.setShouldRemoveOnTrailingIconClick = td.func();
wrapper.setProps({shouldRemoveOnTrailingIconClick: false});
td.verify(wrapper.instance().foundation.setShouldRemoveOnTrailingIconClick(false), {times: 1});
});


test('when props.shouldRemoveOnTrailingIconClick updates to true, ' +
' #foundation.setShouldRemoveOnTrailingIconClick is called ', () => {
const wrapper = shallow<Chip>(<Chip id='6' shouldRemoveOnTrailingIconClick={false} />);
assert.isFalse(wrapper.instance().foundation.getShouldRemoveOnTrailingIconClick());
wrapper.instance().foundation.setShouldRemoveOnTrailingIconClick = td.func();
wrapper.setProps({shouldRemoveOnTrailingIconClick: true});
td.verify(wrapper.instance().foundation.setShouldRemoveOnTrailingIconClick(true), {times: 1});
});

test('classNames adds classes', () => {
const wrapper = shallow(<Chip id='1' className='test-class-name' />);
assert.isTrue(wrapper.hasClass('test-class-name'));
Expand Down Expand Up @@ -139,6 +177,13 @@ test('#adapter.notifySelection calls #props.handleSelect w/ chipId and selected
td.verify(handleSelect('123', true), {times: 1});
});

test('#adapter.notifyTrailingIconInteraction calls #props.handleTrailingIconInteraction w/ chipId', () => {
const handleTrailingIconInteraction = coerceForTesting<(id: string) => void>(td.func());
const wrapper = shallow<Chip>(<Chip id='123' handleTrailingIconInteraction={handleTrailingIconInteraction} />);
wrapper.instance().foundation.adapter_.notifyTrailingIconInteraction();
td.verify(handleTrailingIconInteraction('123'), {times: 1});
});

test('on click calls #props.onClick', () => {
const onClick = coerceForTesting<(event: React.MouseEvent) => void>(td.func());
const wrapper = shallow<Chip>(<Chip id='1' onClick={onClick} />);
Expand Down Expand Up @@ -229,9 +274,9 @@ test('renders leadingIcon with state.leadingIconClassList', () => {
);
});

test('renders remove icon', () => {
const removeIcon = <i className='remove-icon' />;
const wrapper = shallow(<Chip id='1' removeIcon={removeIcon} />);
test('renders trailing icon', () => {
const trailingIcon = <i className='remove-icon' />;
const wrapper = shallow(<Chip id='1' trailingIcon={trailingIcon} />);
assert.equal(
wrapper
.children()
Expand All @@ -241,9 +286,9 @@ test('renders remove icon', () => {
);
});

test('renders remove icon with class name', () => {
const removeIcon = <i className='remove-icon' />;
const wrapper = shallow(<Chip id='1' removeIcon={removeIcon} />);
test('renders trailing icon with class name', () => {
const trailingIcon = <i className='remove-icon' />;
const wrapper = shallow(<Chip id='1' trailingIcon={trailingIcon} />);
assert.isTrue(
wrapper
.children()
Expand All @@ -252,9 +297,9 @@ test('renders remove icon with class name', () => {
);
});

test('renders remove icon with base class names', () => {
const removeIcon = <i className='remove-icon' />;
const wrapper = shallow(<Chip id='1' removeIcon={removeIcon} />);
test('renders trailing icon with base class names', () => {
const trailingIcon = <i className='remove-icon' />;
const wrapper = shallow(<Chip id='1' trailingIcon={trailingIcon} />);
assert.isTrue(
wrapper
.children()
Expand All @@ -269,9 +314,9 @@ test('renders remove icon with base class names', () => {
);
});

test('remove icon click calls #foundation.handleTrailingIconInteraction', () => {
const removeIcon = <i className='remove-icon' />;
const wrapper = shallow<Chip>(<Chip id='1' removeIcon={removeIcon} />);
test('trailing icon click calls #foundation.handleTrailingIconInteraction', () => {
const trailingIcon = <i className='remove-icon' />;
const wrapper = shallow<Chip>(<Chip id='1' trailingIcon={trailingIcon} />);
wrapper.instance().foundation.handleTrailingIconInteraction = td.func();
const evt = {};
wrapper
Expand All @@ -283,9 +328,9 @@ test('remove icon click calls #foundation.handleTrailingIconInteraction', () =>
});
});

test('remove icon keydown calls #foundation.handleTrailingIconInteraction', () => {
const removeIcon = <i className='remove-icon' />;
const wrapper = shallow<Chip>(<Chip id='1' removeIcon={removeIcon} />);
test('trailing icon keydown calls #foundation.handleTrailingIconInteraction', () => {
const trailingIcon = <i className='remove-icon' />;
const wrapper = shallow<Chip>(<Chip id='1' trailingIcon={trailingIcon} />);
wrapper.instance().foundation.handleTrailingIconInteraction = td.func();
const evt = {};
wrapper
Expand All @@ -297,6 +342,7 @@ test('remove icon keydown calls #foundation.handleTrailingIconInteraction', () =
});
});


test('calls #foundation.handleTransitionEnd on transitionend event', () => {
const wrapper = shallow<Chip>(<Chip id='1' />);
wrapper.instance().foundation.handleTransitionEnd = td.func();
Expand Down
14 changes: 7 additions & 7 deletions test/unit/chips/ChipSet.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ suite('ChipSet');


test('creates foundation', () => {
const wrapper = mount<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = mount<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
assert.exists(wrapper.state().foundation);
});

Expand Down Expand Up @@ -277,13 +277,13 @@ test('#removeChip calls #props.updateChips with array of remaining chips', () =>
});

test('#setCheckmarkWidth sets checkmark width', () => {
const wrapper = shallow<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = shallow<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
wrapper.instance().setCheckmarkWidth(coerceForTesting<ChipCheckmark>({width: 20}));
assert.equal(wrapper.instance().checkmarkWidth, 20);
});

test('#setCheckmarkWidth does not set checkmark width if checkmark width is already set', () => {
const wrapper = shallow<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = shallow<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
wrapper.instance().checkmarkWidth = 20;
wrapper.instance().setCheckmarkWidth(coerceForTesting<ChipCheckmark>({width: 40}));
assert.equal(wrapper.instance().checkmarkWidth, 20);
Expand All @@ -296,7 +296,7 @@ test('#setCheckmarkWidth does not set checkmark width if checkmark is null', ()
});

test('#computeBoundingRect returns width and height', () => {
const wrapper = shallow<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = shallow<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
const chipWidth = 20;
const chipHeight = 50;
const chipElement = coerceForTesting<HTMLDivElement>({
Expand All @@ -308,7 +308,7 @@ test('#computeBoundingRect returns width and height', () => {
});

test('#computeBoundingRect returns width and height', () => {
const wrapper = shallow<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = shallow<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
const chipWidth = 20;
const chipHeight = 50;
wrapper.instance().checkmarkWidth = 20;
Expand Down Expand Up @@ -355,7 +355,7 @@ test('#chip.props.handleSelect calls #foundation.handleChipSelection', () => {
});

test('chip is rendered with handleRemove method', () => {
const wrapper = mount<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = mount<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
wrapper.instance().handleRemove = coerceForTesting<(chipId: string) => void>(td.func());
wrapper.setProps({children: <Chip id='1' />});
const chip = wrapper.children().props().children[0];
Expand Down Expand Up @@ -421,7 +421,7 @@ test('chip is rendered with computeBoundingRect method prop if is not filter var
});

test('#componentWillUnmount destroys foundation', () => {
const wrapper = shallow<ChipSet>(<ChipSet><Chip /></ChipSet>);
const wrapper = shallow<ChipSet>(<ChipSet><Chip id='1' /></ChipSet>);
const foundation = wrapper.state().foundation;
foundation.destroy = td.func();
wrapper.unmount();
Expand Down

0 comments on commit c5e87a6

Please sign in to comment.