[chip] Add outlined variant#12680
Conversation
oliviertassinari
changed the title
oliviertassinari
added
type: new feature
|
@orporan Nice job! From the spec it seems that the outlined chip has lighter shades for hover & focused states. Do you think we should add those? |
|
@mbrookes Thank you :) |
|
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. |
|
@mbrookes |
|
I have approved the visual diff, but should outlined perhaps have its own demo section? |
|
@mbrookes |
oliviertassinari
left a comment
oliviertassinari
left a comment
There was a problem hiding this comment.
A quick review while I'm at the beach 🌊☀️.
|
|
||
| ### Outlined Chips | ||
|
|
||
| Outlined chips offer alternative style |
There was a problem hiding this comment.
"offer alternative style" -> "offer an alternative style."
| alert('You clicked the Chip.'); // eslint-disable-line no-alert | ||
| } | ||
|
|
||
| function Chips(props) { |
| | 'deleteIcon' | ||
| | 'deleteIconPrimary' | ||
| | 'deleteIconSecondary'; | ||
| | 'deleteIconSecondary' |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| }, | ||
| }, | ||
| /* Styles applied to the deleteIcon element if `color="primary"` and `variant="outlined"`. */ | ||
| deleteIconColorPrimaryOutlined: { |
There was a problem hiding this comment.
We should probably reverse color primary and outlined.
|
@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) |
|
@mbrookes Thank you for merging the master branch into the PR. I adjusted the package size accordingly and all the tests are passing now. |
|
@mbrookes great! |
There was a problem hiding this comment.
@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
left a comment
mbrookes
left a comment
There was a problem hiding this comment.
That was quick. 😄 One last request...
| <FaceIcon /> | ||
| </Avatar> | ||
| } | ||
| label="Clickable Link Chip" |
There was a problem hiding this comment.
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?
|
@mbrookes all done :) |
|
👌 |
|
@oliviertassinari Ever the eagle eye! 👍 |
|
@orporan Thanks for your perseverance! Less than a week 'till it's released. |
|
@mbrookes I'm releasing a v3.0.1 with the peer dependency fix. |
|
@mbrookes @oliviertassinari Thanks! it was a pleasure :) |
oliviertassinari
changed the title
3 participants
Added support for outlined chip as demonstrated in material design guidelines