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

Fields without description are losing focus when an error is shown for the first time. #2336

Closed
dompuiu opened this issue Sep 14, 2021 · 2 comments · Fixed by #2422
Closed

Comments

@dompuiu
Copy link
Member

dompuiu commented Sep 14, 2021

🐛 Bug Report

If you have a TextField without a description, the focus on the field is lost as soon as an error is shown for the first time. This probably happens for all the fields that have Helper Text option.

🤔 Expected Behavior

Field should keep focus.

😯 Current Behavior

Field is losing the focus when the error is shown.

💁 Possible Solution

Probably use a useLayoutEffect to restore focus to the input if the input had focus previously.

🔦 Context

💻 Code Sample

https://codesandbox.io/s/lost-focus-44wtq

🌍 Your Environment

Software Version(s)
react-spectrum 3.14.0
Browser Chrome
Operating System macOS 11.6

🧢 Your Company/Team

🕷 Tracking Issue (optional)

@dompuiu
Copy link
Member Author

dompuiu commented Sep 15, 2021

The possible solution listed in the description was proposed by @snowystinger during a Slack conversation. That won't prevent the field to be rerendered which is not very efficient. On top of that, when the focus need to be restored, the cursor needs to be added at the right position since it is not always at the end of the text from the field.

A possible alternative would be to render always the helper text container and have a height of 0 when it is empty.

@snowystinger
Copy link
Member

Ah, yeah, that cursor issue is annoying. We should have access to it on the ref though through ref.current.selectionStart, but seems like we'd be reinventing the wheel. Might be better to just render the wrapper, however, it does carry some problems when put into a Form. See this issue:
#2324
Given that Forms are already known to have layout issues though, it may be best to just add the wrapper.
@devongovett @dannify

@LFDanLu LFDanLu added this to Not Groomed in Todo (out of date) via automation Sep 22, 2021
majornista added a commit that referenced this issue Oct 4, 2021
… is shown for the first time.

🐛 Bug Report
If you have a TextField without a description, the focus on the field is lost as soon as an error is shown for the first time. This probably happens for all the fields that have Helper Text option.

🤔 Expected Behavior
Field should keep focus.

😯 Current Behavior
Field is losing the focus when the error is shown.

💁 Possible Solution
Probably use a useLayoutEffect to restore focus to the input if the input had focus previously.

🔦 Context
💻 Code Sample
https://codesandbox.io/s/lost-focus-44wtq

🌍 Your Environment
Software	Version(s)
react-spectrum	3.14.0
Browser	Chrome
Operating System	macOS 11.6
majornista added a commit that referenced this issue Oct 5, 2021
… is shown for the first time.

🐛 Bug Report
If you have a TextField without a description, the focus on the field is lost as soon as an error is shown for the first time. This probably happens for all the fields that have Helper Text option.

🤔 Expected Behavior
Field should keep focus.

😯 Current Behavior
Field is losing the focus when the error is shown.

💁 Possible Solution
Probably use a useLayoutEffect to restore focus to the input if the input had focus previously.

🔦 Context
💻 Code Sample
https://codesandbox.io/s/lost-focus-44wtq

🌍 Your Environment
Software	Version(s)
react-spectrum	3.14.0
Browser	Chrome
Operating System	macOS 11.6
majornista added a commit that referenced this issue Oct 5, 2021
… is shown for the first time.

🐛 Bug Report
If you have a TextField without a description, the focus on the field is lost as soon as an error is shown for the first time. This probably happens for all the fields that have Helper Text option.

🤔 Expected Behavior
Field should keep focus.

😯 Current Behavior
Field is losing the focus when the error is shown.

💁 Possible Solution
Probably use a useLayoutEffect to restore focus to the input if the input had focus previously.

🔦 Context
💻 Code Sample
https://codesandbox.io/s/lost-focus-44wtq

🌍 Your Environment
Software	Version(s)
react-spectrum	3.14.0
Browser	Chrome
Operating System	macOS 11.6
@LFDanLu LFDanLu removed this from Not Groomed in Todo (out of date) Oct 6, 2021
devongovett pushed a commit that referenced this issue Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants