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

Focus on TextField is lost if variant is changed on the fly #20821

Closed
2 tasks done
keul opened this issue Apr 28, 2020 · 5 comments
Closed
2 tasks done

Focus on TextField is lost if variant is changed on the fly #20821

keul opened this issue Apr 28, 2020 · 5 comments
Labels
out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future)

Comments

@keul
Copy link
Contributor

keul commented Apr 28, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

If the variant property of a TextField is changed on the fly the focus on the field is lost

Expected Behavior 🤔

Would be nice if focus is kept.

Steps to Reproduce 🕹

Go to https://odtfp.csb.app/

  1. type something in the field
  2. ...then type an _ char (which enable the error state)
  3. ...see focus being lost
  4. re-focus the field manually
  5. remove the _ (so exit the "error state"
  6. focus is lost again

Context 🔦

What I was trying to achieve is to have a better highlight of the field error so I'm switching from "standard" to "filled" variant on the fly in case of error.

I perfectly know that I can have the same visual behavior just using styles, and I'm going in this direction, but I was not sure if this should be threat as a minor bug or not, so I'm reporting this.

Your Environment 🌎

Tech Version
Material-UI v4.9.9
React 16.12.0
@oliviertassinari oliviertassinari added the out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) label Apr 28, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 28, 2020

@keul Thanks for the report, however, I don't think that we want to support this use case. It will increase the bundle size and distract us from more important problems.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 28, 2020

@marcosvega91 Regarding your proposed fix, it makes me think of #14132.

@marcosvega91
Copy link
Contributor

marcosvega91 commented Apr 28, 2020

I proposed a fix that if i think better has not sense. So I deleted It.

We can check if the variant is changed and the input has the current focus and trigger focus on the element on next render, but i think Is only a workaround and not a solution

@eps1lon
Copy link
Member

eps1lon commented Apr 28, 2020

I think for uncontrolled components the state would be lost as well. It would definitely be nice to be able to switch variants but the amount of work required isn't justified compared to the gain. Generally you don't switch variants since you only have a single variant per form anyway.

@keul
Copy link
Contributor Author

keul commented Apr 29, 2020

@eps1lon @oliviertassinari I updated the codesandox with an uncontrolled example and there's the same issue.
However I really don't find useful adding any complexity for supporting this edge case.
My report was only just in case it was hiding other issues. Changing the variant on the fly is not useful.

Thanks all for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future)
Projects
None yet
Development

No branches or pull requests

4 participants