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.

Sign up for GitHub

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

Jump to bottom

[Autocomplete] Fix Multiple tag delete action #18153

Merged
merged 2 commits into from
Nov 9, 2019
Merged

[Autocomplete] Fix Multiple tag delete action #18153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1b22df
[Docs] pass item index to getTagProps
Nov 2, 2019
5e60be1
add failing test case
oliviertassinari Nov 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/pages/api/autocomplete.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi
| <span class="prop-name">renderGroup</span> | <span class="prop-type">func</span> | | Render the group.<br><br>**Signature:**<br>`function(option: any) => ReactNode`<br>*option:* The group to render. |
| <span class="prop-name required">renderInput&nbsp;*</span> | <span class="prop-type">func</span> | | Render the input.<br><br>**Signature:**<br>`function(params: object) => ReactNode`<br>*params:* null |
| <span class="prop-name">renderOption</span> | <span class="prop-type">func</span> | | Render the option, use `getOptionLabel` by default.<br><br>**Signature:**<br>`function(option: any, state: object) => ReactNode`<br>*option:* The option to render.<br>*state:* The state of the component. |
| <span class="prop-name">renderTags</span> | <span class="prop-type">func</span> | | Render the selected value.<br><br>**Signature:**<br>`function(value: any) => ReactNode`<br>*value:* The `value` provided to the component. |
| <span class="prop-name">renderTags</span> | <span class="prop-type">func</span> | | Render the selected value.<br><br>**Signature:**<br>`function(value: any, getTagProps: function) => ReactNode`<br>*value:* The `value` provided to the component.<br>*getTagProps:* A tag props getter. |
| <span class="prop-name">value</span> | <span class="prop-type">any</span> | | The value of the autocomplete. |

The `ref` is forwarded to the root element.
Expand Down
8 changes: 1 addition & 7 deletions docs/src/pages/components/autocomplete/CustomizedHook.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,7 @@ export default function CustomizedHook() {
<Label {...getInputLabelProps()}>Customized hook</Label>
<InputWrapper ref={setAnchorEl} className={focused ? 'focused' : ''}>
{value.map((option, index) => (
<Tag
key={index}
data-tag-index={index}
tabIndex={-1}
label={option.title}
{...getTagProps()}
/>
<Tag label={option.title} {...getTagProps({ index })} />
))}

<input {...getInputProps()} />
Expand Down
8 changes: 1 addition & 7 deletions docs/src/pages/components/autocomplete/CustomizedHook.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,7 @@ export default function CustomizedHook() {
<Label {...getInputLabelProps()}>Customized hook</Label>
<InputWrapper ref={setAnchorEl} className={focused ? 'focused' : ''}>
{value.map((option: FilmOptionType, index: number) => (
<Tag
key={index}
data-tag-index={index}
tabIndex={-1}
label={option.title}
{...getTagProps()}
/>
<Tag label={option.title} {...getTagProps({ index })} />
))}
<input {...getInputProps()} />
</InputWrapper>
Expand Down
12 changes: 2 additions & 10 deletions docs/src/pages/components/autocomplete/FixedTags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,9 @@ export default function FixedTags() {
options={top100Films}
getOptionLabel={option => option.title}
defaultValue={[top100Films[6], top100Films[13]]}
renderTags={(value, { className, onDelete }) =>
renderTags={(value, getTagProps) =>
value.map((option, index) => (
<Chip
key={index}
disabled={index === 0}
data-tag-index={index}
tabIndex={-1}
label={option.title}
className={className}
onDelete={onDelete}
/>
<Chip disabled={index === 0} label={option.title} {...getTagProps({ index })} />
))
}
style={{ width: 500 }}
Expand Down
12 changes: 2 additions & 10 deletions docs/src/pages/components/autocomplete/FixedTags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,9 @@ export default function FixedTags() {
options={top100Films}
getOptionLabel={(option: FilmOptionType) => option.title}
defaultValue={[top100Films[6], top100Films[13]]}
renderTags={(value: FilmOptionType[], { className, onDelete }) =>
renderTags={(value: FilmOptionType[], getTagProps) =>
value.map((option: FilmOptionType, index: number) => (
<Chip
key={index}
disabled={index === 0}
data-tag-index={index}
tabIndex={-1}
label={option.title}
className={className}
onDelete={onDelete}
/>
<Chip disabled={index === 0} label={option.title} {...getTagProps({ index })} />
))
}
style={{ width: 500 }}
Expand Down
12 changes: 2 additions & 10 deletions docs/src/pages/components/autocomplete/Tags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,9 @@ export default function Tags() {
options={top100Films.map(option => option.title)}
defaultValue={[top100Films[13].title]}
freeSolo
renderTags={(value, { className, onDelete }) =>
renderTags={(value, getTagProps) =>
value.map((option, index) => (
<Chip
key={index}
variant="outlined"
data-tag-index={index}
tabIndex={-1}
label={option}
className={className}
onDelete={onDelete}
/>
<Chip variant="outlined" label={option} {...getTagProps({ index })} />
))
}
renderInput={params => (
Expand Down
12 changes: 2 additions & 10 deletions docs/src/pages/components/autocomplete/Tags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,9 @@ export default function Tags() {
options={top100Films.map(option => option.title)}
defaultValue={[top100Films[13].title]}
freeSolo
renderTags={(value: string[], { className, onDelete }) =>
renderTags={(value: string[], getTagProps) =>
value.map((option: string, index: number) => (
<Chip
key={index}
variant="outlined"
data-tag-index={index}
tabIndex={-1}
label={option}
className={className}
onDelete={onDelete}
/>
<Chip variant="outlined" label={option} {...getTagProps({ index })} />
))
}
renderInput={params => (
Expand Down
8 changes: 3 additions & 5 deletions packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ export interface RenderOptionState {
selected: boolean;
}

export interface RenderValueState {
className: string;
onDelete: () => {};
}
export type GetTagProps = ({ index }: { index: number }) => {};

export interface RenderGroupParams {
key: string;
Expand Down Expand Up @@ -103,9 +100,10 @@ export interface AutocompleteProps
* Render the selected value.
*
* @param {any} value The `value` provided to the component.
* @param {function} getTagProps A tag props getter.
* @returns {ReactNode}
*/
renderTags?: (value: any, state: RenderValueState) => React.ReactNode;
renderTags?: (value: any, getTagProps: GetTagProps) => React.ReactNode;
}

export type AutocompleteClassKey =
Expand Down
17 changes: 6 additions & 11 deletions packages/material-ui-lab/src/Autocomplete/Autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,22 +232,16 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
let startAdornment;

if (multiple && value.length > 0) {
const tagProps = {
...getTagProps(),
const getCustomizedTagProps = params => ({
className: classes.tag,
};
...getTagProps(params),
});

if (renderTags) {
startAdornment = renderTags(value, tagProps);
startAdornment = renderTags(value, getCustomizedTagProps);
} else {
startAdornment = value.map((option, index) => (
<Chip
key={index}
data-tag-index={index}
tabIndex={-1}
label={getOptionLabel(option)}
{...tagProps}
/>
<Chip label={getOptionLabel(option)} {...getCustomizedTagProps({ index })} />
));
}
}
Expand Down Expand Up @@ -570,6 +564,7 @@ Autocomplete.propTypes = {
* Render the selected value.
*
* @param {any} value The `value` provided to the component.
* @param {function} getTagProps A tag props getter.
* @returns {ReactNode}
*/
renderTags: PropTypes.func,
Expand Down
17 changes: 17 additions & 0 deletions packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ describe('<Autocomplete />', () => {
document.activeElement.blur();
input.focus();
});

it('should remove the last option', () => {
const handleChange = spy();
const options = ['one', 'two'];
const { container } = render(
<Autocomplete
defaultValue={options}
options={options}
onChange={handleChange}
renderInput={params => <TextField {...params} />}
multiple
/>,
);
fireEvent.click(container.querySelectorAll('svg[data-mui-test="CancelIcon"]')[1]);
Copy link
Member

Choose a reason for hiding this comment

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

@eps1lon any better idea for this selector?

expect(handleChange.callCount).to.equal(1);
expect(handleChange.args[0][1]).to.deep.equal([options[0]]);
});
});

describe('WAI-ARIA conforming markup', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export default function useAutocomplete(
getInputLabelProps: () => {};
getClearProps: () => {};
getPopupIndicatorProps: () => {};
getTagProps: () => {};
getTagProps: ({ index }: { index: number }) => {};
getListboxProps: () => {};
getOptionProps: ({ option, index }: { option: any; index: number }) => {};
id: string;
Expand Down
12 changes: 7 additions & 5 deletions packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,7 @@ export default function useAutocomplete(props) {
selectNewValue(event, filteredOptions[highlightedIndexRef.current]);
};

const handleTagDelete = event => {
const index = Number(event.currentTarget.getAttribute('data-tag-index'));
const handleTagDelete = index => event => {
const newValue = [...value];
newValue.splice(index, 1);
Copy link

Choose a reason for hiding this comment

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

@oliviertassinari Just wanted to know, why are we using splice here. For example i am having 3 values selected [1,2,3] and trying to delete first element in the array, in this case we will be passing index as 0 to renderTagProps. So if you splice array with 0,1 (newValue.splice(0,1)) will return first element in the array [1].

We are facing issue like if we try to delete any element in our selected value list, it always removes last element instead of actual element which we try to delete.

image

handleValue(event, newValue);
Expand Down Expand Up @@ -699,8 +698,11 @@ export default function useAutocomplete(props) {
event.preventDefault();
},
}),
getTagProps: () => ({
onDelete: handleTagDelete,
getTagProps: ({ index }) => ({
key: index,
'data-tag-index': index,
tabIndex: -1,
onDelete: handleTagDelete(index),
}),
getListboxProps: () => ({
role: 'listbox',
Expand All @@ -716,9 +718,9 @@ export default function useAutocomplete(props) {
const disabled = getOptionDisabled ? getOptionDisabled(option) : false;

return {
key: index,
tabIndex: -1,
role: 'option',
key: index,
id: `${id}-option-${index}`,
onMouseOver: handleOptionMouseOver,
onClick: handleOptionClick,
Expand Down