Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Check whether a focus state transition occurs#87

Open
zhang-peter wants to merge 2 commits intoubuntu-flutter-community:mainfrom
zhang-peter:dev
Open

Check whether a focus state transition occurs#87
zhang-peter wants to merge 2 commits intoubuntu-flutter-community:mainfrom
zhang-peter:dev

Conversation

Copy link

zhang-peter commented May 12, 2023 *
edited
Loading

Checking for valid state transitions is necessary because _handleFocusChanged is called when Spinbox are enabled or disabled.
I did not make this pull request when #80 occur because I am not sure what cause the problem.
Now I found that is relative to FocusNode behavior when propery canRequestFocus is changed in TextField. I believe this pull request can close #80.

jpnurmi reviewed May 12, 2023
Copy link
Member

jpnurmi left a comment

Choose a reason for hiding this comment

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

tracking "previous" state without dealing with didUpdateWidget() tends to result in things getting out of sync when the widget is rebuilt. what if we relied on FocusNode.canRequestFocus instead to avoid our own book keeping? it should be false if the widget is disabled and thus, not focusable, right?

if (_focusNode.canRequestFocus) widget.onSubmitted?.call(value);

Copy link
Author

zhang-peter commented May 12, 2023

tracking "previous" state without dealing with didUpdateWidget() tends to result in things getting out of sync when the widget is rebuilt. what if we relied on FocusNode.canRequestFocus instead to avoid our own book keeping? it should be false if the widget is disabled and thus, not focusable, right?

if (_focusNode.canRequestFocus) widget.onSubmitted?.call(value);

I will check sync about didUpdateWidget. But FocusNode.canRequestFocus condition only avoid submit action when disable Spinbox. Enable Spinbox can still trigger submit action.

Copy link
Member

jpnurmi commented May 12, 2023

Alternatively, we could propagate the "enabled" value from SpinBox and CupertinoSpinBox to BaseSpinBox or SpinBoxMixin so that it knows the value.

Copy link
Author

zhang-peter commented May 12, 2023

"enabled" value from SpinBox and CupertinoSpinBox also can't avoid submit action when enable a disabled Spinbox, because "enabled" is always true in this situation.

Copy link
Contributor

wszak commented May 12, 2023

I will check sync about didUpdateWidget. But FocusNode.canRequestFocus condition only avoid submit action when disable Spinbox. Enable Spinbox can still trigger submit action.

How about something like:

final oldValue = _value;
final newValue = fixupValue(_controller.text);
if (newValue != oldValue && _focusNode.canRequestFocus) widget.onSubmitted?.call(newValue);

So onSubmitted is called only when the value changes, like in setValue.
Change in focus doesn't change value, so it should work for your case too.

Copy link
Author

zhang-peter commented May 12, 2023

I will check sync about didUpdateWidget. But FocusNode.canRequestFocus condition only avoid submit action when disable Spinbox. Enable Spinbox can still trigger submit action.

How about something like:

final oldValue = _value;
final newValue = fixupValue(_controller.text);
if (newValue != oldValue && _focusNode.canRequestFocus) widget.onSubmitted?.call(newValue);

So onSubmitted is called only when the value changes, like in setValue. Change in focus doesn't change value, so it should work for your case too.

Actually, your proposition can cover most case. But a submit aciton is usually tiggered when an input widget lose focus or Enter key is pressed in other UI library, like Qt.
I think we should obey this convention for convenience.

Copy link
Author

zhang-peter commented May 13, 2023

Hi @jpnurmi , I don't think there will be a synchronization problem without dealing with didUpdateWidget() after viewing the code.
hasFocus is property of _focusNode which is final type. _focusNode is determined when state is initilized.
I also move initialization of _hasFocusPrev to initState in new commit. It seems more reasonable.

wszak pushed a commit to wszak/flutter_spinbox that referenced this pull request Aug 23, 2023
wszak referenced this pull request in wszak/flutter_spinbox Aug 23, 2023
wszak referenced this pull request in wszak/flutter_spinbox Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jpnurmi jpnurmi left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[propose] onSubmitted can be called when TextField loses focus and Spinbox is enabled.

3 participants