[Autocomplete] Add closeIcon and popupIcon props #18266

AbdullahElrubi · 2019-11-08T03:49:46Z

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

mui-pr-bot · 2019-11-08T03:59:38Z

Details of bundle changes.

Comparing: ba1f645...6418141

bundle	Size Change	Size	Gzip Change	Gzip
@material-ui/lab	▲ +112 B (+0.07% )	169 kB	▲ +68 B (+0.13% )	51 kB
Autocomplete	▲ +112 B (+0.09% )	125 kB	▲ +51 B (+0.13% )	39.7 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	--	605 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	--	74 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	--	72 kB	--	22.7 kB
TreeView	--	64.8 kB	--	20.3 kB
Typography	--	62.1 kB	--	19.3 kB
useAutocomplete	--	11.8 kB	--	4.35 kB
useMediaQuery	--	2.49 kB	--	1.05 kB
Zoom	--	22.1 kB	--	7.6 kB

Generated by 🚫 dangerJS against 6418141

AbdullahElrubi · 2019-11-08T18:37:22Z

@oliviertassinari what I meant is to change the icon with leaving the IconButton with its properties
So will there be a demo for how to do this? By overriding the IconButton itself

oliviertassinari · 2019-11-08T18:44:08Z

What about a CloseIconProps prop instead?

AbdullahElrubi · 2019-11-08T18:48:19Z

So I think they will be like this

PopupIconProps Prop
CloseIconProps Prop
And they only target the Icons for the endAdornment
I can make the changes if you agree with me on this

oliviertassinari · 2019-11-08T19:34:42Z

Ok, my bad, my proposal wouldn't work. What do you think of not solving this problem, but instead, to use the useAutocomplete hook for your use case?

AbdullahElrubi · 2019-11-08T20:05:02Z

But I think what I've done is better for it in this case
Otherwise it will set the default icons

mbrookes · 2019-11-08T23:25:51Z

Here's a review of icon override props (doesn't include any props that set the default icon):

Checkbox:
checkedIcon: The icon to display when the component is checked.
icon: The icon to display when the component is unchecked.
indeterminateIcon: The icon to display when the component is indeterminate.

Chip:
deleteIcon: Override the default delete icon element.

ExpansionPanelSummary:
expandIcon: The icon to display as the expand indicator.

NativeSelect:
IconComponent: The icon that displays the arrow.

Radio:
checkedIcon: The icon to display when the component is checked.
icon: The icon to display when the component is unchecked.

Select:
IconComponent: The icon that displays the arrow.

StepLabel:
StepIconComponent: The component to render in place of the StepIcon.

Switch:
checkedIcon: The icon to display when the component is checked.
icon: The icon to display when the component is unchecked.

TableSortLabel:
IconComponent: Sort icon to use.

It's a bit of a mish-mash for both prop-naming and descriptions, so could do with being reconciled, but for the purpose of this PR, only 3 have Component in the name - Select, StepLabel, & TabelSortLabel. StepLabel is legitimate, since it replaces the StepIcon component. The other two look like a mistake.

For autocomplete, how about we use closeIcon, and popupIcon for the prop names.

For the description, "The icon to display in place of the default XXX icon."? (Whatever wording is chosen needs to work for the other components.)

oliviertassinari · 2019-11-09T01:22:28Z

@mbrookes I think that the idea case is to accept an element. However, it's not always possible, in the case of the select, it accepts a component instead to avoid the cloneElement API.

I think that there is a tradeoff to find here between allowing to customize the icon, the icon button and more. I'm wondering about the best tradeoff we can pick. Here are the different options:

We accept an icon element. I believe it's what @mbrookes propose, it's a better variation to the initial commit that accepts a component to customize this element. It's ideal to customize the icon
We accept an icon button component prop. It allows more customization but adds an extra step to customize the icon.
We do nothing, we encourage the usage of useAutocomplete for this problem. The problem is that it requires to add some custom style. It's not straightforward.
We wait for the unstyled effort. We will likely need to introduce an API to customize each component (I was doing POCs with a components object prop).

I think that I would prefer the options in this order 4, 1, 3, 2. Now, 1 and 4 can probably be used in conjunction, no strong preference.

oliviertassinari · 2019-11-10T11:14:31Z

@AbdallahElroby Thanks

AbdullahElrubi · 2019-11-10T18:21:51Z

@oliviertassinari you're welcome
I'm willing to contribute more on the lab components as much as I can

oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Nov 8, 2019

oliviertassinari changed the title ~~[Autocomplete] Pass CloseIconComponent and PopupIconComponent~~ [Autocomplete] Add CloseIconComponent and PopupIconComponent props Nov 8, 2019

oliviertassinari approved these changes Nov 8, 2019

View reviewed changes

oliviertassinari added new feature New feature or request PR: ready to ship labels Nov 8, 2019

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

AbdullahElrubi and others added 7 commits November 9, 2019 13:59

[Autocomplete] Pass CloseIconComponent and PopupIconComponent

5768f73

Added CloseIconComponent to props

1056ced

updating proptypes

9475084

checking for Prop CloseIconComponent

7040074

Updated autocomplete.md

54a453a

provide more flexibility

4b16b7c

Matt review

6418141

oliviertassinari force-pushed the autocomplete-icons-change branch from 63ea9b8 to 6418141 Compare November 9, 2019 13:06

oliviertassinari changed the title ~~[Autocomplete] Add CloseIconComponent and PopupIconComponent props~~ [Autocomplete] Add closeIcon and popupIcon props Nov 9, 2019

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

oliviertassinari merged commit 0e0f17e into mui:master Nov 10, 2019

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

hzhz mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 5.12.0 to 9.0.0 hzhz/material-ui#104

Open

zhaleTaheri mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 5.12.0 to 9.0.0 zhaleTaheri/material-ui#73

Open

maniator mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 5.12.0 to 9.0.0 maniator/material-ui#93

Open

akashmitra mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 5.12.0 to 9.0.0 akashmitra/material-ui#65

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Autocomplete] Add closeIcon and popupIcon props #18266

[Autocomplete] Add closeIcon and popupIcon props #18266

AbdullahElrubi commented Nov 8, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Nov 8, 2019 •

edited

Loading

AbdullahElrubi commented Nov 8, 2019

oliviertassinari commented Nov 8, 2019

AbdullahElrubi commented Nov 8, 2019

oliviertassinari commented Nov 8, 2019 •

edited

Loading

AbdullahElrubi commented Nov 8, 2019

mbrookes commented Nov 8, 2019 •

edited

Loading

oliviertassinari commented Nov 9, 2019 •

edited

Loading

oliviertassinari commented Nov 10, 2019

AbdullahElrubi commented Nov 10, 2019

[Autocomplete] Add closeIcon and popupIcon props #18266

[Autocomplete] Add closeIcon and popupIcon props #18266

Conversation

AbdullahElrubi commented Nov 8, 2019 • edited by oliviertassinari Loading

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

AbdullahElrubi commented Nov 8, 2019

oliviertassinari commented Nov 8, 2019

AbdullahElrubi commented Nov 8, 2019

oliviertassinari commented Nov 8, 2019 • edited Loading

AbdullahElrubi commented Nov 8, 2019

mbrookes commented Nov 8, 2019 • edited Loading

oliviertassinari commented Nov 9, 2019 • edited Loading

oliviertassinari commented Nov 10, 2019

AbdullahElrubi commented Nov 10, 2019

AbdullahElrubi commented Nov 8, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Nov 8, 2019 •

edited

Loading

oliviertassinari commented Nov 8, 2019 •

edited

Loading

mbrookes commented Nov 8, 2019 •

edited

Loading

oliviertassinari commented Nov 9, 2019 •

edited

Loading