Skip to content

Commit

Permalink
[iOS] Add omnibox typing shield in form input accessory view
Browse files Browse the repository at this point in the history
Currently, when the bottom omnibox collapses above the keyboard, the
bottom toolbar is placed above the input accessory view. When a
textfield is focused on web, WKWebView scrolls the web page to keep the
textfield visible. Since the toolbar is above the keyboard accessory,
the toolbar blocks the textfield.

This CL adds an omnibox typing shield (a transparent view) in the form
input accessory view for the collapsed omnibox. The collapsed omnibox
will be behind the keyboard input accessory instead of above it.
- A omniboxTypingShield view is added in FormInputAccessoryView. To
  allow this, the remaining views are added into a contentView wrapper.
- The typing shield is only added when the omnibox is at the bottom.
- When tapping on the typing shield, resign the first responder.

Textfield focus recording:
https://drive.google.com/file/d/19Tz3re2vWJJw5o1nEQH_WwzRh2KdFORw/view?usp=sharing

Autofill recording:
https://drive.google.com/file/d/11Gn7AI6rp1oyPh6n6_FXLr6Rnm_USPek/view?usp=drive_link

Voice over recording:
https://drive.google.com/file/d/1NsvFXLaXSLUfJpfb6ccvfatys6ztp5h6/view?usp=drive_link

Bug: 1490601
Change-Id: Id7980a1e3441d723f26886daf7db3ef5c676bc91
Low-Coverage-Reason: TESTS_IN_SEPARATE_CL egtest moved to a separate CL 4931670
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4926228
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Auto-Submit: Christian Xu <christianxu@chromium.org>
Commit-Queue: Christian Xu <christianxu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209354}
  • Loading branch information
ChristianXuG authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent 48e1caf commit 7605509
Show file tree
Hide file tree
Showing 15 changed files with 204 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ - (void)closeKeyboardWithoutButtonPress {
[self closeKeyboardLoggingButtonPressed];
}

- (void)closeKeyboardWithOmniboxTypingShield {
if (_webState) {
UIView* view = _webState->GetView();
CHECK(view);
[view endEditing:YES];
}
}

- (void)selectPreviousElementWithButtonPress {
[self selectPreviousElementLoggingButtonPressed];
}
Expand Down
2 changes: 2 additions & 0 deletions ios/chrome/browser/autofill/form_input_navigator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// Called to close the keyboard when the user did not press the close button
// directly.
- (void)closeKeyboardWithoutButtonPress;
// Called when the omnibox typing shield is tapped by the user.
- (void)closeKeyboardWithOmniboxTypingShield;

// Called when the previous button is pressed by the user.
- (void)selectPreviousElementWithButtonPress;
Expand Down
4 changes: 4 additions & 0 deletions ios/chrome/browser/ui/autofill/form_input_accessory/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ source_set("form_input_accessory") {
"//components/password_manager/core/browser",
"//components/password_manager/core/common:features",
"//components/password_manager/ios",
"//components/prefs",
"//components/strings",
"//ios/chrome/app/application_delegate:app_state_header",
"//ios/chrome/app/strings",
Expand All @@ -46,6 +47,8 @@ source_set("form_input_accessory") {
"//ios/chrome/browser/shared/coordinator/layout_guide",
"//ios/chrome/browser/shared/coordinator/scene:scene_state_header",
"//ios/chrome/browser/shared/model/browser_state",
"//ios/chrome/browser/shared/model/prefs",
"//ios/chrome/browser/shared/model/prefs:pref_names",
"//ios/chrome/browser/shared/model/web_state_list",
"//ios/chrome/browser/shared/public/commands",
"//ios/chrome/browser/shared/public/features",
Expand All @@ -55,6 +58,7 @@ source_set("form_input_accessory") {
"//ios/chrome/browser/ui/autofill/manual_fill",
"//ios/chrome/browser/ui/autofill/manual_fill:manual_fill_ui",
"//ios/chrome/browser/ui/bubble",
"//ios/chrome/browser/ui/toolbar/public",
"//ios/chrome/common:button_config",
"//ios/chrome/common/ui/colors:colors",
"//ios/chrome/common/ui/elements:form_input_accessory",
Expand Down
3 changes: 3 additions & 0 deletions ios/chrome/browser/ui/autofill/form_input_accessory/DEPS
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include_rules = [
"+ios/chrome/browser/ui/toolbar/public/toolbar_utils.h",
]
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
// And form navigation buttons on iPhone (iPad already includes those).
- (void)showAccessorySuggestions:(NSArray<FormSuggestion*>*)suggestions;

// Preferred omnibox position was updated. `isBottomOmnibox`: whether the new
// position is bottom omnibox.
- (void)newOmniboxPositionIsBottom:(BOOL)isBottomOmnibox;

@end

#endif // IOS_CHROME_BROWSER_UI_AUTOFILL_FORM_INPUT_ACCESSORY_FORM_INPUT_ACCESSORY_CONSUMER_H_
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ - (void)start {
self.layoutGuide =
[layoutGuideCenter makeLayoutGuideNamed:kAutofillFirstSuggestionGuide];
[self.baseViewController.view addLayoutGuide:self.layoutGuide];

self.formInputAccessoryMediator.prefService =
ChromeBrowserState::FromBrowserState(self.browser->GetBrowserState())
->GetPrefs();
}

- (void)stop {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#import <Foundation/Foundation.h>

#import "components/password_manager/core/browser/password_store_interface.h"
#import "components/prefs/pref_service.h"
#import "ios/chrome/browser/autofill/form_input_navigator.h"
#import "ios/chrome/browser/autofill/form_suggestion_client.h"
#import "ios/chrome/browser/shared/model/web_state_list/web_state_list_observer_bridge.h"
Expand Down Expand Up @@ -70,6 +71,9 @@ class WebStateList;
@property(nonatomic, readonly, getter=isInputAccessoryViewActive)
BOOL inputAccessoryViewActive;

// Get the preferred omnibox position.
@property(nonatomic, assign) PrefService* prefService;

// Disables suggestions updates and asks the consumer to remove the current
// ones.
- (void)disableSuggestions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#import "ios/chrome/browser/autofill/form_suggestion_tab_helper.h"
#import "ios/chrome/browser/default_browser/model/utils.h"
#import "ios/chrome/browser/shared/coordinator/chrome_coordinator/chrome_coordinator.h"
#import "ios/chrome/browser/shared/model/prefs/pref_backed_boolean.h"
#import "ios/chrome/browser/shared/model/prefs/pref_names.h"
#import "ios/chrome/browser/shared/model/web_state_list/web_state_list.h"
#import "ios/chrome/browser/shared/public/commands/security_alert_commands.h"
#import "ios/chrome/browser/shared/public/features/features.h"
Expand Down Expand Up @@ -72,7 +74,8 @@ void OnPasswordCounterChanged() override {
password_manager::PasswordCounter counter_;
};

@interface FormInputAccessoryMediator () <FormActivityObserver,
@interface FormInputAccessoryMediator () <BooleanObserver,
FormActivityObserver,
FormInputAccessoryViewDelegate,
CRWWebStateObserver,
PasswordCounterObserver,
Expand Down Expand Up @@ -151,6 +154,9 @@ @implementation FormInputAccessoryMediator {

// If YES `_lastSeenParams` is valid.
BOOL _hasLastSeenParams;

// Pref tracking if bottom omnibox is enabled.
PrefBackedBoolean* _bottomOmniboxEnabled;
}

- (instancetype)
Expand Down Expand Up @@ -267,6 +273,9 @@ - (void)disconnect {
_webStateListObserver.reset();
_webStateList = nullptr;
}
[_bottomOmniboxEnabled stop];
[_bottomOmniboxEnabled setObserver:nil];
_bottomOmniboxEnabled = nil;
}

- (void)detachFromWebState {
Expand Down Expand Up @@ -372,6 +381,11 @@ - (FormInputAccessoryViewTextData*)textDataforFormInputAccessoryView:
return ChromiumAccessoryViewTextData();
}

- (void)fromInputAccessoryViewDidTapOmniboxTypingShield:
(FormInputAccessoryView*)sender {
[self.formNavigationHandler closeKeyboardWithOmniboxTypingShield];
}

#pragma mark - CRWWebStateObserver

- (void)webStateWasShown:(web::WebState*)webState {
Expand Down Expand Up @@ -448,6 +462,22 @@ - (void)setCurrentProvider:(id<FormInputSuggestionsProvider>)currentProvider {
_currentProvider.formInputNavigator = self.formNavigationHandler;
}

- (void)setPrefService:(PrefService*)prefService {
_prefService = prefService;
if (IsBottomOmniboxSteadyStateEnabled() && _prefService) {
_bottomOmniboxEnabled =
[[PrefBackedBoolean alloc] initWithPrefService:_prefService
prefName:prefs::kBottomOmnibox];
[_bottomOmniboxEnabled setObserver:self];
// Initialize to the current value.
[self booleanDidChange:_bottomOmniboxEnabled];
} else {
[_bottomOmniboxEnabled stop];
[_bottomOmniboxEnabled setObserver:nil];
_bottomOmniboxEnabled = nil;
}
}

#pragma mark - Private

// Returns the reauthentication module, which can be an override for testing
Expand Down Expand Up @@ -578,6 +608,15 @@ - (void)applicationDidEnterBackground:(NSNotification*)notification {
[self.handler resetFormInputView];
}

#pragma mark - Boolean Observer

- (void)booleanDidChange:(id<ObservableBoolean>)observableBoolean {
if (observableBoolean == _bottomOmniboxEnabled) {
CHECK(IsBottomOmniboxSteadyStateEnabled());
[self.consumer newOmniboxPositionIsBottom:_bottomOmniboxEnabled.value];
}
}

#pragma mark - FormSuggestionClient

- (void)didSelectSuggestion:(FormSuggestion*)formSuggestion {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
#import "base/strings/sys_string_conversions.h"
#import "components/autofill/core/common/autofill_features.h"
#import "ios/chrome/browser/autofill/form_suggestion_client.h"
#import "ios/chrome/browser/shared/public/features/features.h"
#import "ios/chrome/browser/shared/ui/util/uikit_ui_util.h"
#import "ios/chrome/browser/ui/autofill/branding/branding_view_controller.h"
#import "ios/chrome/browser/ui/autofill/form_input_accessory/form_suggestion_view.h"
#import "ios/chrome/browser/ui/autofill/manual_fill/manual_fill_accessory_view_controller.h"
#import "ios/chrome/browser/ui/toolbar/public/toolbar_utils.h"
#import "ios/chrome/common/ui/elements/form_input_accessory_view.h"
#import "ios/chrome/common/ui/util/constraints_ui_util.h"
#import "ios/chrome/grit/ios_strings.h"
Expand Down Expand Up @@ -49,7 +52,10 @@ @interface FormInputAccessoryViewController () <

@end

@implementation FormInputAccessoryViewController
@implementation FormInputAccessoryViewController {
// Is the preferred omnibox position at the bottom.
BOOL _isBottomOmnibox;
}

@synthesize addressButtonHidden = _addressButtonHidden;
@synthesize creditCardButtonHidden = _creditCardButtonHidden;
Expand Down Expand Up @@ -116,6 +122,13 @@ - (FormInputAccessoryView*)formInputAccessoryView {
return base::apple::ObjCCastStrict<FormInputAccessoryView>(self.view);
}

- (void)traitCollectionDidChange:(UITraitCollection*)previousTraitCollection {
[super traitCollectionDidChange:previousTraitCollection];
if (IsBottomOmniboxSteadyStateEnabled()) {
[self updateOmniboxTypingShieldVisibility];
}
}

#pragma mark - Public

- (void)lockManualFallbackView {
Expand All @@ -136,6 +149,11 @@ - (void)showAccessorySuggestions:(NSArray<FormSuggestion*>*)suggestions {
[self announceVoiceOverMessageIfNeeded:[suggestions count]];
}

- (void)newOmniboxPositionIsBottom:(BOOL)isBottomOmnibox {
_isBottomOmnibox = isBottomOmnibox;
[self updateOmniboxTypingShieldVisibility];
}

#pragma mark - Getter

- (BOOL)isFormAccessoryVisible {
Expand Down Expand Up @@ -256,6 +274,19 @@ - (void)announceVoiceOverMessageIfNeeded:(int)suggestionCount {
self.lastAnnouncedFieldId = _currentFieldId;
}

- (void)updateOmniboxTypingShieldVisibility {
CHECK(IsBottomOmniboxSteadyStateEnabled());
const BOOL shouldShowTypingShield =
_isBottomOmnibox && IsSplitToolbarMode(self.traitCollection);
const CGFloat typingShieldHeight =
shouldShowTypingShield
? ToolbarCollapsedHeight(
self.traitCollection.preferredContentSizeCategory)
: 0.0;
[[self formInputAccessoryView]
setOmniboxTypingShieldHeight:typingShieldHeight];
}

#pragma mark - ManualFillAccessoryViewControllerDelegate

- (void)keyboardButtonPressed {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ - (FormInputAccessoryViewTextData*)textDataforFormInputAccessoryView:
return ChromiumAccessoryViewTextData();
}

- (void)fromInputAccessoryViewDidTapOmniboxTypingShield:
(FormInputAccessoryView*)sender {
NOTREACHED() << "The typing shield should only be present on web";
}

#pragma mark - Helper methods

// Returns the cell containing `textField`.
Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/browser/ui/toolbar/secondary_toolbar_view.mm
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ - (void)willMoveToSuperview:(UIView*)newSuperview {
// in view controller.
_locationBarKeyboardConstraint = [newSuperview.keyboardLayoutGuide.topAnchor
constraintGreaterThanOrEqualToAnchor:self.locationBarContainer
.bottomAnchor];
.topAnchor];
}
}

Expand Down
10 changes: 10 additions & 0 deletions ios/chrome/browser/ui/toolbar/secondary_toolbar_view_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ - (void)collapsedToolbarButtonTapped {
[super collapsedToolbarButtonTapped];

if ([self.view.locationBarKeyboardConstraint isActive]) {
// When the bottom omnibox is collapsed above the keyboard, it's positioned
// behind an `omniboxTypingShield` (transparent view) in the
// `formInputAccessoryView`. This allow the keyboard to know about the size
// of the omnibox (crbug.com/1490601).
// When voice over is off, tapping the collapsed bottom omnibox interacts
// with the `omniboxTypingShield`. The logic to dismiss the keyboard is
// handled in `formInputAccessoryViewHandler`. However, the typing shield
// has `isAccessibilityElement` equals NO to let the user interact with the
// omnibox on voice over. In this mode, logic to dismiss the keyboard is
// handled here in `SecondaryToolbarViewController`.
CHECK(IsBottomOmniboxSteadyStateEnabled());
CHECK([self hasOmnibox]);
UIResponder* responder = GetFirstResponder();
Expand Down
8 changes: 8 additions & 0 deletions ios/chrome/common/ui/elements/form_input_accessory_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
- (void)formInputAccessoryViewDidTapCloseButton:(FormInputAccessoryView*)sender;
- (FormInputAccessoryViewTextData*)textDataforFormInputAccessoryView:
(FormInputAccessoryView*)sender;
- (void)fromInputAccessoryViewDidTapOmniboxTypingShield:
(FormInputAccessoryView*)sender;
@end

extern NSString* const kFormInputAccessoryViewAccessibilityID;
Expand Down Expand Up @@ -48,6 +50,12 @@ extern NSString* const kFormInputAccessoryViewAccessibilityID;
- (void)setUpWithLeadingView:(UIView*)leadingView
customTrailingView:(UIView*)customTrailingView;

// Sets the height of the omnibox typing shield. Set a height of 0 to hide the
// typing shield. The omnibox typing shield is a transparent view on the top
// edge of the input accessory view for the collapsed bottom omnibox
// (crbug.com/1490601).
- (void)setOmniboxTypingShieldHeight:(CGFloat)typingShieldHeight;

@end

#endif // IOS_CHROME_COMMON_UI_ELEMENTS_FORM_INPUT_ACCESSORY_VIEW_H_

0 comments on commit 7605509

Please sign in to comment.