[Chip] Add outlined variant #12680

orporan · 2018-08-27T19:32:00Z

Added support for outlined chip as demonstrated in material design guidelines

mbrookes · 2018-08-27T21:23:00Z

From the spec it seems that the outlined chip has lighter shades for hover & focused states. Do you think we should add those?

orporan · 2018-08-27T23:21:38Z

@mbrookes Thank you :)
I added the background colors for hover and focus states, but I keep getting build errors due to issues with the yarn package registry and I don't see a way for me to rerun the tests. how do I go about it?

mbrookes · 2018-08-28T00:28:36Z

The style descriptions are likely failing because they are split over two lines - at the moment that isn't supported. If the descriptions can't be condensed to 100 chars, we'll have to fix that.

orporan · 2018-08-28T07:56:01Z

@mbrookes
I got a failure in argos because I changed the Demo page to show the new Chip behavior.
https://www.argos-ci.com/mui-org/material-ui/builds/31206
What do I do about it?

mbrookes · 2018-08-28T10:57:24Z

I have approved the visual diff, but should outlined perhaps have its own demo section?

orporan · 2018-08-28T12:59:47Z

@mbrookes
Done :)

oliviertassinari

A quick review while I'm at the beach 🌊☀️.

oliviertassinari · 2018-08-28T13:24:30Z

docs/src/pages/demos/chips/chips.md

@@ -25,6 +25,13 @@ and (string) Avatar.

 {{"demo": "pages/demos/chips/Chips.js"}}

+### Outlined Chips
+
+Outlined chips offer alternative style 


"offer alternative style" -> "offer an alternative style."

oliviertassinari · 2018-08-28T13:27:07Z

docs/src/pages/demos/chips/OutlinedChips.js

+  alert('You clicked the Chip.'); // eslint-disable-line no-alert
+}
+
+function Chips(props) {


OutlinedChips

packages/material-ui/src/Chip/Chip.d.ts

@@ -29,7 +30,9 @@ export type ChipClassKey =
  | 'label'
  | 'deleteIcon'
  | 'deleteIconPrimary'
-  | 'deleteIconSecondary';
+  | 'deleteIconSecondary'


oliviertassinari · 2018-08-28T13:36:52Z

packages/material-ui/src/Chip/Chip.js

    deleteIconColorSecondary: {
      color: fade(theme.palette.primary.contrastText, 0.65),
      '&:hover, &:active': {
        color: theme.palette.primary.contrastText,
      },
    },
+    /* Styles applied to the deleteIcon element if `color="primary"` and `variant="outlined"`. */
+    deleteIconColorPrimaryOutlined: {


We should probably reverse color primary and outlined.

orporan · 2018-08-28T16:18:10Z

@mbrookes @oliviertassinari Thanks for the quick review. I addressed all comments and the tests are passing (except for argos, but that's due to the intentional change in Demo components)

orporan · 2018-08-28T18:12:04Z

@mbrookes Thank you for merging the master branch into the PR. I adjusted the package size accordingly and all the tests are passing now.
I hope its okay that I'm asking, but how soon can this be released? I need to use it for something :)

mbrookes

@orporan Looks good to go. 👌

Once merged, it will be released in the next weekly release cycle.

orporan · 2018-08-28T19:09:55Z

@mbrookes great!
Is there anything else I need to do for this to get merged?

mbrookes

@orporan Sorry, yes. I just realised we don't have any tests for the new variant. It should be straightforward enough to duplicate a test and modify it to check that the correct classes are applied when the outlined variant is specified.

mbrookes

That was quick. 😄 One last request...

mbrookes · 2018-08-28T19:56:31Z

docs/src/pages/demos/chips/OutlinedChips.js

+            <FaceIcon />
+          </Avatar>
+        }
+        label="Clickable Link Chip"


Not your fault as it came from the prior demo, but the labels on the last three chips don't seem to match their purpose. The last two are primary color and secondary color, and the third-last seems to be a smoosh of a whole bunch - primary clickable custom delete.

Please could you give them appropriate labels in both examples?

orporan · 2018-08-28T20:28:37Z

@mbrookes all done :)

mbrookes · 2018-08-28T20:47:41Z

👌

mbrookes · 2018-08-28T21:47:49Z

@oliviertassinari Ever the eagle eye! 👍

mbrookes · 2018-08-28T21:48:58Z

@orporan Thanks for your perseverance! Less than a week 'till it's released.

oliviertassinari · 2018-08-28T21:50:31Z

@mbrookes I'm releasing a v3.0.1 with the peer dependency fix.

orporan · 2018-08-28T21:51:32Z

@mbrookes @oliviertassinari Thanks! it was a pleasure :)

orporan added 2 commits August 27, 2018 22:25

add support for outlined chip

1397e2a

fix ChipsPlayground

2477220

oliviertassinari changed the title ~~[core] Add outlined chip~~ [Chip] Add outlined variant Aug 27, 2018

oliviertassinari added new feature New feature or request component: chip This is the name of the generic UI component, not the React module! labels Aug 27, 2018

orporan added 3 commits August 27, 2018 23:26

yarn prettier

a895a1e

yarn docs:api

d961db8

update size limit

e8c5a02

orporan added 5 commits August 28, 2018 01:28

Add hover and focus background color for outlined chip

1adbe87

yarn prettier

0881271

Add focus state for deletable outlined chip

a670700

Fix chip documentation issue

75d73eb

Revert last commit

4500f1e

empty commit to trigger build

3892c51

orporan added 3 commits August 28, 2018 14:52

Add demo seperate demo section to outlined chip

475c92d

empty commit to trigger build

eec38e7

empty commit to trigger build

848a11f

oliviertassinari reviewed Aug 28, 2018

View reviewed changes

orporan added 4 commits August 28, 2018 17:15

PR review fixes

471eca0

PR review fixes

4bddff3

empty commit to trigger build

6dfab0b

empty commit to trigger build

135e45d

Merge branch 'master' into add-outlined-chip

223485d

orporan added 2 commits August 28, 2018 20:36

empty commit to trigger build

0902b25

fix size limit after merge

75af4c7

mbrookes approved these changes Aug 28, 2018

View reviewed changes

mbrookes requested changes Aug 28, 2018

View reviewed changes

orporan added 2 commits August 28, 2018 22:45

Add tests for outlined chip

7c6bfdf

yarn prettier

602caae

mbrookes reviewed Aug 28, 2018

View reviewed changes

orporan added 2 commits August 28, 2018 23:04

empty commit to trigger build

acf70d2

Fix demo labels for chip

2be6635

mbrookes approved these changes Aug 28, 2018

View reviewed changes

oliviertassinari added 2 commits August 28, 2018 23:12

Update chips.md

e4736c6

let's merge

34f7b23

oliviertassinari approved these changes Aug 28, 2018

View reviewed changes

mbrookes merged commit aa5c442 into mui:master Aug 28, 2018

orporan deleted the add-outlined-chip branch August 28, 2018 21:51

marcelpanse pushed a commit to marcelpanse/material-ui that referenced this pull request Oct 2, 2018

[Chip] Add outlined variant (mui#12680)

71f21ad

LorenzHenk mentioned this pull request Dec 18, 2019

[Chip] variant default value #18914

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Chip] Add outlined variant #12680

[Chip] Add outlined variant #12680

orporan commented Aug 27, 2018

mbrookes commented Aug 27, 2018

orporan commented Aug 27, 2018

mbrookes commented Aug 28, 2018

orporan commented Aug 28, 2018

mbrookes commented Aug 28, 2018

orporan commented Aug 28, 2018

oliviertassinari left a comment

oliviertassinari Aug 28, 2018

mbrookes Aug 28, 2018

oliviertassinari Aug 28, 2018

This comment was marked as resolved.

oliviertassinari Aug 28, 2018

orporan commented Aug 28, 2018

orporan commented Aug 28, 2018

mbrookes left a comment

orporan commented Aug 28, 2018

mbrookes left a comment •

edited

Loading

mbrookes left a comment

mbrookes Aug 28, 2018

orporan commented Aug 28, 2018

mbrookes commented Aug 28, 2018

mbrookes commented Aug 28, 2018

mbrookes commented Aug 28, 2018

oliviertassinari commented Aug 28, 2018

orporan commented Aug 28, 2018

[Chip] Add outlined variant #12680

[Chip] Add outlined variant #12680

Conversation

orporan commented Aug 27, 2018

mbrookes commented Aug 27, 2018

orporan commented Aug 27, 2018

mbrookes commented Aug 28, 2018

orporan commented Aug 28, 2018

mbrookes commented Aug 28, 2018

orporan commented Aug 28, 2018

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Aug 28, 2018

Choose a reason for hiding this comment

mbrookes Aug 28, 2018

Choose a reason for hiding this comment

oliviertassinari Aug 28, 2018

Choose a reason for hiding this comment

This comment was marked as resolved.

oliviertassinari Aug 28, 2018

Choose a reason for hiding this comment

orporan commented Aug 28, 2018

orporan commented Aug 28, 2018

mbrookes left a comment

Choose a reason for hiding this comment

orporan commented Aug 28, 2018

mbrookes left a comment • edited Loading

Choose a reason for hiding this comment

mbrookes left a comment

Choose a reason for hiding this comment

mbrookes Aug 28, 2018

Choose a reason for hiding this comment

orporan commented Aug 28, 2018

mbrookes commented Aug 28, 2018

mbrookes commented Aug 28, 2018

mbrookes commented Aug 28, 2018

oliviertassinari commented Aug 28, 2018

orporan commented Aug 28, 2018

mbrookes left a comment •

edited

Loading