Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix focus management for text fields #51009

Merged
merged 30 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 14 additions & 3 deletions lib/web_ui/lib/src/engine/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,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,
};
_focus(options.toJSAnyDeep);
}

@JS('scrollTop')
external JSNumber get _scrollTop;
Expand Down Expand Up @@ -2249,9 +2258,11 @@ extension DomKeyboardEventExtension on DomKeyboardEvent {
external JSBoolean? get _repeat;
bool? get repeat => _repeat?.toDart;

// Safari injects synthetic keyboard events after auto-complete that don't
// have a `shiftKey` attribute, so this property must be nullable.
@JS('shiftKey')
external JSBoolean get _shiftKey;
bool get shiftKey => _shiftKey.toDart;
external JSBoolean? get _shiftKey;
bool? get shiftKey => _shiftKey?.toDart;
Comment on lines +2264 to +2265
Copy link
Member

Choose a reason for hiding this comment

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

This change is unrelated to this PR, I can move it to a separate PR to keep this one small.


@JS('isComposing')
external JSBoolean get _isComposing;
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/keyboard_binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class FlutterHtmlKeyboardEvent {
num? get timeStamp => _event.timeStamp;
bool get altKey => _event.altKey;
bool get ctrlKey => _event.ctrlKey;
bool get shiftKey => _event.shiftKey;
bool get shiftKey => _event.shiftKey ?? false;
bool get metaKey => _event.metaKey;
bool get isComposing => _event.isComposing;

Expand Down
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 All @@ -70,7 +70,7 @@ final class ViewFocusBinding {

late final DomEventListener _handleKeyDown = createDomEventListener((DomEvent event) {
event as DomKeyboardEvent;
if (event.shiftKey) {
if (event.shiftKey ?? false) {
_viewFocusDirection = ui.ViewFocusDirection.backward;
}
});
Expand Down
16 changes: 16 additions & 0 deletions lib/web_ui/lib/src/engine/pointer_binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,22 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin {
);
_convertEventsToPointerData(data: pointerData, event: event, details: down);
_callback(event, pointerData);

if (event.target == _viewTarget) {
// Ensure smooth focus transitions between text fields within the Flutter view.
// Without preventing the default and this delay, the engine may not have fully
// rendered the next input element, leading to the focus incorrectly returning to
// the main Flutter view instead.
// A zero-length timer is sufficient in all tested browsers to achieve this.
event.preventDefault();
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

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