[Autocomplete] Fix Multiple tag delete action #18153

tkanzakic · 2019-11-02T17:33:26Z

I have followed (at least) the PR section of the contributing guide.

Breaking change

diff --git a/docs/src/pages/components/autocomplete/FixedTags.js b/docs/src/pages/components/autocomplete/FixedTags.js
index 757d66a97..a4f36edd5 100644
--- a/docs/src/pages/components/autocomplete/FixedTags.js
+++ b/docs/src/pages/components/autocomplete/FixedTags.js
@@ -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 }}

mui-pr-bot · 2019-11-02T17:44:42Z

Details of bundle changes.

Comparing: 262c8cf...5e60be1

bundle	Size Change	Size	Gzip Change	Gzip
useAutocomplete	▲ +18 B (+0.15% )	11.8 kB	▲ +11 B (+0.25% )	4.34 kB
@material-ui/lab	▲ +10 B (+0.01% )	169 kB	▲ +18 B (+0.04% )	50.9 kB
Autocomplete	▲ +10 B (+0.01% )	125 kB	▲ +5 B (+0.01% )	39.6 kB
@material-ui/core	--	349 kB	--	95.6 kB
@material-ui/core[umd]	--	309 kB	--	88.9 kB
@material-ui/styles	--	50.8 kB	--	15.4 kB
@material-ui/system	--	14.8 kB	--	4.06 kB
AppBar	--	62.2 kB	--	19.5 kB
Avatar	--	61.2 kB	--	19.3 kB
Backdrop	--	66.2 kB	--	20.4 kB
Badge	--	63.8 kB	--	19.7 kB
BottomNavigation	--	60.8 kB	--	19 kB
BottomNavigationAction	--	73.8 kB	--	23.3 kB
Box	--	69.2 kB	--	20.9 kB
Breadcrumbs	--	66.4 kB	--	20.8 kB
Button	--	77.7 kB	--	23.8 kB
ButtonBase	--	72.2 kB	--	22.6 kB
ButtonGroup	--	80.3 kB	--	24.6 kB
Card	--	61.2 kB	--	19.1 kB
CardActionArea	--	73.3 kB	--	23.1 kB
CardActions	--	60.5 kB	--	18.9 kB
CardContent	--	60.4 kB	--	18.9 kB
CardHeader	--	63.5 kB	--	20 kB
CardMedia	--	60.8 kB	--	19.1 kB
Checkbox	--	80.1 kB	--	25.1 kB
Chip	--	81 kB	--	24.7 kB
CircularProgress	--	62.5 kB	--	19.7 kB
ClickAwayListener	--	3.85 kB	--	1.55 kB
Collapse	--	66.3 kB	--	20.5 kB
colorManipulator	--	3.83 kB	--	1.52 kB
Container	--	61.6 kB	--	19.2 kB
CssBaseline	--	56 kB	--	17.5 kB
Dialog	--	80.9 kB	--	25.1 kB
DialogActions	--	60.5 kB	--	18.9 kB
DialogContent	--	60.6 kB	--	19 kB
DialogContentText	--	62.5 kB	--	19.6 kB
DialogTitle	--	62.7 kB	--	19.7 kB
Divider	--	61 kB	--	19.2 kB
docs.landing	--	55.6 kB	--	14.6 kB
docs.main	--	603 kB	--	192 kB
Drawer	--	82.7 kB	--	25.6 kB
ExpansionPanel	--	69.6 kB	--	21.7 kB
ExpansionPanelActions	--	60.5 kB	--	19 kB
ExpansionPanelDetails	--	60.4 kB	--	18.9 kB
ExpansionPanelSummary	--	76.4 kB	--	24.1 kB
Fab	--	75.1 kB	--	23.3 kB
Fade	--	22 kB	--	7.6 kB
FilledInput	--	72 kB	--	22.3 kB
FormControl	--	62.8 kB	--	19.5 kB
FormControlLabel	--	63.9 kB	--	20.1 kB
FormGroup	--	60.4 kB	--	18.9 kB
FormHelperText	--	61.7 kB	--	19.3 kB
FormLabel	--	61.9 kB	--	19.1 kB
Grid	--	63.5 kB	--	19.9 kB
GridList	--	60.9 kB	--	19.1 kB
GridListTile	--	62.2 kB	--	19.5 kB
GridListTileBar	--	61.6 kB	--	19.3 kB
Grow	--	22.6 kB	--	7.72 kB
Hidden	--	64.5 kB	--	20.2 kB
Icon	--	61.2 kB	--	19.2 kB
IconButton	--	74.4 kB	--	23.2 kB
Input	--	70.9 kB	--	22.1 kB
InputAdornment	--	63.5 kB	--	20 kB
InputBase	--	69 kB	--	21.6 kB
InputLabel	--	63.7 kB	--	19.8 kB
LinearProgress	--	63.8 kB	--	19.9 kB
Link	--	65 kB	--	20.6 kB
List	--	60.8 kB	--	18.9 kB
ListItem	--	75.4 kB	--	23.5 kB
ListItemAvatar	--	60.5 kB	--	18.9 kB
ListItemIcon	--	60.6 kB	--	19 kB
ListItemSecondaryAction	--	60.4 kB	--	18.9 kB
ListItemText	--	63.4 kB	--	19.9 kB
ListSubheader	--	61.2 kB	--	19.2 kB
Menu	--	86.6 kB	--	27.2 kB
MenuItem	--	76.4 kB	--	23.8 kB
MenuList	--	64.4 kB	--	20.1 kB
MobileStepper	--	66.2 kB	--	20.6 kB
Modal	--	14.2 kB	--	4.96 kB
NativeSelect	--	75.2 kB	--	23.7 kB
NoSsr	--	2.19 kB	--	1.04 kB
OutlinedInput	--	72.5 kB	--	22.5 kB
Paper	--	60.7 kB	--	18.9 kB
Popover	--	81 kB	--	25 kB
Popper	--	28.5 kB	--	10.2 kB
Portal	--	2.87 kB	--	1.3 kB
Radio	--	80.9 kB	--	25.4 kB
RadioGroup	--	61.7 kB	--	19.3 kB
Rating	--	68.3 kB	--	21.8 kB
RootRef	--	4.43 kB	--	1.67 kB
Select	--	112 kB	--	33.4 kB
Skeleton	--	60.9 kB	--	19.1 kB
Slide	--	24.1 kB	--	8.21 kB
Slider	--	73.9 kB	--	23.3 kB
Snackbar	--	75.5 kB	--	23.5 kB
SnackbarContent	--	64.1 kB	--	20.1 kB
SpeedDial	--	84.3 kB	--	26.6 kB
SpeedDialAction	--	114 kB	--	36 kB
SpeedDialIcon	--	63 kB	--	19.8 kB
Step	--	61 kB	--	19.1 kB
StepButton	--	80.6 kB	--	25.3 kB
StepConnector	--	61.1 kB	--	19.2 kB
StepContent	--	67.4 kB	--	21 kB
StepIcon	--	63.1 kB	--	19.6 kB
StepLabel	--	67 kB	--	21 kB
Stepper	--	63.2 kB	--	19.9 kB
styles/createMuiTheme	--	15.2 kB	--	5.36 kB
SvgIcon	--	61.5 kB	--	19.1 kB
SwipeableDrawer	--	90 kB	--	28 kB
Switch	--	79.4 kB	--	24.7 kB
Tab	--	74.6 kB	--	23.6 kB
Table	--	61 kB	--	19.1 kB
TableBody	--	60.5 kB	--	18.9 kB
TableCell	--	62.5 kB	--	19.6 kB
TableFooter	--	60.5 kB	--	18.9 kB
TableHead	--	60.5 kB	--	18.9 kB
TablePagination	--	139 kB	--	40.5 kB
TableRow	--	61 kB	--	19.1 kB
TableSortLabel	--	75.6 kB	--	23.9 kB
Tabs	--	83.7 kB	--	26.7 kB
TextareaAutosize	--	5.06 kB	--	2.11 kB
TextField	--	121 kB	--	35.4 kB
ToggleButton	--	74.4 kB	--	23.5 kB
ToggleButtonGroup	--	61.6 kB	--	19.4 kB
Toolbar	--	60.8 kB	--	19 kB
Tooltip	--	97.6 kB	--	30.9 kB
TreeItem	--	71.9 kB	--	22.7 kB
TreeView	--	64.8 kB	--	20.3 kB
Typography	--	62.1 kB	--	19.3 kB
useMediaQuery	--	2.49 kB	--	1.05 kB
Zoom	--	22.1 kB	--	7.6 kB

Generated by 🚫 dangerJS against 5e60be1

oliviertassinari · 2019-11-02T17:45:06Z

@tkanzakic Thank you for starting an effort on this issue. Do you think that you can update the rest of the codebase to account for this new approach? We will need to update the demos, the typescript definitions, the docs and add a new test to prevent regressions :).

tkanzakic · 2019-11-02T18:30:36Z

Definitely, let me take a look, thanks for the guidance

oliviertassinari · 2019-11-04T08:57:05Z

@tkanzakic Did you had the time to look at it? :)

tkanzakic · 2019-11-04T09:18:13Z

@oliviertassinari I started to look deeper on this but I’m moving slow as I’m getting used to the source code. If this’s urgent feel free to do it yourself. I can look at a different ticket.

oliviertassinari · 2019-11-04T09:37:30Z

@tkanzakic Ok, no problem. Take your time to dive into the codebase :). I think that it would be great to have it solved in the next release, likely before the end of the week.

tkanzakic · 2019-11-06T12:55:45Z

@oliviertassinari I'm having busy days at work this week, I will try to get back to this today

oliviertassinari · 2019-11-08T12:35:36Z

@tkanzakic I'm doing a follow-up, I will rebase and push a new commit. I think that it would be great to release it this weekend, Saturday, or Sunday. Thank you for the interest, if you could have a look at the diff, it would be awesome.

fix a bug where the remove item always remove the first item in the Autocomplete instead of the selected one

oliviertassinari · 2019-11-08T14:08:08Z

packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js

+          multiple
+        />,
+      );
+      fireEvent.click(container.querySelectorAll('svg[data-mui-test="CancelIcon"]')[1]);


@eps1lon any better idea for this selector?

tkanzakic · 2019-11-08T15:06:47Z

@oliviertassinari thanks for taking the ownership over this task, I am having a long week. Hope to be able to contribute in other tasks.

oliviertassinari · 2019-11-08T15:10:25Z

@tkanzakic Good, this problem was quite important 😁.

karimishe · 2019-11-11T08:41:16Z

Hi, Excuse me ). When this fix will be in npm repository? When can I use correct version?

mdineshari · 2023-06-16T08:59:20Z

packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js

@@ -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);


@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.

oliviertassinari changed the title ~~[Docs] pass item index to getTagProps~~ [Autocomplete] Fix Multiple tag delete action Nov 2, 2019

oliviertassinari added bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! labels Nov 2, 2019

[Docs] pass item index to getTagProps

a1b22df

fix a bug where the remove item always remove the first item in the Autocomplete instead of the selected one

oliviertassinari self-assigned this Nov 8, 2019

add failing test case

5e60be1

oliviertassinari force-pushed the pass-index-to-tag-props branch from a926888 to 5e60be1 Compare November 8, 2019 14:06

oliviertassinari reviewed Nov 8, 2019

View reviewed changes

oliviertassinari removed their assignment Nov 8, 2019

oliviertassinari approved these changes Nov 8, 2019

View reviewed changes

oliviertassinari added the PR: ready to ship label Nov 8, 2019

oliviertassinari merged commit 096bbc0 into mui:master Nov 9, 2019

This comment has been minimized.

Sign in to view

tkanzakic deleted the pass-index-to-tag-props branch January 2, 2020 14:01

snyk-bot mentioned this pull request Jan 7, 2020

[Snyk] Upgrade @material-ui/core from 4.4.0 to 4.8.2 srdjan-19/rent-a-car-service#3

Open

snyk-bot mentioned this pull request Feb 15, 2020

[Snyk] Upgrade @material-ui/core from 4.3.1 to 4.9.0 aubreyyan/1f5374286c#17

Open

mdineshari reviewed Jun 16, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Autocomplete] Fix Multiple tag delete action #18153

[Autocomplete] Fix Multiple tag delete action #18153

tkanzakic commented Nov 2, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Nov 2, 2019 •

edited

Loading

oliviertassinari commented Nov 2, 2019

tkanzakic commented Nov 2, 2019

oliviertassinari commented Nov 4, 2019

tkanzakic commented Nov 4, 2019

oliviertassinari commented Nov 4, 2019

tkanzakic commented Nov 6, 2019

oliviertassinari commented Nov 8, 2019

oliviertassinari Nov 8, 2019

tkanzakic commented Nov 8, 2019

oliviertassinari commented Nov 8, 2019

karimishe commented Nov 11, 2019 •

edited

Loading

This comment has been minimized.

mdineshari Jun 16, 2023

[Autocomplete] Fix Multiple tag delete action #18153

[Autocomplete] Fix Multiple tag delete action #18153

Conversation

tkanzakic commented Nov 2, 2019 • edited by oliviertassinari Loading

Breaking change

mui-pr-bot commented Nov 2, 2019 • edited Loading

oliviertassinari commented Nov 2, 2019

tkanzakic commented Nov 2, 2019

oliviertassinari commented Nov 4, 2019

tkanzakic commented Nov 4, 2019

oliviertassinari commented Nov 4, 2019

tkanzakic commented Nov 6, 2019

oliviertassinari commented Nov 8, 2019

oliviertassinari Nov 8, 2019

Choose a reason for hiding this comment

tkanzakic commented Nov 8, 2019

oliviertassinari commented Nov 8, 2019

karimishe commented Nov 11, 2019 • edited Loading

This comment has been minimized.

mdineshari Jun 16, 2023

Choose a reason for hiding this comment

tkanzakic commented Nov 2, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Nov 2, 2019 •

edited

Loading

karimishe commented Nov 11, 2019 •

edited

Loading