Skip to content

Commit

Permalink
Reverts "Fix focus management for text fields (flutter#51009)" (flutt…
Browse files Browse the repository at this point in the history
…er#53502)

Reverts: flutter#51009
Initiated by: jiahaog
Reason for reverting: This causes b/348598454. We're getting test failures with stack traces like the following after this PR:

```
Cannot get renderObject of inactive element.
In order for an element to have a valid renderObject, it must be active, which means it is part of the tree.
Instead, this element is in the _ElementLifecycle.inactive state.
If you called this method from a State object, consider guarding
```

Original PR Author: tugorez

Reviewed By: {mdebbar}

This change reverts the following previous change:
Fix focus management for text fields

This PR:

1. Refactors the DOM `focus` function to take [options][1]
2. Removes the timers sorrounding the `activeDomElement.focus()` so that the input elements get focused as immediate as possible.
3. Prevents the default on pointerdown in a Flutter View and schedules a `requestViewFocusChange` to claim focus in that view after some time. This gives `2` the opportunity to focus the right `<input />` or `<textarea />` element. This helps focus correctly transition from one input element to another (without jumping to a flutter view in between).
4. Deactivating a `TextField` doesn't blur the focused element anymore, it insteads schedules for later a call to move the focus to the flutter view if nothing inside it claimed focus first.
5. Prevents scroll in all the focus calls (this should help with the view jumping when focusing one text field after another).

## Sample apps

1. Full screen mode: https://tugorez.com/flutter_focus_web
2. Embedded mode: https://tugorez.com/flutter_focus_web?embedded

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
auto-submit[bot] authored Jun 21, 2024
1 parent 328a08b commit cf3ac2d
Show file tree
Hide file tree
Showing 6 changed files with 254 additions and 251 deletions.
17 changes: 3 additions & 14 deletions lib/web_ui/lib/src/engine/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -658,16 +658,7 @@ extension DomElementExtension on DomElement {
external JSNumber? get _tabIndex;
double? get tabIndex => _tabIndex?.toDartDouble;

@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);
}
external JSVoid focus();

@JS('scrollTop')
external JSNumber get _scrollTop;
Expand Down Expand Up @@ -2258,11 +2249,9 @@ 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;

@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 ?? false;
bool get shiftKey => _event.shiftKey;
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 = true;
static bool isEnabled = false;

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(preventScroll: true);
viewElement?.focus();
}
} 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 ?? false) {
if (event.shiftKey) {
_viewFocusDirection = ui.ViewFocusDirection.backward;
}
});
Expand Down
16 changes: 0 additions & 16 deletions lib/web_ui/lib/src/engine/pointer_binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -982,22 +982,6 @@ 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, () {
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

0 comments on commit cf3ac2d

Please sign in to comment.