[Button] Add loading prop support by oliviertassinari · Pull Request #17810 · mui/material-ui

oliviertassinari · 2019-10-09T17:16:36Z

      <Button variant="outlined" loading>
        Loading…
      </Button>
      <Button
        variant="contained"
        color="primary"
        loading
        startIcon={<CircularProgress color="inherit" size={16} />}
      >
        Duplicate
      </Button>

Benchmark

The implementation was benchmarked with the following sources. I have noticed that the loading and disabled states are often styled independently.

evergreen

blueprint

vuetify

cosmos

antd

element

sancho

semantic ui

quasar

baseui

atlaskit

chakra

orbit

mui-pr-bot · 2019-10-09T17:26:45Z

Details of bundle changes.

Comparing: 31d3008...326e15f

bundle	Size Change	Size	Gzip Change	Gzip
Button	▲ +163 B (+0.21% )	79.4 kB	▲ +79 B (+0.32% )	24.6 kB
docs.main	▲ +163 B (+0.03% )	599 kB	▲ +66 B (+0.03% )	191 kB
@material-ui/core	▲ +163 B (+0.05% )	347 kB	▲ +48 B (+0.05% )	95.1 kB
@material-ui/core[umd]	▲ +160 B (+0.05% )	306 kB	▲ +53 B (+0.06% )	88.1 kB
ExpansionPanelSummary	▲ +43 B (+0.06% )	77.9 kB	▲ +19 B (+0.08% )	24.5 kB
SpeedDial	▲ +43 B (+0.05% )	85.9 kB	▲ +19 B (+0.07% )	27 kB
Tabs	▲ +43 B (+0.05% )	85.3 kB	▲ +19 B (+0.07% )	27.1 kB
SpeedDialAction	▲ +43 B (+0.04% )	115 kB	▲ +18 B (+0.05% )	36.4 kB
Switch	▲ +43 B (+0.05% )	80.8 kB	▲ +18 B (+0.07% )	25.1 kB
Checkbox	▲ +43 B (+0.05% )	81.6 kB	▲ +17 B (+0.07% )	25.6 kB
IconButton	▲ +43 B (+0.06% )	76 kB	▲ +17 B (+0.07% )	23.7 kB
ListItem	▲ +43 B (+0.06% )	77 kB	▲ +17 B (+0.07% )	24 kB
MenuItem	▲ +43 B (+0.06% )	78 kB	▲ +17 B (+0.07% )	24.3 kB
Radio	▲ +43 B (+0.05% )	82.5 kB	▲ +17 B (+0.07% )	25.9 kB
StepButton	▲ +43 B (+0.05% )	82.2 kB	▲ +17 B (+0.07% )	25.8 kB
TableSortLabel	▲ +43 B (+0.06% )	77.2 kB	▲ +17 B (+0.07% )	24.4 kB
@material-ui/lab	▲ +43 B (+0.03% )	145 kB	▲ +16 B (+0.04% )	45 kB
ButtonBase	▲ +43 B (+0.06% )	73.9 kB	▲ +16 B (+0.07% )	23.1 kB
BottomNavigationAction	▲ +43 B (+0.06% )	75.4 kB	▲ +15 B (+0.06% )	23.7 kB
CardActionArea	▲ +43 B (+0.06% )	74.9 kB	▲ +15 B (+0.06% )	23.6 kB
Tab	▲ +43 B (+0.06% )	76.2 kB	▲ +15 B (+0.06% )	24.1 kB
ToggleButton	▲ +43 B (+0.06% )	76 kB	▲ +15 B (+0.06% )	23.9 kB
Fab	▲ +43 B (+0.06% )	76.7 kB	▲ +14 B (+0.06% )	23.8 kB
TablePagination	▲ +43 B (+0.03% )	139 kB	▲ +10 B (+0.02% )	40.6 kB
@material-ui/styles	--	51.8 kB	--	15.6 kB
@material-ui/system	--	15.7 kB	--	4.35 kB
AppBar	--	63.9 kB	--	19.9 kB
Avatar	--	62.7 kB	--	19.6 kB
Backdrop	--	67.7 kB	--	20.9 kB
Badge	--	65.3 kB	--	20.2 kB
BottomNavigation	--	62.4 kB	--	19.5 kB
Box	--	70.8 kB	--	21.3 kB
Breadcrumbs	--	68 kB	--	21.3 kB
ButtonGroup	--	64.2 kB	--	20 kB
Card	--	62.9 kB	--	19.6 kB
CardActions	--	62 kB	--	19.3 kB
CardContent	--	62 kB	--	19.3 kB
CardHeader	--	65.1 kB	--	20.4 kB
CardMedia	--	62.4 kB	--	19.5 kB
Chip	--	70.6 kB	--	21.8 kB
CircularProgress	--	64.1 kB	--	20.1 kB
ClickAwayListener	--	3.85 kB	--	1.55 kB
Collapse	--	67.8 kB	--	20.9 kB
colorManipulator	--	3.83 kB	--	1.52 kB
Container	--	63.1 kB	--	19.6 kB
CssBaseline	--	57.6 kB	--	18 kB
Dialog	--	82.5 kB	--	25.5 kB
DialogActions	--	62.1 kB	--	19.4 kB
DialogContent	--	62.2 kB	--	19.4 kB
DialogContentText	--	64 kB	--	20 kB
DialogTitle	--	64.3 kB	--	20.1 kB
Divider	--	62.6 kB	--	19.6 kB
docs.landing	--	54.3 kB	--	14.3 kB
Drawer	--	84.3 kB	--	26 kB
ExpansionPanel	--	71.1 kB	--	22.1 kB
ExpansionPanelActions	--	62.1 kB	--	19.4 kB
ExpansionPanelDetails	--	61.9 kB	--	19.3 kB
Fade	--	23.1 kB	--	8.03 kB
FilledInput	--	72.9 kB	--	22.6 kB
FormControl	--	64.3 kB	--	19.9 kB
FormControlLabel	--	65.5 kB	--	20.5 kB
FormGroup	--	62 kB	--	19.3 kB
FormHelperText	--	63.3 kB	--	19.7 kB
FormLabel	--	63.2 kB	--	19.5 kB
Grid	--	65.1 kB	--	20.3 kB
GridList	--	62.5 kB	--	19.5 kB
GridListTile	--	63.7 kB	--	19.9 kB
GridListTileBar	--	63.2 kB	--	19.7 kB
Grow	--	23.7 kB	--	8.15 kB
Hidden	--	66.1 kB	--	20.6 kB
Icon	--	62.8 kB	--	19.6 kB
Input	--	72 kB	--	22.4 kB
InputAdornment	--	65.1 kB	--	20.4 kB
InputBase	--	70.2 kB	--	22 kB
InputLabel	--	65.1 kB	--	20.2 kB
LinearProgress	--	65.3 kB	--	20.3 kB
Link	--	66.6 kB	--	21 kB
List	--	62.4 kB	--	19.3 kB
ListItemAvatar	--	62.1 kB	--	19.3 kB
ListItemIcon	--	62.2 kB	--	19.4 kB
ListItemSecondaryAction	--	62 kB	--	19.3 kB
ListItemText	--	65 kB	--	20.3 kB
ListSubheader	--	62.8 kB	--	19.6 kB
Menu	--	88.2 kB	--	27.6 kB
MenuList	--	66 kB	--	20.5 kB
MobileStepper	--	67.7 kB	--	21 kB
Modal	--	14.3 kB	--	4.96 kB
NativeSelect	--	76.4 kB	--	24 kB
NoSsr	--	2.19 kB	--	1.04 kB
OutlinedInput	--	73.5 kB	--	22.8 kB
Paper	--	62.4 kB	--	19.3 kB
Popover	--	82.6 kB	--	25.4 kB
Popper	--	28.3 kB	--	10.2 kB
Portal	--	2.87 kB	--	1.29 kB
RadioGroup	--	63.2 kB	--	19.7 kB
Rating	--	69.8 kB	--	22.2 kB
RootRef	--	4.43 kB	--	1.67 kB
Select	--	113 kB	--	33.5 kB
Skeleton	--	62.5 kB	--	19.5 kB
Slide	--	25.1 kB	--	8.66 kB
Slider	--	75.3 kB	--	23.7 kB
Snackbar	--	77.1 kB	--	24 kB
SnackbarContent	--	65.8 kB	--	20.6 kB
SpeedDialIcon	--	64.6 kB	--	20.2 kB
Step	--	62.6 kB	--	19.5 kB
StepConnector	--	62.7 kB	--	19.6 kB
StepContent	--	68.9 kB	--	21.4 kB
StepIcon	--	64.7 kB	--	20.1 kB
StepLabel	--	68.6 kB	--	21.4 kB
Stepper	--	64.9 kB	--	20.3 kB
styles/createMuiTheme	--	16.2 kB	--	5.78 kB
SvgIcon	--	63 kB	--	19.6 kB
SwipeableDrawer	--	90.7 kB	--	28 kB
Table	--	62.4 kB	--	19.4 kB
TableBody	--	62.1 kB	--	19.3 kB
TableCell	--	64.1 kB	--	20 kB
TableFooter	--	62.1 kB	--	19.3 kB
TableHead	--	62.1 kB	--	19.3 kB
TableRow	--	62.5 kB	--	19.5 kB
TextareaAutosize	--	5.06 kB	--	2.11 kB
TextField	--	121 kB	--	35.4 kB
ToggleButtonGroup	--	63.2 kB	--	19.8 kB
Toolbar	--	62.3 kB	--	19.4 kB
Tooltip	--	99 kB	--	31.3 kB
TreeItem	--	73.4 kB	--	23 kB
TreeView	--	66 kB	--	20.6 kB
Typography	--	63.6 kB	--	19.8 kB
useMediaQuery	--	2.49 kB	--	1.05 kB
Zoom	--	23.1 kB	--	8.04 kB

Generated by 🚫 dangerJS against 326e15f

oliviertassinari

Two other concerns to take into account:

The button width change when the circular progress is added or when the button label changes
The button with circular progress only is no supported

oliviertassinari · 2019-10-09T17:22:03Z

+  loading: {
+    pointerEvents: 'none', // Disable link interactions
+    cursor: 'default',
+    opacity: 0.4,


Same value as the disabled rating.

oliviertassinari · 2019-10-09T17:23:54Z

        classes.root,
        {
-          [classes.disabled]: disabled,
+          [classes.disabled]: disabledProp,


I had to introduce a private loading prop to disable the interaction without applying the disabled CSS. I don't like that, but I can't think of a better approach.

oliviertassinari · 2019-10-09T17:24:49Z

+        variant="contained"
+        color="primary"
+        loading
+        startIcon={<CircularProgress color="inherit" size={16} />}


Most implementations bundle the circular directly in the button. I'm not confident this is best from a bundle size perspective. I'm wondering. It might also complexify the API to expose in case people want to change how the loading is displayed.

This is a general statement not targeted at anyone particular: I'm getting weary of this "props everything" approach. React introduced "component based ui development" not "props based ui development". It's ok to add another LoadableButton where one could flip the switch and we provide the spinner.

"BusyButton" 🙂

eps1lon · 2019-10-11T09:32:53Z

  /* Pseudo-class applied to the root element if `disabled={true}`. */
  disabled: {},
+  /* Styles applied to the root element if `loading={true}`. */
+  loading: {


It's not actually related to loading (it's only used in this context). What this actually conveys is that the button is "deactivated" or "not actionable" or "temporary disabled". loading implies that there is another indicator.

As I said below I'd be perfectly fine with introducing a separate component that overrides Button properly.

As I said below I'd be perfectly fine with introducing a separate component that overrides Button properly.

The loading state is transient, how would you switch between the loading and non-loading state with a new component? In this regard, I think that a switch of a prop has the potential to be simpler to use.

I don't think transient applies to a loading state or any UI in that matter.

But even then you can still have the loading prop which then actually conveys the proper meaning by switching styles and adding an indicator.

oliviertassinari · 2019-10-17T18:36:23Z

I'm closing, it doesn't seem to be a very frequent user request (while I expect it to be frequently needed) but it seems to require more effort to reach a consensus at least, we have: https://material-ui.com/components/progress/#interactive-integration.

Thanks for the review guys.

oliviertassinari · 2019-10-17T18:40:42Z

more effort to reach a consensus

Well, actually, BusyButton might be a good enough solution, but I would rather focus on different problems. If somebody wants to resume this effort 👍.

[Button] Add loading prop support

326e15f

oliviertassinari commented Oct 9, 2019

View reviewed changes

oliviertassinari added scope: button Changes related to the button. type: new feature Expand the scope of the product to solve a new problem. labels Oct 9, 2019

oliviertassinari marked this pull request as ready for review October 9, 2019 21:01

oliviertassinari added the PR: needs review label Oct 10, 2019

eps1lon reviewed Oct 11, 2019

View reviewed changes

oliviertassinari added PR: needs revision and removed PR: needs review labels Oct 17, 2019

oliviertassinari closed this Oct 17, 2019

oliviertassinari deleted the loading-button branch October 17, 2019 18:43

oliviertassinari mentioned this pull request Dec 16, 2019

[Button] Add loading support #7223

Closed

mnajdova mentioned this pull request Jun 10, 2020

[LoadingButton] Introduce new component #21389

Merged

Uh oh!

Uh oh!

Conversation

oliviertassinari commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark

Uh oh!

mui-pr-bot commented Oct 9, 2019

Uh oh!

oliviertassinari left a comment

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Oct 9, 2019

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Oct 9, 2019

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eps1lon Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

mbrookes Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

eps1lon Oct 11, 2019

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Oct 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eps1lon Oct 11, 2019

Choose a reason for hiding this comment

Uh oh!

oliviertassinari commented Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

oliviertassinari commented Oct 9, 2019 •

edited

Loading

oliviertassinari Oct 9, 2019 •

edited

Loading

oliviertassinari Oct 11, 2019 •

edited

Loading

oliviertassinari commented Oct 17, 2019 •

edited

Loading

oliviertassinari commented Oct 17, 2019 •

edited

Loading