Skip to content
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

fix(material/form-field): outline label position #29123

Conversation

adumitrescu-plenty
Copy link
Contributor

Fixes the outline label position when a prefix is present and the form field is not yet rendered.

Fixes #29064

Fixes the outline label position when a prefix is present and the form field is not yet rendered.

Fixes angular#29064
@adumitrescu-plenty adumitrescu-plenty changed the title fix(form-field): outline label position fix(material/form-field): outline label position May 27, 2024
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

By the way, are we sure that it resolves #29064? The example there isn't using shadow DOM at all.

src/material/form-field/form-field.ts Outdated Show resolved Hide resolved
@adumitrescu-plenty
Copy link
Contributor Author

By the way, are we sure that it resolves #29064? The example there isn't using shadow DOM at all.

No but the actual check fix it by narrowing down the fact that is not yet rendered. Which is the root cause of the issue. I tried eliminating the possible places that it could be (shadow dom, document or itself) which left us with the case where the element is not yet rendered thus the functionality inside the form field that gets the width of the prefix will return 0 for the prefix container which is not yet rendered. Hopefully this makes sense.

src/material/form-field/form-field.ts Outdated Show resolved Hide resolved
src/material/form-field/form-field.ts Outdated Show resolved Hide resolved
src/material/form-field/form-field.ts Outdated Show resolved Hide resolved
@crisbeto crisbeto self-assigned this May 28, 2024
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed labels May 28, 2024
@crisbeto crisbeto merged commit eb22e2e into angular:main May 28, 2024
22 of 25 checks passed
crisbeto pushed a commit that referenced this pull request May 28, 2024
* fix(form-field): outline label position

Fixes the outline label position when a prefix is present and the form field is not yet rendered.

Fixes #29064

* fix(material/form-field): adjust text

* fix(material/form-field): requested changes

* fix(material/form-field): adjustments for shadow root

* fix(material/form-field): adjust shadow node check

* fix(material/form-field): adjust logic

Simplify the method

(cherry picked from commit eb22e2e)
crisbeto added a commit to crisbeto/material2 that referenced this pull request May 29, 2024
crisbeto added a commit that referenced this pull request May 29, 2024
crisbeto added a commit that referenced this pull request May 29, 2024
@crisbeto
Copy link
Member

This had to be rolled back, because it broke a test internally. I can't share much about it, aside from it being a form field inside of a sidenav that wasn't floating its label correctly anymore.

@adumitrescu-plenty
Copy link
Contributor Author

This had to be rolled back, because it broke a test internally. I can't share much about it, aside from it being a form field inside of a sidenav that wasn't floating its label correctly anymore.

I'll try to see why and fix it

@adumitrescu-plenty
Copy link
Contributor Author

@crisbeto any way I can help or test? Maybe we can try including the document check?

@crisbeto
Copy link
Member

I can't run the test myself, I've only seen the failing screenshot. It has a sidenav with some form fields in it and one of them has the label on top of the prefix, similarly to how it failed for me when the check was broken inside the shadow DOM.

@adumitrescu-plenty
Copy link
Contributor Author

@crisbeto I assume that is the case for any form field inside a side-nav. This is the case now as well. Check this https://stackblitz.com/edit/ad4b8f-dq4pmf?file=src%2Fexample%2Fsidenav-autosize-example.ts . I will try to see if I can fix this also

@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 Jul 1, 2024
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 merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(form-field): form field prefix in overlay
2 participants