Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2230963004:
[MD User Menu] Un-hardcode profile card's spacing (Closed)
Description[MD User Menu] Un-hardcode profile card's spacing
On MD user menu, vertical spacings in the profile card used to be
hardcoded, hence causing some display problems on certain locales
(see first bug for how Hindi names would get chopped off).
This CL calculates the spacing needed given the height of the profile
name label and/or username label in the profile card, hence fixing
the chopped-off problem - see a new screenshot here: https://screenshot.googleplex.com/FHK5snyG0PH.
BUG=636321
BUG=615893
Committed: https://crrev.com/b409fbb2cd01a0d05a40dff2daa22685fa14d2f3
Cr-Commit-Position: refs/heads/master@{#414167}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Removed floor and rebased #Messages
Total messages: 19 (10 generated)
Description was changed from ========== 636321 Fix Attempt BUG= ========== to ========== [MD User Menu] Un-hardcode profile card's spacing On MD user menu, vertical spacings in the profile card used to be hardcoded, hence causing some display problems on certain locales (see first bug for how Hindi names would get chopped off). This CL calculates the spacing needed given the height of the profile name label and/or username label in the profile card, hence fixing the chopped-off problem - see a new screenshot here: https://screenshot.googleplex.com/FHK5snyG0PH. BUG=636321 BUG=615893 ==========
janeliulwq@google.com changed reviewers: + sky@chromium.org
Thanks!
https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right): https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... chrome/browser/ui/views/profiles/profile_chooser_view.cc:1700: std::floor((current_profile_photo->GetPreferredSize().height() - Do you really need the floor here? Don't you get what you want without it? https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... chrome/browser/ui/views/profiles/profile_chooser_view.cc:1744: std::floor((current_profile_photo->GetPreferredSize().height() - Same comment here.
https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right): https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... chrome/browser/ui/views/profiles/profile_chooser_view.cc:1700: std::floor((current_profile_photo->GetPreferredSize().height() - On 2016/08/24 19:40:54, sky wrote: > Do you really need the floor here? Don't you get what you want without it? It's for just in case that the divided value has a .5 - I've had weird rendering on mac when the position is at half pixel. Do you think it's not necessary on lin/win?
https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right): https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... chrome/browser/ui/views/profiles/profile_chooser_view.cc:1700: std::floor((current_profile_photo->GetPreferredSize().height() - On 2016/08/24 19:54:34, Jane wrote: > On 2016/08/24 19:40:54, sky wrote: > > Do you really need the floor here? Don't you get what you want without it? > > It's for just in case that the divided value has a .5 - I've had weird rendering > on mac when the position is at half pixel. Do you think it's not necessary on > lin/win? You're setting this to an int, so it's going to be floored, right?
Patchset #2 (id:20001) has been deleted
https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right): https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... chrome/browser/ui/views/profiles/profile_chooser_view.cc:1700: std::floor((current_profile_photo->GetPreferredSize().height() - On 2016/08/24 20:29:12, sky wrote: > On 2016/08/24 19:54:34, Jane wrote: > > On 2016/08/24 19:40:54, sky wrote: > > > Do you really need the floor here? Don't you get what you want without it? > > > > It's for just in case that the divided value has a .5 - I've had weird > rendering > > on mac when the position is at half pixel. Do you think it's not necessary on > > lin/win? > > You're setting this to an int, so it's going to be floored, right? Oh oops, sorry totally forgot! https://codereview.chromium.org/2230963004/diff/1/chrome/browser/ui/views/pro... chrome/browser/ui/views/profiles/profile_chooser_view.cc:1744: std::floor((current_profile_photo->GetPreferredSize().height() - On 2016/08/24 19:40:54, sky wrote: > Same comment here. Done.
LGTM
The CQ bit was checked by janeliulwq@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by janeliulwq@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [MD User Menu] Un-hardcode profile card's spacing On MD user menu, vertical spacings in the profile card used to be hardcoded, hence causing some display problems on certain locales (see first bug for how Hindi names would get chopped off). This CL calculates the spacing needed given the height of the profile name label and/or username label in the profile card, hence fixing the chopped-off problem - see a new screenshot here: https://screenshot.googleplex.com/FHK5snyG0PH. BUG=636321 BUG=615893 ========== to ========== [MD User Menu] Un-hardcode profile card's spacing On MD user menu, vertical spacings in the profile card used to be hardcoded, hence causing some display problems on certain locales (see first bug for how Hindi names would get chopped off). This CL calculates the spacing needed given the height of the profile name label and/or username label in the profile card, hence fixing the chopped-off problem - see a new screenshot here: https://screenshot.googleplex.com/FHK5snyG0PH. BUG=636321 BUG=615893 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [MD User Menu] Un-hardcode profile card's spacing On MD user menu, vertical spacings in the profile card used to be hardcoded, hence causing some display problems on certain locales (see first bug for how Hindi names would get chopped off). This CL calculates the spacing needed given the height of the profile name label and/or username label in the profile card, hence fixing the chopped-off problem - see a new screenshot here: https://screenshot.googleplex.com/FHK5snyG0PH. BUG=636321 BUG=615893 ========== to ========== [MD User Menu] Un-hardcode profile card's spacing On MD user menu, vertical spacings in the profile card used to be hardcoded, hence causing some display problems on certain locales (see first bug for how Hindi names would get chopped off). This CL calculates the spacing needed given the height of the profile name label and/or username label in the profile card, hence fixing the chopped-off problem - see a new screenshot here: https://screenshot.googleplex.com/FHK5snyG0PH. BUG=636321 BUG=615893 Committed: https://crrev.com/b409fbb2cd01a0d05a40dff2daa22685fa14d2f3 Cr-Commit-Position: refs/heads/master@{#414167} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b409fbb2cd01a0d05a40dff2daa22685fa14d2f3 Cr-Commit-Position: refs/heads/master@{#414167} |
