fix(material-experimental/mdc-form-field): fix outline notch width #23005

devversion · 2021-06-17T15:29:14Z

This commit fixes a couple of issues that have been introduced
by accident over time:

The outline notch width is incorrectly calculated since fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676. This happens because we manually override
the label's typography level to body1 but didn't account for the font-size set
by the notched-outline. This resulted in the computed label width being incorrect.
fix(material-experimental/mdc-form-field): fix height for form field … #22089 added line-height: 0 to fix textarea's accidentally expanding the infix vertically.
This breaks custom controls and their alignment. Textarea's seem to behave a
little different compared to a native input, and the actual fix seems
to be to fix the alignment from top to middle so that the infix
does not try to add additional padding (due to the inherited body1
line-height and to satisfy min-height). Example:
https://jsfiddle.net/rpvm4bkL/6/.
Typography has been moved with
fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676 to the
input control (notice how font: inherit is removed too). This
breaks custom form controls that rely on the typography provided
by the form-field (these controls would just inherit typography)
Not an actual fix, but a cleanup since 21ab17f
added a class to avoid styles from the form-field leaking into
standalone inputs. The class has been updated to be more specific that
it only matches form-control inputs, and not any type of form
control.

Also cleans up logic for the label offset computation. Instead of hard-coding values extracted from MDC, we should have a constant describing where this value comes from. Lastly, deletes a .swp file that has sneaked into the repo.

mmalerba · 2021-06-17T16:30:19Z

src/material-experimental/mdc-form-field/_mdc-text-field-textarea-overrides.scss

+    // Set the vertical alignment for textareas inside form fields to be the middle. This
+    // ensures that textareas do not stretch the infix container vertically without having
+    // multiple rows of text. See: https://github.com/angular/components/pull/22089.
+    vertical-align: middle;


Are you sure this doesn't mess up prefix/suffix icon alignment?

I've tested with a text suffix/prefix and icons, looks good. Here is a screenshot for you to verify:

It looks like the icon aligns in the middle vertically if the textarea is resized to be larger; but that seems unrelated to this change, and I'm not sure what is expected here

Is it just me or are there some extra lines in that screenshot?

This is unrelated to this PR (can be observed in Miles' form-field demo too; https://mmalerba-demo-1.web.app/mdc-input). I'll have a look at this in a separate PR; it's also observable on the MDC textfield itself but not as obvious.

Yeah I've noticed that extra line before too, it comes from the MDC styles. I remember having a similar issue back when I was writing our outline form field too.

crisbeto

LGTM, but leaving to Miles for final approval.

mmalerba

LGTM

mmalerba · 2021-07-12T20:35:38Z

@devversion needs rebase

devversion · 2021-07-12T21:59:27Z

@mmalerba Done.

wagnermaciel · 2021-07-13T18:40:50Z

I believe the additional padding is causing the text area to overflow

Before

After

This is causing a lot of screenshot test failures internally

wagnermaciel · 2021-07-13T21:28:56Z

Did some more digging. Turns out there used to be resize: none on the textareas coming from cdk/text-field/_index.scss. They are now getting resize: auto from material-experimental/mdc-form-field/_mdc-text-field-textarea-overrides.scss. That plus some of the other styles from there are probably what's breaking the screenshot tests

devversion · 2021-07-13T21:35:29Z

@wagnermaciel Nice findings. It makes sense that this happens now because our MDC styles for the textarea have increased in specificity. I'd need to look how we can make this work. It seems like even with same specificity of the CDK textarea styles this is prone to break.

wagnermaciel · 2021-07-13T21:47:56Z

Does that mean this an expected failure? Is this how the text input is meant to look now?

devversion · 2021-07-14T13:57:35Z

@wagnermaciel It's not expected. I didn't realize this was happening because our examples of the form-field are quite large. Updated the PR to let cdk-textarea-autosize override the resize CSS property to none. Should be good for another presubmit.

mmalerba · 2021-07-22T17:16:14Z

src/material-experimental/mdc-form-field/_form-field-theme.scss

+    // MDC uses `subtitle1` for the input value, placeholder and floating label. The spec
+    // shows `body1` for text fields though, so we manually override the typography.
+    // Note: Form controls inherit the typography from the parent form field.
+    .mat-mdc-form-field,


I think changing these selectors broke some of the typography. I'm seeing a few screenshots in g3 where the font changed. Can we change them back?

This commit fixes a couple of issues that have been introduced by accident over time: 1. The outline notch width is incorrectly calculated since angular#21676. This happens because we manually override the label's typography level to `body1` but didn't account for the `font-size` set by the `notched-outline`. This resulted in the computed label width being incorrect. 2. angular#22089 added `line-height: 0` to fix textarea's accidentally expanding the infix vertically. This breaks custom controls and their alignment. Textarea's behave a little different compared to a native input, and the actual fix seems to be to fix the alignment from `top` to `middle` so that the infix does not try to add additional padding (due to the inherited `body1` line-height and to satisfy `min-height`). Example: https://jsfiddle.net/rpvm4bkL/6/. 3. Typography has been moved with angular#21676 to the input control (notice how `font: inherit` is removed too). This breaks custom form controls that rely on the typography provided by the form-field (these controls would just inherit typography) 4. Not an actual fix, but a cleanup since 21ab17f added a class to avoid styles from the form-field leaking into standalone inputs. The class has been updated to be more specific that it only matches form-control inputs, and not _any_ type of form control.

…ine-height: normal` on the floating label

…23005) * fix(material-experimental/mdc-form-field): fix outline notch width This commit fixes a couple of issues that have been introduced by accident over time: 1. The outline notch width is incorrectly calculated since #21676. This happens because we manually override the label's typography level to `body1` but didn't account for the `font-size` set by the `notched-outline`. This resulted in the computed label width being incorrect. 2. #22089 added `line-height: 0` to fix textarea's accidentally expanding the infix vertically. This breaks custom controls and their alignment. Textarea's behave a little different compared to a native input, and the actual fix seems to be to fix the alignment from `top` to `middle` so that the infix does not try to add additional padding (due to the inherited `body1` line-height and to satisfy `min-height`). Example: https://jsfiddle.net/rpvm4bkL/6/. 3. Typography has been moved with #21676 to the input control (notice how `font: inherit` is removed too). This breaks custom form controls that rely on the typography provided by the form-field (these controls would just inherit typography) 4. Not an actual fix, but a cleanup since 21ab17f added a class to avoid styles from the form-field leaking into standalone inputs. The class has been updated to be more specific that it only matches form-control inputs, and not _any_ type of form control. * fix(material-experimental/mdc-form-field): increase specificity of `line-height: normal` on the floating label Co-authored-by: Miles Malerba <mmalerba@google.com> (cherry picked from commit 04d2aaa)

angular-automatic-lock-bot · 2021-10-15T00:02:28Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 17, 2021

devversion changed the title ~~fix(material-experimental/mdc-form-fiel): fix outline notch width~~ fix(material-experimental/mdc-form-field): fix outline notch width Jun 17, 2021

devversion force-pushed the fix/form-field-fixes-mdc branch from 280e279 to 1bf9ac9 Compare June 17, 2021 15:29

devversion requested review from mmalerba and crisbeto June 17, 2021 15:36

devversion added the target: patch This PR is targeted for the next patch release label Jun 17, 2021

devversion marked this pull request as ready for review June 17, 2021 15:36

mmalerba reviewed Jun 17, 2021

View reviewed changes

crisbeto approved these changes Jun 18, 2021

View reviewed changes

mmalerba approved these changes Jun 18, 2021

View reviewed changes

mmalerba added the action: merge The PR is ready for merge by the caretaker label Jun 18, 2021

devversion added the P2 The issue is important to a large percentage of users, with a workaround label Jun 24, 2021

devversion force-pushed the fix/form-field-fixes-mdc branch 2 times, most recently from 4882d9f to cc8bb0e Compare July 6, 2021 13:11

devversion mentioned this pull request Jul 9, 2021

fix(material-experimental/mdc-form-field): add workaround for incorrect MDC label size #23137

Closed

mmalerba added the G This is is related to a Google internal issue label Jul 12, 2021

devversion force-pushed the fix/form-field-fixes-mdc branch from cc8bb0e to d0f2900 Compare July 12, 2021 21:58

devversion force-pushed the fix/form-field-fixes-mdc branch from d0f2900 to 74ac140 Compare July 13, 2021 14:07

devversion force-pushed the fix/form-field-fixes-mdc branch from 74ac140 to 23ad76a Compare July 14, 2021 13:54

mmalerba reviewed Jul 22, 2021

View reviewed changes

zarend added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jul 29, 2021

devversion force-pushed the fix/form-field-fixes-mdc branch from 23ad76a to 4384745 Compare August 17, 2021 10:07

mmalerba mentioned this pull request Sep 2, 2021

MDC-based form-field doesn't inherit custom typography #23509

Open

devversion force-pushed the fix/form-field-fixes-mdc branch from 4384745 to 707efb6 Compare September 10, 2021 21:16

mmalerba removed the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Sep 14, 2021

fix(material-experimental/mdc-form-field): increase specificity of `l…

aac6264

…ine-height: normal` on the floating label

mmalerba force-pushed the fix/form-field-fixes-mdc branch from b9ef72f to aac6264 Compare September 14, 2021 02:56

andrewseguin merged commit 04d2aaa into angular:master Sep 14, 2021

sync-by-unito bot mentioned this pull request Oct 1, 2021

build(deps): bump @angular/material from 12.2.4 to 12.2.8 numbersprotocol/capture-lite#925

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(material-experimental/mdc-form-field): fix outline notch width #23005

fix(material-experimental/mdc-form-field): fix outline notch width #23005

devversion commented Jun 17, 2021

mmalerba Jun 17, 2021

devversion Jun 17, 2021 •

edited

crisbeto Jun 17, 2021

devversion Jun 17, 2021 •

edited

mmalerba Jun 18, 2021

crisbeto left a comment

mmalerba left a comment

mmalerba commented Jul 12, 2021

devversion commented Jul 12, 2021

wagnermaciel commented Jul 13, 2021

wagnermaciel commented Jul 13, 2021

devversion commented Jul 13, 2021

wagnermaciel commented Jul 13, 2021

devversion commented Jul 14, 2021

mmalerba Jul 22, 2021

angular-automatic-lock-bot bot commented Oct 15, 2021

fix(material-experimental/mdc-form-field): fix outline notch width #23005

fix(material-experimental/mdc-form-field): fix outline notch width #23005

Conversation

devversion commented Jun 17, 2021

mmalerba Jun 17, 2021

Choose a reason for hiding this comment

devversion Jun 17, 2021 • edited

Choose a reason for hiding this comment

crisbeto Jun 17, 2021

Choose a reason for hiding this comment

devversion Jun 17, 2021 • edited

Choose a reason for hiding this comment

mmalerba Jun 18, 2021

Choose a reason for hiding this comment

crisbeto left a comment

Choose a reason for hiding this comment

mmalerba left a comment

Choose a reason for hiding this comment

mmalerba commented Jul 12, 2021

devversion commented Jul 12, 2021

wagnermaciel commented Jul 13, 2021

Before

After

wagnermaciel commented Jul 13, 2021

devversion commented Jul 13, 2021

wagnermaciel commented Jul 13, 2021

devversion commented Jul 14, 2021

mmalerba Jul 22, 2021

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Oct 15, 2021

devversion Jun 17, 2021 •

edited

devversion Jun 17, 2021 •

edited