Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Chip] Support non-avatared icon on Chip #12789

Closed
2 tasks done
hidai opened this issue Sep 6, 2018 · 4 comments
Closed
2 tasks done

[Chip] Support non-avatared icon on Chip #12789

hidai opened this issue Sep 6, 2018 · 4 comments
Labels
component: chip This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process new feature New feature or request

Comments

@hidai
Copy link

hidai commented Sep 6, 2018

The Material Design spec allows us to put a simple icon which is not wrapped by an avatar component on Chips.
See https://material.io/design/components/chips.html#
But Chip of material-ui presupposes an Avatar as icon.

  • This is a v1.x and v3.0 issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Support <Chip avatar={<SvgIcon />} /> or add a new props like <Chip icon={<SvgIcon />}.

Current Behavior

<Chip avatar={<SvgIcon />} /> renders the icon too big.

Examples

https://codesandbox.io/s/m5mkwyy19y?module=%2Fsrc%2Fpages%2Findex.js

@oliviertassinari oliviertassinari added new feature New feature or request component: chip This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process labels Sep 6, 2018
@aretheregods
Copy link
Contributor

@oliviertassinari @hidai I'd like to tackle this issue. I'm thinking it'd be better to use an "icon" attribute as opposed to extending the avatar attribute. That's more explicit and I imagine a little easier to code, easier to deal with from a testing standpoint and also a little more modular. What do you think?

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 11, 2018

@aretheregods I had the same thought. I think that adding an icon property has potential.

On a side note, here is a quick workaround:

capture d ecran 2018-09-11 a 23 10 52

      <Chip
        label={
          <React.Fragment>
            <FaceIcon style={{ marginRight: 8, marginLeft: -8 }} />
            Basic Chip
          </React.Fragment>
        }
      />

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 11, 2018

@fzaninotto Regarding your icon positioning issue. I just had a look at vuetify, they are doing something interesting https://vuetifyjs.com/en/components/chips#introduction.

    <v-chip color="orange" text-color="white">
      Premium
      <v-icon right>star</v-icon>
    </v-chip>

    <v-chip color="primary" text-color="white">
      <v-icon left>cake</v-icon>
      1 Year
    </v-chip>

What do you think of adding a property like this to the Icon & SvgIcon component?:

placement?: 'start' | 'end';

Alternatively, I really like what styled-system is doing. It's a great source of inspiration for more Material-UI utilities component, like a Spacing or something that can be composed into a Box.

@aretheregods
Copy link
Contributor

aretheregods commented Sep 16, 2018

@oliviertassinari I made added some icon stuff and wanted to get your opinion of what changes I should make (I haven't added tests yet, I know), I made a pull request: #12881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: chip This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants