[Chip] Support overflow ellipsis by default

[rbarkhouse]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

My chips might contain very long strings (e.g. "Antigonish_NOVA_SCOTIA_CANADA_A1B2C3_7b2b3b4b1b2b3ffba3b4b5cd_ZZZ"), so I would like to limit the width of the chip, and show the full text in a Tooltip.

However after setting a maxWidth, and customizing the overflow CSS, I unexpectedly see:

• a truncated section from the middle of the string
• no ellipsis
• the Chip's delete button has disappeared

  chip: {
    maxWidth: 100,
    whiteSpace: "nowrap",
    overflow: "hidden",
    textOverflow: "ellipsis"
  }

If I instead use "withStyles" and create my own StyledChip, and override the "label" portion of the styling, it is almost there, but I still don't see the ellipsis, and now I've lost the left-padding on the delete button:

const StyledChip = withStyles(theme => ({
  root: {
    maxWidth: 100
  },
  label: {
    whiteSpace: "nowrap",
    overflow: "hidden",
    textOverflow: "ellipsis"
  }
}))(Chip);

Expected Behavior 🤔

By setting a maxWidth, overflow:hidden and textOverflow:ellipsis, I expected to see a narrower Chip, displaying the first x characters of the string, then an ellipsis... and the delete button should remain visible.

Steps to Reproduce 🕹

I modified the Chips example from Material-UI docs to demonstrate the issue:
https://codesandbox.io/s/material-demo-zt72h

Context 🔦

My chips might contain very long strings, so I would like to limit the width of the chip, and show the full text in a Tooltip.

Your Environment 🌎

Tech	Version
Material-UI	v4.7.0
React	v16.12.0
Browser	Chrome
TypeScript
etc.

[gmltA]

@rbarkhouse, working on the same thing right now. You need to set label display to inline-block for ellipsis to work properly.

[oliviertassinari]

Display flex containers can't overflow ellipse, I believe it's a limitation of the platform.

In our case, it doesn't seem that we need the label to be a flex container, as the parent already handle vertical alignment:

diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js
index 70ea4c0b6..3c5a41679 100644
--- a/packages/material-ui/src/Chip/Chip.js
+++ b/packages/material-ui/src/Chip/Chip.js
@@ -199,8 +199,6 @@ export const styles = theme => {
     },
     /* Styles applied to the label `span` element`. */
     label: {
-      display: 'flex',
-      alignItems: 'center',
       paddingLeft: 12,
       paddingRight: 12,
       whiteSpace: 'nowrap',

@mbrookes What do you think about the changes?

[mbrookes]

diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js
index 70ea4c0b6..3c5a41679 100644
--- a/packages/material-ui/src/Chip/Chip.js
+++ b/packages/material-ui/src/Chip/Chip.js
@@ -199,8 +199,6 @@ export const styles = theme => {
     },
     /* Styles applied to the label `span` element`. */
     label: {
-      display: 'flex',
-      alignItems: 'center',
+      overflow: 'hidden',
+      text-overflow: 'ellipsis',
       paddingLeft: 12,
       paddingRight: 12,
       whiteSpace: 'nowrap',

So that just setting max-width on root works?

[oliviertassinari]

@rbarkhouse Do you want to work on the changes proposed by @mbrookes?

[suliskh]

Is there any ongoing work or PR? If no, I would be happy to contribute to this issue

[oliviertassinari]

@suliskh I think that you are free to go :)

[suliskh]

Thank you! Working on it

[oliviertassinari]

@suliskh Did you make some progress?

[suliskh]

PR #18708 submitted. It's my first PR, and it would be very kind of you if you guide me along.

Thank you very much!