fix(material-experimental/form-field): fix notch width after appearance change - #19682
Conversation
andrewseguin
added
P2
googlebot
added
the
cla: yes
| if (this._needsOutlineNotchWidthRefresh) { | ||
| this._refreshOutlineNotchWidth(); | ||
| this._changeDetectorRef.detectChanges(); | ||
| this._needsOutlineNotchWidthRefresh = false; |
There was a problem hiding this comment.
Couldn't we do this either inside the appearance setter or in ngOnChanges?
There was a problem hiding this comment.
ngOnChanges probably would have been a better place for this - but removed due to Paul's suggestion
| const fixture = createComponent(MatInputWithAppearance); | ||
| fixture.detectChanges(); | ||
|
|
||
| expect(document.querySelector('.mdc-notched-outline__notch')).toBe(null); |
There was a problem hiding this comment.
It might be better to use fixture.nativeElement.querySelector. Querying it off the document could fail if we have some other test that didn't clean up after itself. The same goes for the other two querySelector calls below.
There was a problem hiding this comment.
Good to know - I wondered why we might not be using document and that makes sense
| document.querySelector('input')!.focus(); | ||
| fixture.detectChanges(); | ||
|
|
||
| expect(notch.style.width).not.toBe(''); |
There was a problem hiding this comment.
Maybe this should be toBeTruthy? Otherwise this would pass for null or undefined.
| if (this._appearance === 'outline' && this._appearance !== oldValue) { | ||
| // After the view changes the appearance to outline, the label's width should be measured | ||
| // so that it can be provided to the notched outline. | ||
| this._needsOutlineNotchWidthRefresh = true; |
There was a problem hiding this comment.
This worked before: e3413ba, but we just regressed due those changes. It surprised me initially that those changes had an effect here, but since we had the asterisk rendered through an ngIf template, the CDK content observer triggered and updated the notch width. This is no longer the case as of e3413ba, so the notched outline no longer works.
I think the right fix would be to just explicitly call _refreshOutlineNotchWidth here, and fixing that the label scales to full width unnecessarily (causing incorrect measurement in filled). We stretch it unnecessarily due to our CSS overrides where we set left: 0 and right: 0. We can set those to initial.
There was a problem hiding this comment.
That works much better - glad to know that initial is valid so we can measure the right width immediately
There was a problem hiding this comment.
Switched directly to using left and right's default value, auto, since initial is blocklisted due to IE
andrewseguin
added
P1
|
Marking as P1 because this is currently causing our internal Google3 tests to fail |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
5 participants
When the form field initializes, it will determine the floating label notch width if it is
outlineappearance. This is done once at initialization.If the form field initializes with an appearance other than
outline, it does not determine the correct width.This change measures the correct width after the appearance is changed to
outlineand provides that input to the notched outline.