feat(material-experimental): initial prototype of mdc-form-field #17903

devversion · 2019-12-09T20:44:53Z

Creates the initial prototype of form-field based on MDC text-field.

A few things that required some special handling in terms of extra CSS:

MDC text-field bottom-lines are limited to input and textarea. We want them to work always and to also cover prefixes and suffixes.
MDC text-field does not have a real standard appearance that works well for inputs and textarea's.
MDC text-field relies on special class being applied if textarea is the control. This doesn't match our model of "abstract" form field controls (so we needed custom handling; see SCSS comments)

Based on demo comparisons and unit test comparisons there are no breaking changes in terms of API and features, except that prefixes and suffixes do not work properly in the outline appearance. This requires some more thoughts and most likely upstream changes in MDC text-field. I could imagine this being something we can tackle in follow-ups's as the PR is already quite large.

jelbourn

I got about halfway through before I have to head out for the day; will try to finish tomorrow

Can you file an issue on the MDC repo for the challenges with prefix/suffix? I know their team is planning on supporting this soon, so we should make our requirements known

cc @abhiomkar

src/material-experimental/mdc-form-field/BUILD.bazel

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

src/material-experimental/mdc-form-field/_mdc-form-field.scss

src/material-experimental/mdc-form-field/directives/error.ts

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

src/material-experimental/mdc-form-field/form-field.ts

devversion · 2019-12-12T16:59:29Z

@jelbourn I created material-components/material-components-web#5326 for the outline appearance prefix/suffix issue (and linked to it in the Sass where needed)

jelbourn

I noticed that the error text is a bit darker- is that intentional?

I also noticed that the text color inside a toolbar isn't contrasted; was that built in before or was it part of the demo?

src/material-experimental/mdc-form-field/_mdc-text-field-structure-overwrites.scss

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

src/material-experimental/mdc-form-field/form-field.ts

jelbourn · 2019-12-12T23:09:08Z

src/material-experimental/mdc-form-field/form-field.ts

+   * Sets the list of element IDs that describe the child control. This allows the control to update
+   * its `aria-describedby` attribute accordingly.
+   */
+  private _syncDescribedByIds() {


Would using AriaDescriber simplify this method at all?

I don't think so. We want to use existing DOM elements and the actual custom form-field control is responsible for setting them. We just pass the hint/error id's down to the registered control.

src/material-experimental/mdc-input/input.ts

mmalerba

Still reviewing, but here's a couple comments I had so far

src/material-experimental/mdc-form-field/_form-field-standard-appearance.scss

src/material-experimental/mdc-form-field/_form-field-bottom-line.scss

src/material-experimental/mdc-form-field/_mdc-text-field-structure-overwrites.scss

devversion · 2019-12-13T15:15:54Z

For reference: Theming and colors are inaccurate since we do not have feature targeting yet. Without feature targeting I cannot wrap the MDC text-field styles in the theming mixin (limitation of Sass)

devversion · 2019-12-16T19:01:14Z

@jelbourn @mmalerba I addressed all of the feedback. Can you please have another look?

mmalerba

I hope that we'll be able to make some changes in MDC over time to reduce the number of workaround needed, but this looks good to me as a first pass.

Also, amazing work on this! Thank you for the detailed comments and explanations!

jelbourn

LGTM

Your comments in this PR are great; they could be in a textbook about good comments.

abhiomkar · 2019-12-16T20:52:18Z

@jelbourn Thanks for bringing this up! I sent you a note internally for potential changes to text field for prefix / suffix support.

Initial prototype of form-field based on MDC text-field.

Address feedback

…orm-field Address feedback 2

Address feedback

…lar#17903) * feat(material-experimental): initial prototype of mdc-form-field Initial prototype of form-field based on MDC text-field. * fixup! feat(material-experimental): initial prototype of mdc-form-field Address feedback * Use MDC text-field variables as much as possible. * fixup! fixup! feat(material-experimental): initial prototype of mdc-form-field Address feedback 2 * fixup! feat(material-experimental): initial prototype of mdc-form-field Address feedback

angular-automatic-lock-bot · 2020-02-07T00:02:30Z

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.}

devversion requested review from jelbourn, mmalerba and a team as code owners December 9, 2019 20:44

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 9, 2019

devversion force-pushed the feat/mdc-input branch 6 times, most recently from bb8d163 to 9bce312 Compare December 10, 2019 14:42

devversion assigned mmalerba Dec 10, 2019

devversion added the new component This issue is tracking a new component to be added to the library label Dec 10, 2019

devversion force-pushed the feat/mdc-input branch from 9bce312 to 4b258ac Compare December 10, 2019 15:09

jelbourn reviewed Dec 12, 2019

View reviewed changes

devversion force-pushed the feat/mdc-input branch from 4b258ac to 136d6c5 Compare December 12, 2019 16:10

jelbourn reviewed Dec 12, 2019

View reviewed changes

mmalerba reviewed Dec 13, 2019

View reviewed changes

devversion force-pushed the feat/mdc-input branch 2 times, most recently from 631fd63 to c728167 Compare December 16, 2019 18:36

mmalerba approved these changes Dec 16, 2019

View reviewed changes

mmalerba added the pr: lgtm label Dec 16, 2019

jelbourn approved these changes Dec 16, 2019

View reviewed changes

jelbourn added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Dec 16, 2019

devversion added 2 commits January 6, 2020 13:06

feat(material-experimental): initial prototype of mdc-form-field

57f8f5f

Initial prototype of form-field based on MDC text-field.

fixup! feat(material-experimental): initial prototype of mdc-form-field

f7f9b27

Address feedback

devversion added 2 commits January 6, 2020 13:06

Use MDC text-field variables as much as possible.

000e443

fixup! fixup! feat(material-experimental): initial prototype of mdc-f…

923196b

…orm-field Address feedback 2

devversion force-pushed the feat/mdc-input branch from 28f6c54 to 0940afe Compare January 6, 2020 12:06

fixup! feat(material-experimental): initial prototype of mdc-form-field

09d5790

Address feedback

devversion force-pushed the feat/mdc-input branch from 0940afe to 09d5790 Compare January 6, 2020 12:29

mmalerba merged commit 697c3a0 into angular:master Jan 7, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(material-experimental): initial prototype of mdc-form-field #17903

feat(material-experimental): initial prototype of mdc-form-field #17903

devversion commented Dec 9, 2019 •

edited

jelbourn left a comment •

edited

devversion commented Dec 12, 2019

jelbourn left a comment

jelbourn Dec 12, 2019

devversion Dec 13, 2019

mmalerba left a comment

devversion commented Dec 13, 2019

devversion commented Dec 16, 2019

mmalerba left a comment

jelbourn left a comment

abhiomkar commented Dec 16, 2019

angular-automatic-lock-bot bot commented Feb 7, 2020

feat(material-experimental): initial prototype of mdc-form-field #17903

feat(material-experimental): initial prototype of mdc-form-field #17903

Conversation

devversion commented Dec 9, 2019 • edited

jelbourn left a comment • edited

Choose a reason for hiding this comment

devversion commented Dec 12, 2019

jelbourn left a comment

Choose a reason for hiding this comment

jelbourn Dec 12, 2019

Choose a reason for hiding this comment

devversion Dec 13, 2019

Choose a reason for hiding this comment

mmalerba left a comment

Choose a reason for hiding this comment

devversion commented Dec 13, 2019

devversion commented Dec 16, 2019

mmalerba left a comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

abhiomkar commented Dec 16, 2019

angular-automatic-lock-bot bot commented Feb 7, 2020

devversion commented Dec 9, 2019 •

edited

jelbourn left a comment •

edited