Global error animated icon |
|||||||||||||||||||||||||
Issue descriptionTo replace the icons that cover the 3 dot menu icon, and alert for software updates and other global error alerts. Specs: https://direct.googleplex.com/#/group/Moticon Reference video: https://drive.google.com/file/d/0B7C_rrbeaTQOUXNnNWV1SVY3b1E/view
,
Apr 3 2017
Issue 648501 has been merged into this issue.
,
Apr 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/064a811986ce8b3bdcb9bd134d4927b9b36fee2a commit 064a811986ce8b3bdcb9bd134d4927b9b36fee2a Author: spqchan <spqchan@chromium.org> Date: Tue Apr 18 16:46:32 2017 [Views] App Menu Animated Icon Implementation of the new app menu animated icon. The new icon is set behind the "app-menu-icon" flag. The flag gives us three options: 1) Old Behavior - Shows the old non-animated icon 2) Persistent Opened State - Shows the new icon which animated and stays in an opened state. The animation is only triggered when a new window is created 3) Persistent Closed State - Shows the new icon which animates to an opened state before it animates back to a closed state. The animation is triggered when a new window/tab is created and when the menu is opened. The flag's default option is the "Old Behavior" BUG= 704786 Review-Url: https://codereview.chromium.org/2789203003 Cr-Commit-Position: refs/heads/master@{#465267} [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/about_flags.cc [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/flag_descriptions.h [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/toolbar/app_menu_icon_controller.cc [add] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/views/toolbar/app_menu_animation.cc [add] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/views/toolbar/app_menu_animation.h [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/views/toolbar/app_menu_button.cc [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/views/toolbar/app_menu_button.h [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/common/chrome_switches.cc [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/chrome/common/chrome_switches.h [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/tools/metrics/histograms/histograms.xml [modify] https://crrev.com/064a811986ce8b3bdcb9bd134d4927b9b36fee2a/ui/views/controls/button/label_button.h
,
Apr 20 2017
The icon is available on Canary on Linux and Windows You can play with it by changing the #app-menu-icon flag. I implemented Option 1 and Option 3 from Helene's slides. Option 1 is "Persistent Opened State" and Option 3 is "Persistent Closed State" You can simulate the update behavior with the --simulate-critical-update flag. Let me know if you have any questions!
,
Apr 20 2017
Screencast of the animation for Option 3: https://screencast.googleplex.com/cast/NDcxOTg2MDcyNjYyODM1MnxlMjg1NWJkOS1iMg
,
Apr 27 2017
Discussion with helenepark@ and bettes@: "Persistent Opened State" will be killed and there needs to be a small fix for the animation. Once that's done, I'll start implementing this on Mac
,
Apr 27 2017
Issue 715760 has been merged into this issue.
,
Apr 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f9127a09a54711eed16c5e386ed02453309ed91 commit 6f9127a09a54711eed16c5e386ed02453309ed91 Author: spqchan <spqchan@chromium.org> Date: Sat Apr 29 03:42:02 2017 [Views] App Menu Icon Update Removed the "Persistent Open State" behavior and changed the "app-menu-icon" flag to "enable-new-app-menu-icon". Fixed an issue with the animation where it transitions between different severity levels with an incorrect color. BUG= 704786 Review-Url: https://codereview.chromium.org/2843413003 Cr-Commit-Position: refs/heads/master@{#468222} [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/about_flags.cc [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/flag_descriptions.h [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/ui/views/toolbar/app_menu_animation.cc [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/ui/views/toolbar/app_menu_animation.h [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/ui/views/toolbar/app_menu_button.cc [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/browser/ui/views/toolbar/app_menu_button.h [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/common/chrome_switches.cc [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/chrome/common/chrome_switches.h [modify] https://crrev.com/6f9127a09a54711eed16c5e386ed02453309ed91/tools/metrics/histograms/histograms.xml
,
May 3 2017
Feedback on the new app icon in Views: The icon is blurry in its steady state. The old, non-animated icon takes care to achieve pixel alignment with separate 1x and !1x definitions (sgabriel is responsible for this). See attachment: old on top, new/animated on bottom. I would suggest that maintaining pixel perfection in the steady state should be a requirement when adding motion. (Note that I'm working on a more general solution to adding motion to icons, and I aim for this to be one thing that it handles.)
,
May 3 2017
Thanks for pointing it out. Do you have any suggestions on how to draw it pixel perfectly? If there's not one for drawing, one thing I can do is to display the vector when it reaches it's steady state Also, can you elaborate on the "general solution to adding motion to icons"?
,
May 3 2017
> Do you have any suggestions on how to draw it pixel perfectly? This is a matter of concern for the designer who provides the spec for an icon. If you look at static icons, we often have 1x and >1x versions, both of which come from separate SVGs that the designer provides after making sure they look good at the appropriate scale. This might entail half-pixel offsets, differing stroke widths, etc. > one thing I can do is to display the vector when it reaches it's steady state That might not be a bad solution as long as the transition is not noticeable. It is one of the approaches under consideration for the general solution. And on that note, the "general solution" is a framework I'm working on for specifying animated icons so that we don't need lots of bespoke code for every place we want to add animations. My intention is for the definition to look something like an Android AnimatedVectorDrawable.[1] It would eventually be applicable to the app menu icon, but it's not close to being ready yet. Assuming we want to launch the animated app icon sooner rather than later, we probably shouldn't be waiting around for the general solution. [1] https://developer.android.com/reference/android/graphics/drawable/AnimatedVectorDrawable.html
,
May 4 2017
Sounds good. Since we want to launch the animated app icon soon, I give that solution a try. Once your framework is ready, we can switch to that.
,
May 5 2017
Here is another issue that seems important to me: theming. Since we color the upgrade dots red no matter what the browser theme, a) you won't be able to even see the dots on a theme with a red bg b) you won't be able to determine the state of the icon if the icon is normally red This was not much of an issue previously because the upgrade icon used both shape and color to distinguish itself, and a white arrow on a red bg would always be visible.
,
May 5 2017
Also, I should note that after a day of trying something more complicated I found a promising way to extend vector icon definitions to include animation (i.e. the framework mentioned above) and I don't think it will take that long to land it. This should address the blurriness but we still need some design input to fix the theming.
,
May 5 2017
#14 How soon do you think you can land it?
,
May 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6dc6477fbfd23d601d72dc1ad941699c65a47353 commit 6dc6477fbfd23d601d72dc1ad941699c65a47353 Author: spqchan <spqchan@chromium.org> Date: Sat May 06 19:23:17 2017 Refactored AppMenuAnimation Refactored and moved AppMenuAnimation out of Views so that it can be used in cross platforms. AppMenuAnimation no longer paints the dot, instead it will handle the animation and delegate the painting to AppMenuAnimationDelegate. BUG= 704786 Review-Url: https://codereview.chromium.org/2858313002 Cr-Commit-Position: refs/heads/master@{#469885} [modify] https://crrev.com/6dc6477fbfd23d601d72dc1ad941699c65a47353/chrome/browser/ui/BUILD.gn [rename] https://crrev.com/6dc6477fbfd23d601d72dc1ad941699c65a47353/chrome/browser/ui/toolbar/app_menu_animation.cc [rename] https://crrev.com/6dc6477fbfd23d601d72dc1ad941699c65a47353/chrome/browser/ui/toolbar/app_menu_animation.h [modify] https://crrev.com/6dc6477fbfd23d601d72dc1ad941699c65a47353/chrome/browser/ui/views/toolbar/app_menu_button.cc [modify] https://crrev.com/6dc6477fbfd23d601d72dc1ad941699c65a47353/chrome/browser/ui/views/toolbar/app_menu_button.h
,
May 8 2017
re: 15 it's code complete and the first patch is in review. I don't know when exactly it will all land. I am still concerned that the design needs to take themes into consideration (as well as the accessibility angle --- not sure if changing the color alone is enough of a distinction for colorblind users). Otherwise I could have an animated version of browser_tools.icon ready in short order.
,
May 8 2017
Great, I'll hold my work for the Mac implementation then
,
May 10 2017
Regarding the theming question, assuming that we're building on top of https://developer.android.com/reference/android/graphics/drawable/AnimatedVectorDrawable.html we might just want t extend that API to support "default" as a fill color (and/or strokeColor) in a path <vector <group <path android:fillColor="default" />/>/>
,
May 10 2017
The objective is to make the menu visible on the toolbar in the case of an error. a) you won't be able to even see the dots on a theme with a red bg b) you won't be able to determine the state of the icon if the icon is normally red The UI in both of these scenarios are no worse then the situation we have today. Obviously, it's not the ideal solution, but it doesn't make things worse and it solves for our original problem. There are plenty of instances where the experience on Chrome is degraded when personalizing the UI (https://bugs.chromium.org/p/chromium/issues/detail?id=646060#c8 ), but I think that it's reasonable to optimize for the default state first and support the other cases as much as we can.
,
May 17 2017
Discussed in a separate thread: We'll be sticking to the default or theme defined colors (regardless of the severity level) if the browser is using a theme.
,
May 18 2017
Decision from meeting: On themed chrome toolbars: use animation only On default chrome toolbars: use animation and color Thanks!
,
May 31 2017
Hi Helene, is https://direct.googleplex.com/#/group/Moticon up to date? I remember reading somewhere that the spec had been tweaked slightly but that Direct description hadn't been updated to match. If not, could you update it?
,
Jun 1 2017
The animation in this direct spec is the most up to date. Which speec has been tweaked? On the color or the animation, or both?
,
Jun 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d068dc8a60aea79f24f02f6a659972936387087 commit 9d068dc8a60aea79f24f02f6a659972936387087 Author: estade <estade@chromium.org> Date: Thu Jun 01 20:48:02 2017 Use animated vector icon for app menu notification animation. 1. Expose IconDescription in vector icon code so when we add or modify parameters we don't have to update the API. TODO: remove the parts of the API that use multiple parameters and replace them all with IconDescription. 2. Add AnimatedIconView, which caches the start and end states and controls animating in between as well. 3. Add an animated version of browser_tools.icon. This was crafted by hand based on the non-animated version and a visual description, but in the future designers should deliver Android vector drawables which will make the process of creating these icons much easier (and perhaps automate-able). TODO: actually craft a 2x version instead of reusing 1x. 4. Apply this all to the app menu. This solves the issue of pixel perfection in the steady state. BUG= 718549 , 704786 Review-Url: https://codereview.chromium.org/2892563004 Cr-Commit-Position: refs/heads/master@{#476425} [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/app/vector_icons/BUILD.gn [add] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/app/vector_icons/browser_tools_animated.1x.icon [add] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/app/vector_icons/browser_tools_animated.icon [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/app/vector_icons/vector_icons.cc.template [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/browser/ui/BUILD.gn [delete] https://crrev.com/81ba45d614d7e3abd6f4d9062baab9f43c5f283b/chrome/browser/ui/toolbar/app_menu_animation.cc [delete] https://crrev.com/81ba45d614d7e3abd6f4d9062baab9f43c5f283b/chrome/browser/ui/toolbar/app_menu_animation.h [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/browser/ui/views/toolbar/app_menu_button.cc [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/chrome/browser/ui/views/toolbar/app_menu_button.h [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/ui/gfx/paint_vector_icon.cc [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/ui/gfx/paint_vector_icon.h [modify] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/ui/views/BUILD.gn [add] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/ui/views/controls/animated_icon_view.cc [add] https://crrev.com/9d068dc8a60aea79f24f02f6a659972936387087/ui/views/controls/animated_icon_view.h
,
Jun 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6343a446839e50636e78352df4627c61edbfe4f8 commit 6343a446839e50636e78352df4627c61edbfe4f8 Author: Evan Stade <estade@chromium.org> Date: Tue Jun 06 17:35:00 2017 Update browser_tools_animated.icon for pixel perfection. Starting with browser_tools.icon as a baseline, apply animations. This brings it up to par with the 1x version. Bug: 704786 Change-Id: I7ed787d432bac63bd72234f5e6bec5e1b877b7d3 Reviewed-on: https://chromium-review.googlesource.com/524733 Reviewed-by: Terry Anderson <tdanderson@chromium.org> Commit-Queue: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/heads/master@{#477329} [modify] https://crrev.com/6343a446839e50636e78352df4627c61edbfe4f8/chrome/app/vector_icons/browser_tools_animated.1x.icon [modify] https://crrev.com/6343a446839e50636e78352df4627c61edbfe4f8/chrome/app/vector_icons/browser_tools_animated.icon
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da58dc6dc45f1be6496a1819bad563de20fead00 commit da58dc6dc45f1be6496a1819bad563de20fead00 Author: spqchan <spqchan@chromium.org> Date: Fri Jun 16 21:12:06 2017 [Mac] Implement the Animated App Menu Icon - Use the new animated icon for the App Menu - The icon is used when the "new-app-menu-icon" flag is enabled. - The animation should be triggered when: - A new window/tab is created - The app menu is opened - To use animated .icon files in Cocoa, AnimatedIcon is create to control the animation and draw the icon. Bug: 704786 Change-Id: Idb43507b4a278b2115f8d46f8172dee61721c68d Reviewed-on: https://chromium-review.googlesource.com/530030 Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Sarah Chan <spqchan@chromium.org> Cr-Commit-Position: refs/heads/master@{#480169} [modify] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/BUILD.gn [add] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/cocoa/animated_icon.h [add] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/cocoa/animated_icon.mm [modify] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm [modify] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/cocoa/browser_window_controller.mm [modify] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h [modify] https://crrev.com/da58dc6dc45f1be6496a1819bad563de20fead00/chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm
,
Jun 17 2017
Hi spqchan@: In latest Chromium Build 480307 the Menu Button is missing. Probably caused by your change? It must be broken between build 480156 and 480174: https://chromium.googlesource.com/chromium/src/+log/cf5dbc034cc63e47e35f758b37af2eed2368124e..0c2987eee004d427ace7113e539b93f5a1946979 I am on a MacBook Air 11" (NonRetina) with macOS 10.12.5. Please find attached a screenshot.
,
Jun 20 2017
Tested the issue on Win-10, Mac 10.12.5 and Ubuntu 14.04 using chrome dev version #61.0.3135.4. Attached a screen cast and screen shots for reference. Following are the steps followed to reproduce the issue. ------------ 1. Launched chrome. 2. Enabled the flag "new-app-menu-icon" in chrome://flags. 3. Relaunched chrome. 4. Observed that the 3-dot menu color got changed to red as in 3-dot_menu.jpg. Note: In OS-Mac, even after enabling the flag "new-app-menu-icon" in chrome://flags, the 3-dot menu color did not get changed(as in a 3-dot_menu@mac.png) spqchan@ - Could you please verify the attached screen cast and screen shots and please let us know if it is the expected behavior. Also please confirm the expected behavior of OS-Mac. Thanks...!!
,
Jun 20 2017
The CL got reverted since it was causing some issues. I just relanded it with a fix
,
Jun 22 2017
This is now implemented on all Desktop platforms
,
Jun 27 2017
Tested the issue on Win-10, Mac 10.12.5 and Ubuntu 14.04 using chrome dev version #61.0.3141.0. Attached a screen cast and screen shot for reference. Following are the steps followed to reproduce the issue. ------------ 1. Launched chrome. 2. Enabled the flag "new-app-menu-icon" in chrome://flags. 3. Relaunched chrome. 4. Observed that the 3-dot menu color got changed to red as in "animated icon@Win_and_Linux". Note: In OS-Mac, the 3-dot menu seemed to be changing its shape as in globalerror.mov (Comment #0) when clicking on it. spqchan@ - Could you please verify the attached screen casts and screen shot and please let us know if it is the expected behavior. Also please confirm the expected behavior of OS-Mac. Thanks...!!
,
Jun 28 2017
Hmm, the icon is suppose to animate when it's clicked or a new tab gets inserted. Looks like that behavior got regressed on Windows/Linux. I won't be able to work on it now because I'm currently working remotely and won't have access to my Windows machine. I'll look at this when I get back Also, I noticed that this bug includes iOS. Do you mean ChromeOS?
,
Jun 28 2017
,
Jun 28 2017
Yes, that's correct. Removed iOS and added ChromeOS
,
Jun 30 2017
Opened a launch bug here: https://bugs.chromium.org/p/chromium/issues/detail?id=738608 Sarah, let me know when the feature is fixed and ready for reviews and I'll initiate them.
,
Jul 5 2017
On OSX - 2nd tab creation of the 1st window doesn't show the animation, which seems like a bug Screen recording: https://drive.google.com/file/d/0B63LqjsMmAX_cFI1YUpQbk1pT3M/view?pli=1
,
Jul 6 2017
At #37: I can't reproduce this. What version are you on?
,
Jul 6 2017
c#38 - it's 61.0.3150.0
,
Jul 10 2017
I'm back from vacation #32 You should be testing this with the --simulate-critical-update flag. The red icon color for zero updates is a bug, I have a fix for it #39 I still can't replicate it but I suspect it has to do with animation timing. I'm working on something that might fix it
,
Jul 11 2017
Hello, It looks like there is a misalignment on a non-retina display (1x, not 2x) Hwi mentioned that the dots look misaligned by 1 point off. Also the red dot and the regular grey dot use different pixels e.g. 3x3 vs. 4x4, which seem related to the mis-alignment.
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b79bb8538ba6be9a898f414e5c768c442773a7f commit 8b79bb8538ba6be9a898f414e5c768c442773a7f Author: spqchan <spqchan@chromium.org> Date: Wed Jul 12 00:04:12 2017 [Mac] Change the Animated App Menu Icon's Behavior The icon should restart from the beginning each time it animates. Bug: 704786 Change-Id: I4c00a92023fe8dde2bdf7a92659d00a29476648c Reviewed-on: https://chromium-review.googlesource.com/567207 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Sarah Chan <spqchan@chromium.org> Cr-Commit-Position: refs/heads/master@{#485734} [modify] https://crrev.com/8b79bb8538ba6be9a898f414e5c768c442773a7f/chrome/browser/ui/cocoa/animated_icon.h [modify] https://crrev.com/8b79bb8538ba6be9a898f414e5c768c442773a7f/chrome/browser/ui/cocoa/animated_icon.mm
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67f13dd5c65cccd6b90f8b3a68241e4ddba67f63 commit 67f13dd5c65cccd6b90f8b3a68241e4ddba67f63 Author: spqchan <spqchan@chromium.org> Date: Wed Jul 12 01:49:51 2017 [Views] Fix Color Issue With App Menu Icon When the icon's color is updated, the static image should also be updated. Bug: 704786 Change-Id: I123da5c43b57d0ec4343a2497a0ab0c711e46803 Reviewed-on: https://chromium-review.googlesource.com/566025 Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Sarah Chan <spqchan@chromium.org> Cr-Commit-Position: refs/heads/master@{#485785} [modify] https://crrev.com/67f13dd5c65cccd6b90f8b3a68241e4ddba67f63/chrome/browser/ui/views/toolbar/app_menu_button.cc [modify] https://crrev.com/67f13dd5c65cccd6b90f8b3a68241e4ddba67f63/ui/views/controls/animated_icon_view.cc [modify] https://crrev.com/67f13dd5c65cccd6b90f8b3a68241e4ddba67f63/ui/views/controls/animated_icon_view.h
,
Jul 12 2017
Issues list in #40 are addressed. Will look into #41. I don't think that is a blocking issue though
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e24ae394d8f58cd49025ec8822ea72ce09e73dcf commit e24ae394d8f58cd49025ec8822ea72ce09e73dcf Author: spqchan <spqchan@chromium.org> Date: Fri Jul 21 16:38:42 2017 [Mac] Fix alignment for App Menu Icon The animated App Menu Icon is misaligned by 1pt on non-retina screens. To fix this, the icon needs to be translated by 0.5pt. Bug: 704786 Change-Id: Ida9155be424eb9ea46acb6f6ae41d245de3ae28c Reviewed-on: https://chromium-review.googlesource.com/580694 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Sarah Chan <spqchan@chromium.org> Cr-Commit-Position: refs/heads/master@{#488677} [modify] https://crrev.com/e24ae394d8f58cd49025ec8822ea72ce09e73dcf/chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm
,
Jul 21 2017
The alignment issue is fixed. However, I'm unable to reproduce the issue hwi@ has with the animation on my machine. I'll dig into it and hopefully will come up with something
,
Aug 8 2017
Sarah, looks like this is 62 unless you can merge the change in #45 to 61. Is this feasible?
,
Aug 8 2017
Merge request for the CL in #45
,
Aug 8 2017
The bug is marked as P3 or Feature. It should not be merged as M61 is in beta. Please contact the approriate milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 8 2017
I'm going to change this to P2, since it's actually something we want.
,
Aug 8 2017
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 8 2017
Before we approve merge to M61, please answer followings: * Is this M61 regression? Is it critical? * Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61? * Any other important details to justify the merge. Please note M61 is already in Beta, so merge bar is very high. Thank you.
,
Aug 8 2017
1) It's not a regression, it is critical for this feature. 2) Yes, it has been baked/verified in Canary for over two weeks. Sorry for the late merge request!
,
Aug 8 2017
Thank you Sarah. Approving merge for CL listed at #45 to M61 branch 3163 based on comment #53.
,
Aug 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4df93687915eb977eeca0ad7b30bf8c74b3fabbe commit 4df93687915eb977eeca0ad7b30bf8c74b3fabbe Author: spqchan <spqchan@chromium.org> Date: Tue Aug 08 22:29:55 2017 [Mac] Fix alignment for App Menu Icon The animated App Menu Icon is misaligned by 1pt on non-retina screens. To fix this, the icon needs to be translated by 0.5pt. (cherry picked from commit e24ae394d8f58cd49025ec8822ea72ce09e73dcf) Bug: 704786 Change-Id: Ida9155be424eb9ea46acb6f6ae41d245de3ae28c Reviewed-on: https://chromium-review.googlesource.com/580694 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Sarah Chan <spqchan@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#488677} Reviewed-on: https://chromium-review.googlesource.com/607348 Reviewed-by: Sarah Chan <spqchan@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#390} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/4df93687915eb977eeca0ad7b30bf8c74b3fabbe/chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm
,
Nov 29 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by spqc...@chromium.org
, Mar 27 2017