[Avatar] Add square variant and documentation #18116

ghost · 2019-10-31T05:33:40Z

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

Closes #8426

What I did:

Took guidance from existing code for Button and Page components
Added round and square options for the variant property (default is round)
Added an example in the Avatar component documentation
Applied yarn typescript, yarn prettier and yarn lint
Generated new API documentation

What needs to be done next:

Write tests to confirm round or square class has been applied, and that the correct borderRadius value is set

I've not used these test frameworks but I am keen to learn.

mui-pr-bot · 2019-10-31T05:44:22Z

Details of bundle changes.

Comparing: 8a78af8...e056a5c

bundle	Size Change	Size	Gzip Change	Gzip
Avatar	▲ +129 B (+0.21% )	61.2 kB	▲ +58 B (+0.30% )	19.3 kB
@material-ui/core[umd]	▲ +129 B (+0.04% )	308 kB	▲ +40 B (+0.05% )	88.8 kB
@material-ui/core	▲ +129 B (+0.04% )	349 kB	▲ +35 B (+0.04% )	95.5 kB
@material-ui/lab	--	168 kB	--	50.6 kB
@material-ui/styles	--	50.8 kB	--	15.4 kB
@material-ui/system	--	14.8 kB	--	4.07 kB
AppBar	--	62.2 kB	--	19.5 kB
Autocomplete	--	125 kB	--	39.4 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.8 kB	--	24.1 kB
ButtonBase	--	72.3 kB	--	22.6 kB
ButtonGroup	--	62.7 kB	--	19.5 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 kB	--	25.1 kB
Chip	--	80.9 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.7 kB	--	19 kB
DialogContentText	--	62.5 kB	--	19.6 kB
DialogTitle	--	62.7 kB	--	19.7 kB
Divider	--	61 kB	--	19.1 kB
docs.landing	--	56 kB	--	14.6 kB
docs.main	--	602 kB	--	192 kB
Drawer	--	82.7 kB	--	25.6 kB
ExpansionPanel	--	69.7 kB	--	21.7 kB
ExpansionPanelActions	--	60.5 kB	--	18.9 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	--	64 kB	--	20.1 kB
FormGroup	--	60.5 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.7 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.1 kB	--	21.6 kB
InputLabel	--	63.8 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.6 kB	--	18.9 kB
ListItemIcon	--	60.6 kB	--	19 kB
ListItemSecondaryAction	--	60.5 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.29 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.8 kB	--	23.3 kB
Snackbar	--	75.6 kB	--	23.5 kB
SnackbarContent	--	64.1 kB	--	20.1 kB
SpeedDial	--	84.3 kB	--	26.5 kB
SpeedDialAction	--	114 kB	--	36 kB
SpeedDialIcon	--	63 kB	--	19.8 kB
Step	--	61.1 kB	--	19.1 kB
StepButton	--	80.6 kB	--	25.3 kB
StepConnector	--	61.2 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.1 kB	--	27.9 kB
Switch	--	79.3 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.6 kB	--	18.9 kB
TableHead	--	60.6 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.6 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.7 kB	--	30.9 kB
TreeItem	--	71.8 kB	--	22.6 kB
TreeView	--	64.4 kB	--	20.1 kB
Typography	--	62.1 kB	--	19.3 kB
useAutocomplete	--	11.1 kB	--	4.21 kB
useMediaQuery	--	2.49 kB	--	1.05 kB
Zoom	--	22.1 kB	--	7.6 kB

Generated by 🚫 dangerJS against e056a5c

oliviertassinari

Great start! I have added a third variant to the list:

(circle / square | rounded)

Does the wording look good?

docs/src/pages/components/avatars/avatars.md

mbrookes · 2019-10-31T19:28:55Z

Paper:

Name	Type	Default	Description
square	bool	false	If true, rounded corners are disabled.

How do we reconcile these APIs? (We did a big job for v1 to standardise this sort of thing, but drift is inevitable unless kept in check.)

Do you need square or rounded avatars? Use the variant prop.

Can we avoid the use of "Want X? Do Y." where possible? (it doesn't read so well in English.)

"If you need square or rounded avatars, use the variant prop."

oliviertassinari · 2019-11-01T00:47:25Z

@mbrookes Great question, I have been keen to solve this Paper concern since v1 :). What do you think of this normalization changes:

We rename Fab variant round to circle

diff --git a/packages/material-ui/src/Paper/Paper.js b/packages/material-ui/src/Paper/Paper.js
index cf6c41260..85395f8c2 100644
--- a/packages/material-ui/src/Paper/Paper.js
+++ b/packages/material-ui/src/Paper/Paper.js
@@ -18,11 +18,12 @@ export const styles = theme => {
       backgroundColor: theme.palette.background.paper,
       color: theme.palette.text.primary,
       transition: theme.transitions.create('box-shadow'),
-    },
-    /* Styles applied to the root element if `square={false}`. */
-    rounded: {
       borderRadius: theme.shape.borderRadius,
     },
+    /* Styles applied to the root element if `square={true}`. */
+    square: {
+      borderRadius: 0,
+    },
     ...elevations,
   };
 };
@@ -51,7 +52,7 @@ const Paper = React.forwardRef(function Paper(props, ref) {
         classes.root,
         classes[`elevation${elevation}`],
         {
-          [classes.rounded]: !square,
+          [classes.square]: square,
         },
         className,
       )}

"If you need square or rounded avatars, use the variant prop."

👍

mbrookes · 2019-11-01T22:44:40Z

We rename Fab variant round to circle

"circular" is synonymous with "round", while a circle is "a two-dimensional geometric figure, a line, consisting of the set of all those points in a plane that are equally distant from another point". The Fab isn't a circle. What's wrong with "round" though?

diff --git a/packages/material-ui/src/Paper/Paper.js b/packages/material-ui/src/Paper/Paper.js

How does changing the class name help reconcile the API difference (variant prop vs. square prop)?

oliviertassinari · 2019-11-01T23:39:45Z

"circular" is synonymous with "round", while a "circle" is "a two-dimensional geometric figure, a line, consisting of the set of all those points in a plane that are equally distant from another point". The Fab isn't a circle. What's wrong with "round" though?

@mbrookes The main motivation for the proposal was to find the least amount of changes required to unify the API at the library level. We use "circle" over "round" for a couple of other cases: Badge and Skeleton.
As long as we have something coherent, we should be good. I don't have a strong preference for either wording. If you think that round and circle have a different enough meaning, why not using both 👍.

How does changing the class name help reconcile the API difference (variant prop vs. square prop)?

The API shape difference (boolean vs enum) could be justified by the number of cases supported. Paper has 2, Avatar would have 3. I believe it's something we document in the API guide.

docs/src/pages/components/avatars/VariantAvatars.tsx

oliviertassinari · 2019-11-04T08:24:01Z

@mattdotam Well done

[Avatar] Add square variant and documentation

bf7801d

ghost mentioned this pull request Oct 31, 2019

[Avatar] Add shape="square" support #8426

Closed

oliviertassinari added the component: avatar This is the name of the generic UI component, not the React module! label Oct 31, 2019

Add a third variant

62bd0ff

oliviertassinari reviewed Oct 31, 2019

View reviewed changes

oliviertassinari requested a review from joshwooding October 31, 2019 09:13

oliviertassinari added the new feature New feature or request label Oct 31, 2019

rename round rounded

42e0e7a

joshwooding reviewed Oct 31, 2019

View reviewed changes

docs/src/pages/components/avatars/avatars.md Outdated Show resolved Hide resolved

Matt review

869619a

oliviertassinari approved these changes Nov 3, 2019

View reviewed changes

oliviertassinari requested a review from mbrookes November 3, 2019 10:38

mbrookes reviewed Nov 3, 2019

View reviewed changes

docs/src/pages/components/avatars/VariantAvatars.tsx Outdated Show resolved Hide resolved

Matt review

e056a5c

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

oliviertassinari merged commit 21fb133 into mui:master Nov 4, 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

oliviertassinari mentioned this pull request Jul 24, 2020

[Fab] Rename round -> circular for consistency #21903

Merged

1 task

kodai3 mentioned this pull request Jul 27, 2020

[RFC] Renaming API for consistency #21964

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Avatar] Add square variant and documentation #18116

[Avatar] Add square variant and documentation #18116

ghost commented Oct 31, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Oct 31, 2019 •

edited

Loading

oliviertassinari left a comment •

edited

Loading

mbrookes commented Oct 31, 2019 •

edited

Loading

oliviertassinari commented Nov 1, 2019

mbrookes commented Nov 1, 2019 •

edited

Loading

oliviertassinari commented Nov 1, 2019 •

edited

Loading

oliviertassinari commented Nov 4, 2019

[Avatar] Add square variant and documentation #18116

[Avatar] Add square variant and documentation #18116

Conversation

ghost commented Oct 31, 2019 • edited by oliviertassinari Loading

mui-pr-bot commented Oct 31, 2019 • edited Loading

oliviertassinari left a comment • edited Loading

Choose a reason for hiding this comment

mbrookes commented Oct 31, 2019 • edited Loading

oliviertassinari commented Nov 1, 2019

mbrookes commented Nov 1, 2019 • edited Loading

oliviertassinari commented Nov 1, 2019 • edited Loading

oliviertassinari commented Nov 4, 2019

ghost commented Oct 31, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Oct 31, 2019 •

edited

Loading

oliviertassinari left a comment •

edited

Loading

mbrookes commented Oct 31, 2019 •

edited

Loading

mbrookes commented Nov 1, 2019 •

edited

Loading

oliviertassinari commented Nov 1, 2019 •

edited

Loading