[MaterialDesign] [Mac] Omnibox text is 1px too low |
||||
Issue descriptionVersion: Version 51.0.2673.0 canary (64-bit) OS: OS X 10.11.3 What steps will reproduce the problem? (1) Enable MD (2) Take a look at the URL-text in the Omnibox What is the expected output? What do you see instead? It seems that the URL-text is 1px to low. (In Non-MD it is aligned.) Maybe it will be aligned, when the new icons arrive? Please use labels and text to provide additional information. Screenshots are attached. Actual vs. Expected Thanks and regards Mehmet
,
Mar 13 2016
Hello sgabriel@, Here is a screenshot of text in the omnibox alongside the MD-style lock icon (for the record I have not landed the MD lock update to the omnibox - that will happen soon). Can you tell me what you think about text vertical placement in the textfield? I think it's about right.
,
Mar 13 2016
Hello sgabriel@, Thank you for the clarification. I have adjusted the text upwards by 1 pt. See attached screenshot. I have also attached a screenshot that shows the text selected. It looks like with this 1pt adjustment (but not because of it) the selection rect is off vertical center by 1px on a Retina screen. It looks like I can adjust the rect by 1px on Retina to center the selection rect. Do you want me to make that change? The distance from the bottom of the textfield to the baseline would be 8.5pt on Retina. Note that I'm not sure I can make a similar adjustment on non-Retina.
,
Mar 13 2016
Awesome. Your suggestion for the selected state sounds good to me.
,
Mar 13 2016
Great. I hope your both adjustments will also affect the Non-Retina Macs. :-) Thanks so much!
,
Mar 14 2016
Hello sgabriel@, I think the adjustments I'm trying to make are a result of another change. According to the Mac toolbar spec the button click target is 28pt tall. The spec shows that the omnibox is also this exact height, and that's what I put in the code. However, it appears that the selection rect is 17pts tall, which means on non-Retina you will never get it perfectly centered (unless you adjust it vertically by 0.5pt, but that will make the text fuzzy). So either we leave things as they are, meaning the text will be centered vertically on Retina but not non-Retina, or we adjust the height of the omnibox which will center the selection rect on Retina and non-Retina, but will leave the toolbar button icons slightly off-center vertically. I think in the current design the toolbar buttons are 29pt tall and so matching the omnibox's height to that is fine (29pt - 17pt = an even number of pixels above and below the selection rect).
,
Mar 14 2016
Let's leave things as they are. Thanks for looking into this.
,
Mar 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4c4dbdf32ba3c71b928eb5169029571cd1713fa3 commit 4c4dbdf32ba3c71b928eb5169029571cd1713fa3 Author: shrike <shrike@chromium.org> Date: Wed Mar 16 16:48:26 2016 Adjust locationbar text vertical position under Material Design. Under Material Design The location bar's text is currently positioned 1pt too low on non-Retina and 0.5pt too low on Retina. BUG= 593727 Review URL: https://codereview.chromium.org/1802643002 Cr-Commit-Position: refs/heads/master@{#381472} [modify] https://crrev.com/4c4dbdf32ba3c71b928eb5169029571cd1713fa3/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm [modify] https://crrev.com/4c4dbdf32ba3c71b928eb5169029571cd1713fa3/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.h [modify] https://crrev.com/4c4dbdf32ba3c71b928eb5169029571cd1713fa3/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm
,
Mar 16 2016
,
Mar 17 2016
Hi shrike@. Thanks for fixing this. I noticed something strange: I learned, that on Non-retina it was not possible to fix the vertical position of selection rect. But when I check it in an Incognito window on Non-retina, the selection rect is centered?! Please see the screenshots. Do you know why? Thanks.
,
Mar 17 2016
Hello mehmet@, Incognito on non-Retina will also not be centered. Please see bug 594847 for an explanation of what's currently going on.
,
Mar 17 2016
Thank you for the explanation. |
||||
►
Sign in to add a comment |
||||
Comment 1 by shrike@chromium.org
, Mar 13 2016