Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2959233002:
[iOS] Add EG tests for the Keyboard accessory view. (Closed)
|
Created:
3 years, 5 months ago by jif Modified:
3 years, 5 months ago Reviewers:
justincohen CC:
chromium-reviews, marq+watch_chromium.org, ios-reviews+chrome_chromium.org, noyau+watch_chromium.org, ios-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[iOS] Add EG tests for the Keyboard Accessory View.
This CL adds a test that uses the Keyboard Accessory View (KAV) buttons to type in
the omnibox, and makes it so that the QR Code Scanner tests use the QR Code
Scanner button from KAV.
BUG=629776, 737690
Review-Url: https://codereview.chromium.org/2959233002
Cr-Commit-Position: refs/heads/master@{#483435}
Committed: https://chromium.googlesource.com/chromium/src/+/8870512d4dbb64cec03da1d1353284d9db218187
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed comment. #
Messages
Total messages: 18 (11 generated)
Description was changed from ========== [iOS] Add EG tests for the Keyboard accessory view. BUG=629776 ========== to ========== [iOS] Add EG tests for the Keyboard accessory view. BUG=629776 ==========
jif@chromium.org changed reviewers: + justincohen@google.com
Description was changed from ========== [iOS] Add EG tests for the Keyboard accessory view. BUG=629776 ========== to ========== [iOS] Add EG tests for the Keyboard accessory view. BUG=629776, 737690 ==========
Description was changed from ========== [iOS] Add EG tests for the Keyboard accessory view. BUG=629776, 737690 ========== to ========== [iOS] Add EG tests for the Keyboard Accessory View. This LC adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 ==========
jif@chromium.org changed reviewers: + justincohen@chromium.org - justincohen@google.com
Description was changed from ========== [iOS] Add EG tests for the Keyboard Accessory View. This LC adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 ========== to ========== [iOS] Add EG tests for the Keyboard Accessory View. This LC adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 ==========
ptal
Description was changed from ========== [iOS] Add EG tests for the Keyboard Accessory View. This LC adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 ========== to ========== [iOS] Add EG tests for the Keyboard Accessory View. This CL adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 ==========
https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... File ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm (right): https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm:117: if (experimental_flags::IsKeyboardAccessoryViewWithCameraSearchEnabled()) { Is this enabled by default?
https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... File ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm (right): https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm:117: if (experimental_flags::IsKeyboardAccessoryViewWithCameraSearchEnabled()) { On 2017/06/28 21:20:53, justincohen wrote: > Is this enabled by default? It is. I was thinking of removing the "else" once |IsKeyboardAccessoryViewWithCameraSearchEnabled| is removed.
lgtm https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... File ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm (right): https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm:117: if (experimental_flags::IsKeyboardAccessoryViewWithCameraSearchEnabled()) { On 2017/06/29 15:57:04, jif wrote: > On 2017/06/28 21:20:53, justincohen wrote: > > Is this enabled by default? > > It is. > I was thinking of removing the "else" once > |IsKeyboardAccessoryViewWithCameraSearchEnabled| is removed. If you think it's reasonable, maybe add a comment with a bug to track the removal?
thanks https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... File ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm (right): https://codereview.chromium.org/2959233002/diff/1/ios/chrome/browser/ui/qr_sc... ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm:117: if (experimental_flags::IsKeyboardAccessoryViewWithCameraSearchEnabled()) { On 2017/06/29 15:58:24, justincohen wrote: > On 2017/06/29 15:57:04, jif wrote: > > On 2017/06/28 21:20:53, justincohen wrote: > > > Is this enabled by default? > > > > It is. > > I was thinking of removing the "else" once > > |IsKeyboardAccessoryViewWithCameraSearchEnabled| is removed. > > If you think it's reasonable, maybe add a comment with a bug to track the > removal? Done.
The CQ bit was checked by jif@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from justincohen@chromium.org Link to the patchset: https://codereview.chromium.org/2959233002/#ps20001 (title: "Addressed comment.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498760974445680,
"parent_rev": "4d840c2c1c461a5afe64e8138fc79a34f3ab79c1", "commit_rev":
"8870512d4dbb64cec03da1d1353284d9db218187"}
Message was sent while issue was closed.
Description was changed from ========== [iOS] Add EG tests for the Keyboard Accessory View. This CL adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 ========== to ========== [iOS] Add EG tests for the Keyboard Accessory View. This CL adds a test that uses the Keyboard Accessory View (KAV) buttons to type in the omnibox, and makes it so that the QR Code Scanner tests use the QR Code Scanner button from KAV. BUG=629776, 737690 Review-Url: https://codereview.chromium.org/2959233002 Cr-Commit-Position: refs/heads/master@{#483435} Committed: https://chromium.googlesource.com/chromium/src/+/8870512d4dbb64cec03da1d13532... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/8870512d4dbb64cec03da1d13532... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
