-
- Notifications
You must be signed in to change notification settings - Fork 559
Description
Description
I would like to pick up on the discussion in #908 and suggest a change to the semantics of shouldRequestFocus. In the past we have seen many “A FocusNode was used after being disposed“ exceptions but I thing that the current fix with PR #1045 will solve this for good.
In PR #856 in the requestFocus() method Scrollable.ensureVisible(context) was added and this introduced new issues. In larger lists with checkboxes the content was jumping back and forth with each check of a checkbox. The semantics of Scrollable.ensureVisible are not clearly defined. As we all experienced this method can also move a context that is already visible and this is very distracting to the user. Also if a user is already interacting with a form field we can assume that it is visible. There are several open issues with this ensureVisible behavior, for example flutter-form-builder-ecosystem/form_builder_extra_fields#10. Here the user is complaining that the dropdown moves to the top because “focus on the form behave strange”. But it is not requestFocus that is causing the problem. It is ensureVisible.
For both issues the solution with shouldRequestFocus (=false) seems to fix all problems as it disables both requestFocus and ensureVisible. At the same time it disables the focus of a form field which is a very important part of every form. With focus disabled it is not possible to navigate through a form with a keyboard and if no focus is given to a checkbox or another form field it cannot hide a soft keyboard if a user was in a text field before that.
If I understand the idea behind PR #856 it was so a validation error is visible to a user. I think this is a good point but we should ensure that a validation error is visible only after calling validate(). During auto validation the user is already entering data into a form field so that we can assume that the field and the error message are visible. For this it might be helpful to extend the validate() method with an optional parameter "bool ensureValidationErrorVisible" that would if true call on the first form field with a validation error Scrollable.ensureVisible.
So my suggestion would be:
- shouldRequestFocus should be true by default
- requestFocus should not call Scrollable.ensureVisible
- Scrollable.ensureVisible might be called during validate if an optional parameter is set to true
What do you think?