fix(input): Add pure-CSS floating label logic that will work on... #8491
Conversation
googlebot
added
the
cla: yes
...server-side-rendered pages.
| .mat-input-server:focus + .mat-form-field-placeholder-wrapper .mat-form-field-placeholder { | ||
| @include _mat-form-field-placeholder-floating( | ||
| $subscript-font-scale, $infix-padding, $infix-margin-top); | ||
| } |
There was a problem hiding this comment.
Can't this one be combined with the other one above?
(&.mat-form-field-should-float .mat-form-field-placeholder)
| width: (100% / $font-scale) + 0.0001; | ||
| } | ||
|
|
||
| @mixin _mat-form-field-placeholder-float-nodedupe2($font-scale, $infix-padding, $infix-margin-top) { |
There was a problem hiding this comment.
Instead of adding another mixin, I realized we could programatically change the rule each time it's stamped out, e.g.:
$uniqueId: 0;
@mixin cool-style() {
content: 'Hello' + $uniqueId;
$uniqueId: $uniqueId + 1 !global;
}
.one {
@include cool-style(); // content: "Hello0";
}
.two {
@include cool-style(); // content: "Hello1";
}
src/lib/input/input.ts
Outdated
| @@ -162,7 +168,8 @@ export class MatInput implements MatFormFieldControl<any>, OnChanges, OnDestroy, | |||
| @Optional() protected _parentForm: NgForm, | |||
| @Optional() protected _parentFormGroup: FormGroupDirective, | |||
| private _defaultErrorStateMatcher: ErrorStateMatcher, | |||
| @Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any) { | |||
| @Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any, | |||
| @Optional() @Inject(PLATFORM_ID) platformId?: Object) { | |||
There was a problem hiding this comment.
This class already injects Platform from cdk/platform, you could just use !this._platform.isBrowser
There was a problem hiding this comment.
Platform just checks if the document exists and has type object. This probably won't work now that universal gives us a domino document right?
We probably need to update the Platform.isBrowser logic, but I didn't want to do it in this PR.
There was a problem hiding this comment.
I'd be hesitant to change it now because it could be a breaking change
There was a problem hiding this comment.
That's why I left it out of this PR, but I do think we have to change it in the future. We need to audit the places where we're using this and check which ones are using it as a way to check if the document is defined and which ones really care that we're on the server. The latter checks are now incorrect due to Angular's changes. (The one I added in this PR for example, would be broken if I depended on Platform.isBrowser)
edit: according to @crisbeto Universal doesn't actually simulate the global document variable, so Platform.isBrowser should still work. I'll change it to that, but I still think we should update Platform in the future to use the official method of checking, in case they ever do decide to simulate the document global.
|
comments addressed |
mmalerba
added
the
P2
jelbourn
added
pr: lgtm
action: merge
…ngular#8491) * fix(input): Add pure-CSS floating label logic that will work on... ...server-side-rendered pages. * address comments
|
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. |
...server-side-rendered pages.
fixes #8247