Skip to content

Change semantics of shouldRequestFocus #1049

@grundid

Description

@grundid

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:

  1. shouldRequestFocus should be true by default
  2. requestFocus should not call Scrollable.ensureVisible
  3. Scrollable.ensureVisible might be called during validate if an optional parameter is set to true

What do you think?

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions