Omnibox UI Refresh: Textfield Jogging Experimentation continued |
|||||||||
Issue descriptionContinue experimenting with textfield jogging under a feature flag.
,
Jul 27
Per offline discussion with jdonnelly: We are waiting for this to hit Canary, and have folks try it out for a few days before deciding whether or not to merge to 69.
,
Jul 28
Hey tommycli@, It seems that your CL introduced an alignment issue in both cases - using the jog or using the piper: The Omnibox Text field is placed during the input state 1px too far right now compared to the suggestions in the popup. Screenshots is are attached. Can you please take a look at the regression? Thank you very much :) Mehmet ( Regression range: https://chromium.googlesource.com/chromium/src/+log/4bbcd4ee4025df780e1b044f17b37ffacd084e52..cc5e5302e3ffda76e9ef8899232ba49edb267b71 )
,
Jul 30
mehmet: Thanks for the feedback. Can your confirm that it is wrong even if chrome://flags/#omnibox-ui-jog-textfield-on-popup is set to Disabled? It looks correct when the flag is Disabled.
,
Jul 30
Ah yes, sorry. - It looks correct, when the the flag is Disabled. - It is 1px too far right, when the flag is Enabled.
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b3dceef9a0a59603cbd5832f2cba518d3837a45 commit 6b3dceef9a0a59603cbd5832f2cba518d3837a45 Author: Tommy C. Li <tommycli@chromium.org> Date: Mon Jul 30 21:32:14 2018 Omnibox UI Refresh: Update textfield jog to match new suggestions indent The suggestions indent was updated from 48dp to 47dp in a previous CL. This CL updates the jog length from 12dp to 11dp to match that. Bug: 868157 Change-Id: Ief7e64861e2a35fa2711209728c6078cb249c61d Reviewed-on: https://chromium-review.googlesource.com/1155386 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#579178} [modify] https://crrev.com/6b3dceef9a0a59603cbd5832f2cba518d3837a45/chrome/browser/ui/views/location_bar/location_bar_view.cc
,
Jul 31
,
Jul 31
Tested this issue on Windows 10 and Mac 10.13.5 on the build without fix 70.0.3503.0 and unable to find the difference between latest build and reported build. Launched Chrome and entered chrome in omnibox. Attached is the screenshot for reference. tommycli@chromium.org @ Request you to check and confirm if anything is missed from our end in verifying the issue? Thanks..
,
Jul 31
Thanks tommycli@, the CL from C#6 fixes the alignment issue for me in latest Canary 70.0.3508.0.
,
Jul 31
Great. Requesting merge to 69 for both patches. In c#1 and c#6. We've tested both in Canary now.
,
Jul 31
Approving merge to M69 branch 3497 based on comment #10. Please merge now. Thank you.
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/208b44e19bf38b368a43cc92ca021384bfb15468 commit 208b44e19bf38b368a43cc92ca021384bfb15468 Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Jul 31 17:42:49 2018 Omnibox UI Refresh: Implement textfield jogging behind a flag This implements the previously-removed textfield jogging behavior behind a feature flag accessible from chrome://flags. Followup work: We may implement a special accommodation to the jog flag to make the keystroke-backspace case less jarring of a transition. Bug: 868157 Change-Id: I63fbb8f8f171f1d5ccf09f1cd92332963f567ae3 Reviewed-on: https://chromium-review.googlesource.com/1152459 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#578666}(cherry picked from commit cc5e5302e3ffda76e9ef8899232ba49edb267b71) Reviewed-on: https://chromium-review.googlesource.com/1157028 Reviewed-by: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#282} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/chrome/browser/about_flags.cc [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/chrome/browser/flag_descriptions.h [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/chrome/browser/ui/views/location_bar/location_icon_view.cc [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/components/omnibox/browser/omnibox_field_trial.cc [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/components/omnibox/browser/omnibox_field_trial.h [modify] https://crrev.com/208b44e19bf38b368a43cc92ca021384bfb15468/tools/metrics/histograms/enums.xml
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/afcf849cfcaf7abcc275fa437d1982327591bf3e commit afcf849cfcaf7abcc275fa437d1982327591bf3e Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Jul 31 17:43:29 2018 Omnibox UI Refresh: Update textfield jog to match new suggestions indent The suggestions indent was updated from 48dp to 47dp in a previous CL. This CL updates the jog length from 12dp to 11dp to match that. Bug: 868157 Change-Id: Ief7e64861e2a35fa2711209728c6078cb249c61d Reviewed-on: https://chromium-review.googlesource.com/1155386 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#579178}(cherry picked from commit 6b3dceef9a0a59603cbd5832f2cba518d3837a45) Reviewed-on: https://chromium-review.googlesource.com/1156928 Reviewed-by: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#283} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/afcf849cfcaf7abcc275fa437d1982327591bf3e/chrome/browser/ui/views/location_bar/location_bar_view.cc
,
Jul 31
Both are merged now. Closing.
,
Aug 14
+abdulsyed@ fyi, M69 merges taken for Proj-MdRefresh . |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by bugdroid1@chromium.org
, Jul 27