[Omnibox/MdRefresh] Add some space to the right side of the suggestions to be aligned with the Tab-Switch button |
||||||
Issue descriptionChrome Version: Canary 69.0.3493.0 OS: macOS, but probably OS=All What steps will reproduce the problem? (0) Enable MdRefresh (1) type something into the Omnibox (2) take a look at the right side of the suggestions What is the expected result? Maybe there could be some pix space at the right side of the suggestions to be aligned with the Tab-Switch button. What happens instead? Some suggestions end with "..." some are very close to the Suggestion Pop Up border. Screenshots are attached. Thanks Mehmet
,
Jul 17
,
Jul 17
,
Jul 17
,
Jul 18
Hmm -- making those line up is going to be very difficult. The right-margin of the tab-switch button is dynamically calculated here [1] and is going to vary by the font metrics of the system. Making the text margin match that is architecturally kind of weird because: the list of suggestions may not contain any tab switch suggestions -- so instantiating one just to calculate the margin would be weird. Instead, I recommend provide a fixed right-margin to both the tab-switch button as well as the text. [1] https://cs.chromium.org/chromium/src/chrome/browser/ui/views/omnibox/omnibox_result_view.cc?gsn=suggestion_tab_switch_button_&g=0&l=245-247
,
Jul 18
Attaching a screenshot of what it looks like if we make both the button and text right-margin a fixed 8dp. 8dp matches the vertical margin of the tab-switch button on my system, so it's identical on my system.
,
Jul 18
Looks nice 👍 Thanks.
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdaf4f4f9b9821617e98d57eb88bf4945ccb2ac8 commit bdaf4f4f9b9821617e98d57eb88bf4945ccb2ac8 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Jul 18 17:31:24 2018 Omnibox UI Refresh: Update naming of OmniboxMatchCellView constants This CL has no behavioral changes and is a name clarification only. 1) kRichSuggestionMarginHeight => kRefreshTwoLineRowMarginHeight We are using this margin height for all two-line suggestions, which include tab-switch suggestions and old-style answers in addition to rich entity suggestions. This rename makes that explicit. 2) "Split" suggestions => "One-line" suggestions. We originally named these split suggestions because the title and URL were split by a hyphen. However, since we are calling two-line suggestions "two-line", I think calling these "one-line" suggestions would be most clear. Bug: 863982 Change-Id: Id56f6798c9a609276bbd0b0de5e565a3cf2cd475 Reviewed-on: https://chromium-review.googlesource.com/1141207 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#576115} [modify] https://crrev.com/bdaf4f4f9b9821617e98d57eb88bf4945ccb2ac8/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc [modify] https://crrev.com/bdaf4f4f9b9821617e98d57eb88bf4945ccb2ac8/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.h
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e320ad35e7f1583401c5234a82793848d18729f commit 4e320ad35e7f1583401c5234a82793848d18729f Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Jul 18 19:26:02 2018 Omnibox UI Refresh: Add right-padding to the match cell of suggestions. This CL adds an 8dp right-padding to the match cell of suggestions. This CL also makes the right-padding of the tab-switch to be the same as the match cell. Previously it was set to be the same as the vertical padding. That should also increase layout predictability in the face of very tall glyphs. Bug: 863982 Change-Id: I1bdbfc0062d1ddeba57d21cc2813ed30d42548c7 Reviewed-on: https://chromium-review.googlesource.com/1141288 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#576162} [modify] https://crrev.com/4e320ad35e7f1583401c5234a82793848d18729f/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc [modify] https://crrev.com/4e320ad35e7f1583401c5234a82793848d18729f/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.h [modify] https://crrev.com/4e320ad35e7f1583401c5234a82793848d18729f/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
,
Jul 18
Done - thanks!
,
Jul 19
Able to reproduce the issue on Mac 10.13.3 using chrome reported version #69.0.3493.0 Verified the fix on Mac 10.13.3, Win-10 and Ubuntu 17.10 using latest chrome version #69.0.3496.0 as per the comment #0. Attaching screen shot for reference. Observed that some pix space is present at the right side of the suggestions to be aligned with the Tab-Switch button. Hence, the fix is working as expected. Adding the verified labels. Thanks...!! |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by jdonnelly@chromium.org
, Jul 17