Skip to content

fix(material-experimental/mdc-input): only apply styling when inside a form field #21876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 4, 2021

Conversation

crisbeto
Copy link
Member

Currently mat-form-field brings in the styles for MatInput. but the problem is that they target a class that is applied even to inputs that aren't inside a form field.

These changes aim to prevent CSS from bleeding out by only styling inputs inside a mat-form-field.

Fixes #21871.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note G This is is related to a Google internal issue target: patch This PR is targeted for the next patch release labels Feb 11, 2021
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 11, 2021
@crisbeto
Copy link
Member Author

Caretaker note: this has the potential of breaking existing apps. We should re-evaluate the fix if it affects a lot of g3 targets.

@crisbeto crisbeto force-pushed the 21871/input-styling branch from 5df18ab to fc105f5 Compare February 11, 2021 17:28
@mmalerba mmalerba added P2 The issue is important to a large percentage of users, with a workaround and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Feb 11, 2021
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Feb 11, 2021
@mmalerba
Copy link
Contributor

I'll try presubmitting it, if it proves too breaking we could at least try again with just the MDC components.

@mmalerba
Copy link
Contributor

broke ~200 targets, lets try just the MDC ones

@jelbourn
Copy link
Member

@mmalerba if we do end up submitting this with only the MDC version fixed, we'll need to update the docs to mention this difference in behavior.

@mmalerba
Copy link
Contributor

Alternatively we could just not do this at all. There's no real reason to use MatInput outside of a MatFormField, but I think some people are doing it just for the styles. Making the change would hopefully take away people's incentive to do that

@jelbourn
Copy link
Member

I do see the desire to do this specifically for matChipInput, though. I think it's valid to use a chip + input combination outside of a form-field.

@crisbeto
Copy link
Member Author

Are we sure that the presubmit is due to people using MatInput without a form field, rather than me messing something up while making the change? I did spend time testing it, but it's possible that I missed something.

An alternative could be to throw an error if MatInput isn't in a form field, but that'll likely be even more breaking.

@mmalerba
Copy link
Contributor

I think the main issue is that people are intentionally using it outside the form-field, simply as a way to clear the input styles (background, border, etc)

@crisbeto crisbeto force-pushed the 21871/input-styling branch from fc105f5 to d502206 Compare February 17, 2021 20:18
@crisbeto
Copy link
Member Author

I've changed the PR so it only affects the MDC version of the input.

@mmalerba mmalerba changed the title fix(material/input): only apply styling when inside a form field fix(material-experimental/mdc-input): only apply styling when inside a form field Feb 17, 2021
@crisbeto crisbeto force-pushed the 21871/input-styling branch 2 times, most recently from cf6df45 to c3e472a Compare February 20, 2021 08:22
@crisbeto crisbeto force-pushed the 21871/input-styling branch from c3e472a to 7c2c448 Compare February 28, 2021 14:26
@crisbeto crisbeto force-pushed the 21871/input-styling branch from 7c2c448 to 4a40ac4 Compare March 25, 2021 20:32
…a form field

Currently `mat-form-field` brings in the styles for `MatInput`. but the problem is that
they target a class that is applied even to inputs that aren't inside a form field.

These changes aim to prevent CSS from bleeding out by only styling inputs inside a `mat-form-field`.

Fixes angular#21871.
@crisbeto crisbeto force-pushed the 21871/input-styling branch from 4a40ac4 to f03c0ac Compare April 15, 2021 18:12
@annieyw annieyw added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels May 4, 2021
@annieyw annieyw merged commit 21ab17f into angular:master May 4, 2021
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement G This is is related to a Google internal issue merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note P2 The issue is important to a large percentage of users, with a workaround target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(input): only apply MatInput styles when inside of a form-field
5 participants