|
|
Created:
4 years, 5 months ago by Jane Modified:
4 years, 5 months ago Reviewers:
*groby-ooo-7-16 CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionBringing back fast user switching on Mac material design user menu
Specifically:
1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page;
2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect;
3. Made UI modifications on paddings, seperators/not, using new skia icons, etc.
See screenshot:
https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view
Mocks:
https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20Sign%20In/user_menu/specs#%2Fspec-2.png
Design doc:
https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnqik/edit?ts=57445a70#heading=h.ulgws5x4kc7r
BUG=615893
Committed: https://crrev.com/d123727f6a71838bc658c255a685fed19bd34d6e
Cr-Commit-Position: refs/heads/master@{#406849}
Patch Set 1 #
Total comments: 31
Patch Set 2 : Addressed comments #Patch Set 3 : Final nits #Messages
Total messages: 26 (18 generated)
Description was changed from ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page 2. Added a "Guest" button 3. Added a "Close all your windows" button 4. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ========== to ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, with is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ==========
Description was changed from ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, with is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ========== to ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ==========
janeliulwq@google.com changed reviewers: + groby@chromium.org
janeliulwq@google.com changed required reviewers: + groby@chromium.org
Hi Rachel, PTAL, thanks!
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...
Description was changed from ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ========== to ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thank you - the structure LG, just a bunch of nits and questions https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right): https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:455: - (void)addRightMarginSpacing:(int)rightMarginSpacing { nit: setRightMargingSpacing seems more appropriate than add... https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:471: NSDivideRect(frame, &marginRect, &frame, rightMarginSpacing_, NSMaxXEdge); You can probably skip the ">0" test. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1079: PrefService* service = g_browser_process->local_state(); Curious - why go through the global process, instead of browser_->profile()->GetPrefs()? https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1388: - (CGFloat)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles Is there any reason this needs to be a mutable array? https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1536: inContainer:container.get()]; No reason for .get(), I think. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1541: otherProfiles:otherProfiles.get() see above https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2297: - (NSView*)createMaterialDesignOptionsViewWithRect:(NSRect)rect Traditionally, this would be named create...WithFrame: (We also wouldn't have a method called createXYZ, but that ship has sailed :) https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2298: displayLock:(BOOL)displayLock { showLock: instead of displayLock: is usually the convention. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2322: browser_->profile()->GetOriginalProfile()) nit: can you hoist the rhs into a separate variable, outside the loop? (I had to read this a few times before seeing the underscore :) https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2357: PrefService* service = g_browser_process->local_state(); See above question re: getting the prefs from |browser_| https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2626: ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance(); nit: Reference please. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2652: ? (kHorizontalSpacing - 2.0) Why -2.0? (Please leave a comment explaining magic numbers. Even if it's just "so it looks right, not sure why we have to move 2.0")
Patchset #2 (id:20001) has been deleted
Thanks for the fast reply, PTAL! https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right): https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:455: - (void)addRightMarginSpacing:(int)rightMarginSpacing { On 2016/07/20 20:54:22, groby wrote: > nit: setRightMargingSpacing seems more appropriate than add... Done. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:471: NSDivideRect(frame, &marginRect, &frame, rightMarginSpacing_, NSMaxXEdge); On 2016/07/20 20:54:22, groby wrote: > You can probably skip the ">0" test. Done. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1079: PrefService* service = g_browser_process->local_state(); On 2016/07/20 20:54:22, groby wrote: > Curious - why go through the global process, instead of > browser_->profile()->GetPrefs()? It doesn't seem like profile()->GetPrefs() have this value (?) - I get a "trying to read an unregistered pref" error when I use that. The global process way is how it's done in profile_chooser_view.cc so I just took it. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1388: - (CGFloat)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles On 2016/07/20 20:54:22, groby wrote: > Is there any reason this needs to be a mutable array? I think it's because this array needs the dynamic insertion operations for its construction. See L1521 in this patch. Should this be something else? https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1536: inContainer:container.get()]; On 2016/07/20 20:54:22, groby wrote: > No reason for .get(), I think. Done. I'm assuming all the .get()'s here? (What is .get() usually used for?) https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1541: otherProfiles:otherProfiles.get() On 2016/07/20 20:54:22, groby wrote: > see above Done. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2297: - (NSView*)createMaterialDesignOptionsViewWithRect:(NSRect)rect On 2016/07/20 20:54:22, groby wrote: > Traditionally, this would be named create...WithFrame: > > (We also wouldn't have a method called createXYZ, but that ship has sailed :) Done. But there are soo many methods called createXYZ now... https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2298: displayLock:(BOOL)displayLock { On 2016/07/20 20:54:22, groby wrote: > showLock: instead of displayLock: is usually the convention. Done. I went ahead to change all the displayLock -> showLock. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2322: browser_->profile()->GetOriginalProfile()) On 2016/07/20 20:54:22, groby wrote: > nit: can you hoist the rhs into a separate variable, outside the loop? (I had to > read this a few times before seeing the underscore :) > Done. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2357: PrefService* service = g_browser_process->local_state(); On 2016/07/20 20:54:22, groby wrote: > See above question re: getting the prefs from |browser_| See above. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2626: ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance(); On 2016/07/20 20:54:22, groby wrote: > nit: Reference please. Done. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2652: ? (kHorizontalSpacing - 2.0) On 2016/07/20 20:54:22, groby wrote: > Why -2.0? (Please leave a comment explaining magic numbers. Even if it's just > "so it looks right, not sure why we have to move 2.0") Done. Is this clear, and is this too long of a comment?
LGTM % nits https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right): https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1079: PrefService* service = g_browser_process->local_state(); On 2016/07/20 22:03:46, Jane wrote: > On 2016/07/20 20:54:22, groby wrote: > > Curious - why go through the global process, instead of > > browser_->profile()->GetPrefs()? > > It doesn't seem like profile()->GetPrefs() have this value (?) - I get a "trying > to read an unregistered pref" error when I use that. The global process way is > how it's done in profile_chooser_view.cc so I just took it. Ah, nevermind, my brain misfired - kBrowserGuestModeEnabled is indeed a browser-wide preference instead of per-profile (it has to be, obviously :) https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1388: - (CGFloat)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles On 2016/07/20 22:03:46, Jane wrote: > On 2016/07/20 20:54:22, groby wrote: > > Is there any reason this needs to be a mutable array? > > I think it's because this array needs the dynamic insertion operations for its > construction. See L1521 in this patch. Should this be something else? You need that only at construction time, though. The method here doesn't need to modify, so NSArray is fine. (In cocoa, NSArray vs NSMutableArray - same naming for other ObjC containers - is the equivalent of specifying const for C++ containers) https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1536: inContainer:container.get()]; On 2016/07/20 22:03:46, Jane wrote: > On 2016/07/20 20:54:22, groby wrote: > > No reason for .get(), I think. > > Done. I'm assuming all the .get()'s here? (What is .get() usually used for?) get() is usually used if the types don't match exactly - i.e. if you have a scoped_nsobject<NSMutableArray>, but the method takes an NSArray*, you'll need get(). scoped_nsobject<T> safely degrades to T*, though. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2297: - (NSView*)createMaterialDesignOptionsViewWithRect:(NSRect)rect On 2016/07/20 22:03:46, Jane wrote: > On 2016/07/20 20:54:22, groby wrote: > > Traditionally, this would be named create...WithFrame: > > > > (We also wouldn't have a method called createXYZ, but that ship has sailed :) > > Done. But there are soo many methods called createXYZ now... I know - that's why I said the ship has sailed for the create prefix :) https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:2652: ? (kHorizontalSpacing - 2.0) On 2016/07/20 22:03:46, Jane wrote: > On 2016/07/20 20:54:22, groby wrote: > > Why -2.0? (Please leave a comment explaining magic numbers. Even if it's just > > "so it looks right, not sure why we have to move 2.0") > > Done. Is this clear, and is this too long of a comment? It's a bit long - but not exceedingly so. If you can shorten it a bit, great, if not, also good. (I'd rather have a long comment than a cryptic comment :)
https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right): https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1388: - (CGFloat)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles On 2016/07/21 00:01:46, groby wrote: > On 2016/07/20 22:03:46, Jane wrote: > > On 2016/07/20 20:54:22, groby wrote: > > > Is there any reason this needs to be a mutable array? > > > > I think it's because this array needs the dynamic insertion operations for its > > construction. See L1521 in this patch. Should this be something else? > > You need that only at construction time, though. The method here doesn't need to > modify, so NSArray is fine. (In cocoa, NSArray vs NSMutableArray - same naming > for other ObjC containers - is the equivalent of specifying const for C++ > containers) Done. Ah, I see what you mean! I changed it to be NSArray everywhere else. https://codereview.chromium.org/2171473002/diff/1/chrome/browser/ui/cocoa/pro... chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1536: inContainer:container.get()]; On 2016/07/21 00:01:46, groby wrote: > On 2016/07/20 22:03:46, Jane wrote: > > On 2016/07/20 20:54:22, groby wrote: > > > No reason for .get(), I think. > > > > Done. I'm assuming all the .get()'s here? (What is .get() usually used for?) > > get() is usually used if the types don't match exactly - i.e. if you have a > scoped_nsobject<NSMutableArray>, but the method takes an NSArray*, you'll need > get(). scoped_nsobject<T> safely degrades to T*, though. Ah, I see! Now that I modified these two methods to be NSArray, I added back the .get()'s for otherProfiles.
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
The patchset sent to the CQ was uploaded after l-g-t-m from groby@chromium.org Link to the patchset: https://codereview.chromium.org/2171473002/#ps60001 (title: "Final nits")
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 ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ========== to ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 ========== to ========== Bringing back fast user switching on Mac material design user menu Specifically: 1. Migrated the fast user switching buttons view (triggered by right-clicking the avatar button) back to user menu's main page; 2. Added a "Guest" and "Close all your windows" button. Implemented with createMaterialDesignOptionsViewWithRect, which is based on/parallel to createOptionsViewWithRect; 3. Made UI modifications on paddings, seperators/not, using new skia icons, etc. See screenshot: https://drive.google.com/a/google.com/file/d/0B7Fvv7JszRyGTndKT2pBWlhRY2s/view Mocks: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Desktop%20S... Design doc: https://docs.google.com/document/d/17yHrKd_EU6WoIAgljnZmNCSJLRlbM3O2Pxr7aPEnq... BUG=615893 Committed: https://crrev.com/d123727f6a71838bc658c255a685fed19bd34d6e Cr-Commit-Position: refs/heads/master@{#406849} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d123727f6a71838bc658c255a685fed19bd34d6e Cr-Commit-Position: refs/heads/master@{#406849} |