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.

Sign up for GitHub

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

Fix focus management for text fields #51009

Merged
merged 30 commits into from
Jun 20, 2024
Merged

Fix focus management for text fields #51009

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d2e4ccc
Make sure the elements can't be reached by keyboard
tugorez Apr 23, 2024
ea09d6e
Move the focus to the <flutter-view /> instead of blur.
tugorez Apr 23, 2024
edab1dc
Add listener to the subscription (otherwise never gets cleaned up).
tugorez May 10, 2024
8bbc206
Delay the input removal
tugorez May 10, 2024
ed2f851
Prevent default on pointer down on flutter views
tugorez May 10, 2024
78521ec
Format
tugorez May 10, 2024
72d3956
Add a test that checks focus goes from one input to another
tugorez May 10, 2024
7a6c596
Remove the blur listeners
tugorez May 13, 2024
11b7dae
Enable view focus binding
tugorez May 14, 2024
df75938
Disable view focus binding.
tugorez May 14, 2024
edfdacf
Add ios awaits
tugorez May 14, 2024
7daf2a1
Remove safari desktop delay
tugorez May 17, 2024
7e811da
Remove spaces
tugorez May 17, 2024
c23c944
Bring blur handlers back
tugorez May 18, 2024
08ab966
Format
tugorez May 18, 2024
8c99cb4
Add mising blur handler
tugorez May 18, 2024
153eee4
Remove blur events again lol
tugorez May 18, 2024
e8cf8e8
Prevent scroll on focus
tugorez May 18, 2024
5cf50ac
Make the linter happy
tugorez May 20, 2024
6f85439
Disable view focus
tugorez May 20, 2024
e931fbc
Formatting
tugorez May 20, 2024
d70596d
Refactor focus active dom element
tugorez May 20, 2024
e5af604
Enable view focus binding
tugorez May 20, 2024
e54f9ad
Use handleBlur
tugorez Jun 5, 2024
9a194d3
Apply feedback
tugorez Jun 6, 2024
dca6a02
Apply feedback
tugorez Jun 6, 2024
318a2dd
Apply feedback
tugorez Jun 6, 2024
2a6c339
Merge branch 'main' into focus-management-input
ditman Jun 13, 2024
e35705e
Do not attach a blur handler on Safari
ditman Jun 18, 2024
39c5c8d
Makes shiftKey in DomKeyboardEvent nullable.
ditman Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/web_ui/lib/src/engine/dom.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,16 @@ extension DomElementExtension on DomElement {
external JSNumber? get _tabIndex;
double? get tabIndex => _tabIndex?.toDartDouble;

external JSVoid focus();
@JS('focus')
external JSVoid _focus(JSAny options);

void focus({bool? preventScroll, bool? focusVisible}) {
final Map<String, bool> options = <String, bool>{
if (preventScroll != null) 'preventScroll': preventScroll,
if (focusVisible != null) 'focusVisible': focusVisible,
};
tugorez marked this conversation as resolved.
Show resolved Hide resolved
_focus(options.toJSAnyDeep);
}

@JS('scrollTop')
external JSNumber get _scrollTop;
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ final class ViewFocusBinding {
///
/// DO NOT rely on this bit as it will go away soon. You're warned :)!
@visibleForTesting
static bool isEnabled = false;
static bool isEnabled = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ditman we might want to reland these changes but with this one set to false. Not sure what's breaking on the framework side but don't think the rest of these changes is related.

Copy link
Member

Choose a reason for hiding this comment

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

Will take a look!

Copy link
Member

Choose a reason for hiding this comment

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

Reland posted: #53537 /cc @tugorez


final FlutterViewManager _viewManager;
final ui.ViewFocusChangeCallback _onViewFocusChange;
Expand Down Expand Up @@ -51,7 +51,7 @@ final class ViewFocusBinding {
if (state == ui.ViewFocusState.focused) {
// Only move the focus to the flutter view if nothing inside it is focused already.
if (viewId != _viewId(domDocument.activeElement)) {
viewElement?.focus();
viewElement?.focus(preventScroll: true);
}
} else {
viewElement?.blur();
Expand Down
15 changes: 15 additions & 0 deletions lib/web_ui/lib/src/engine/pointer_binding.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,21 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin {
);
_convertEventsToPointerData(data: pointerData, event: event, details: down);
_callback(event, pointerData);

if (event.target == _viewTarget) {
// The event default is prevented so that the <flutter-view /> element doens't gain focus.
event.preventDefault();
// We give time to TextFields to move the focus to the right <input /> or <textarea />
// elements. When requestViewFocus is called it will make sure that a child of the
// <flutter-view /> is focused, otherwise it will focus the <flutter-view /> itself.
Timer(Duration.zero, () {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be useful to comment on the choice of Duration.zero. It could be as simple as "Zero-length timer was chosen because it seemed to work fine in all browsers at the time".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL

ditman marked this conversation as resolved.
Show resolved Hide resolved
EnginePlatformDispatcher.instance.requestViewFocusChange(
viewId: _view.viewId,
state: ui.ViewFocusState.focused,
direction: ui.ViewFocusDirection.undefined,
);
});
}
});

// Why `domWindow` you ask? See this fiddle: https://jsfiddle.net/ditman/7towxaqp
Expand Down
Loading