fix(notched-outline): Fix label overflow #4171
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4171 +/- ##
=========================================
- Coverage 98.84% 98.5% -0.34%
=========================================
Files 127 127
Lines 5629 5631 +2
Branches 747 746 -1
=========================================
- Hits 5564 5547 -17
- Misses 65 84 +19
Continue to review full report at Codecov.
|
| @@ -85,7 +85,13 @@ class MDCNotchedOutline extends MDCComponent { | |||
| /** @type {!MDCNotchedOutlineAdapter} */ (Object.assign({ | |||
| addClass: (className) => this.root_.classList.add(className), | |||
| removeClass: (className) => this.root_.classList.remove(className), | |||
| setNotchWidthProperty: (width) => this.notchElement_.style.setProperty('width', width > 0 ? width + 'px' : '0'), | |||
| setNotchWidthProperty: (width) => { | |||
There was a problem hiding this comment.
Should this be handled in the foundation and exposed as 2 adapter APIs rather than making this one more complex (and less intuitive given its name)?
This is kind of a breaking change regardless of whether we make it 2 APIs or change the behavior of the 1. And if we change it here, we need to document the API's expected behavior. IMO splitting it is simpler to understand by wrapper libraries.
There was a problem hiding this comment.
I'll split this up into 2 API's and move the logic to the foundation.
| @@ -55,13 +55,14 @@ test('adapter#addClass adds a class to the root element', () => { | |||
| test('adapter#removeClass removes a class to the root element', () => { | |||
| const {root, component} = setupTest(); | |||
| component.getDefaultFoundation().adapter_.removeClass('foo'); | |||
| assert.isFalse(root.classList.contains('foo')); | |||
| component.getDefaultFoundation().adapter_.removeNotchWidthProperty(); | |||
There was a problem hiding this comment.
Is the width property actually set on this element before these statements run? Otherwise we're not really verifying anything.
|
|
||
| &--upgraded .mdc-floating-label--float-above { | ||
| max-width: calc(100% / .75); | ||
| text-overflow: clip; |
There was a problem hiding this comment.
(Discussed in DM) Is it safe to apply this even when not upgraded, to avoid the ellipsis appearing then disappearing once JS runs?
|
🤖 Beep boop! Screenshot test report 🚦4 screenshots changed from Details4 Added: |
|
🤖 Beep boop! Screenshot test report 🚦4 screenshots changed from Details4 Added: |
fixes: #4170
The new notched outline allowed the label to overflow the parent elements, so this causes the label to ellipse when down and to clip when floating.
BREAKING CHANGE: Adds a new adapter API, removeNotchWidthProperty. Also changes the vanilla implementation of MDCFloatingLabelAdapter#getWidth to return scrollWidth rather than offsetWidth.