Issue metadata
Sign in to add a comment
|
[Touchable] Badged extension icons overlap toolbar border |
||||||||||||||||||||||
Issue descriptionChrome Version: 67 (r549974) OS: Chrome OS What steps will reproduce the problem? With --top-chrome-md=material-touch-optimized (1) Install extension https://chrome.google.com/webstore/detail/quick-tabs/jnjfeinjfmenlddahdjdmgpbokiacbbb What is the expected result? Extension has a badge, within the toolbar bounds. What happens instead? Extension has a badge (showing the number of open tabs). It is too low, overlapping the toolbar border and generally looking terrible.
,
Apr 12 2018
My 2cents, we should move them and there's also an opportunity to make them slightly larger, which was an issue as I can remember at the time on regular Chrome.
,
Apr 12 2018
Yeah, the badge probably is currently positioned based on the button size, but needs to be positioned based on the icon size. Also, making things larger during the process would be a win. There's a separate bug on the size of the badge, the padding inside, and the font size/style, but I don't have time to look it up right now.
,
Apr 13 2018
,
Apr 13 2018
I found out that there are other types of decorations that should also change as part of this bug: - The Blocked action decoration (the red x at the top right). - The page action decoration (the blue circle at the bottom left)
,
Apr 13 2018
,
Apr 13 2018
re#7: this is the same thing I saw, sorry I didn't provide screenshots!
,
Apr 13 2018
So here are the changes sgabriel@ and I agreed on over IM (See attached screenshot): - Make all badges / decorators positioned at the corners of the icon area, not the full button area. Changes specific to touchable Chrome: - Make the badge height 12dp instead of 11dp. - Make the page action decoration inner blue circle 1dp smaller than the outer white circle.
,
Apr 13 2018
Oh, and also: - Badge's inner rounded corner radius is 2dp, and out is 3dp. This change is also limited to Touchable Chrome.
,
Apr 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94f048689670a751702fdd2e3c0b56e403d38964 commit 94f048689670a751702fdd2e3c0b56e403d38964 Author: Ahmed Fakhry <afakhry@google.com> Date: Sat Apr 14 01:59:35 2018 Fix extensions badges and decorations positions The extensions browser actions badges and decorations should be positioned at the corners of the icon area, rather than the corners of the full button's area (i.e. ignoring the paddings). Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c11 BUG= 831946 Change-Id: I977883a3e6127e1f91779a7e73b4adb7fa7c6471 Reviewed-on: https://chromium-review.googlesource.com/1013290 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#550855} [modify] https://crrev.com/94f048689670a751702fdd2e3c0b56e403d38964/chrome/browser/ui/extensions/icon_with_badge_image_source.cc [modify] https://crrev.com/94f048689670a751702fdd2e3c0b56e403d38964/chrome/browser/ui/extensions/icon_with_badge_image_source.h
,
Apr 16 2018
I'm reverting the above CL. This won't work for PWAs properly. I will reland with a better fix so that it can be merged to M-67 as a single CL.
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71a2cc1b3292d9b4c5128da2db008e843e3b608c commit 71a2cc1b3292d9b4c5128da2db008e843e3b608c Author: Ahmed Fakhry <afakhry@chromium.org> Date: Mon Apr 16 17:25:36 2018 Revert "Fix extensions badges and decorations positions" This reverts commit 94f048689670a751702fdd2e3c0b56e403d38964. Reason for revert: This doesn't work for PWAs, I will reland with a better fix so that it can be merged as a single CL. Original change's description: > Fix extensions badges and decorations positions > > The extensions browser actions badges and decorations should > be positioned at the corners of the icon area, rather than the > corners of the full button's area (i.e. ignoring the paddings). > > Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c11 > > BUG= 831946 > > Change-Id: I977883a3e6127e1f91779a7e73b4adb7fa7c6471 > Reviewed-on: https://chromium-review.googlesource.com/1013290 > Reviewed-by: Devlin <rdevlin.cronin@chromium.org> > Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> > Cr-Commit-Position: refs/heads/master@{#550855} TBR=thestig@chromium.org,rdevlin.cronin@chromium.org,afakhry@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 831946 Change-Id: I718ab6842d4bf55f5f1cf6f833e3638974c2fea8 Reviewed-on: https://chromium-review.googlesource.com/1012734 Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#551020} [modify] https://crrev.com/71a2cc1b3292d9b4c5128da2db008e843e3b608c/chrome/browser/ui/extensions/icon_with_badge_image_source.cc [modify] https://crrev.com/71a2cc1b3292d9b4c5128da2db008e843e3b608c/chrome/browser/ui/extensions/icon_with_badge_image_source.h
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94f048689670a751702fdd2e3c0b56e403d38964 commit 94f048689670a751702fdd2e3c0b56e403d38964 Author: Ahmed Fakhry <afakhry@google.com> Date: Sat Apr 14 01:59:35 2018 Fix extensions badges and decorations positions The extensions browser actions badges and decorations should be positioned at the corners of the icon area, rather than the corners of the full button's area (i.e. ignoring the paddings). Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c11 BUG= 831946 Change-Id: I977883a3e6127e1f91779a7e73b4adb7fa7c6471 Reviewed-on: https://chromium-review.googlesource.com/1013290 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#550855} [modify] https://crrev.com/94f048689670a751702fdd2e3c0b56e403d38964/chrome/browser/ui/extensions/icon_with_badge_image_source.cc [modify] https://crrev.com/94f048689670a751702fdd2e3c0b56e403d38964/chrome/browser/ui/extensions/icon_with_badge_image_source.h
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71a2cc1b3292d9b4c5128da2db008e843e3b608c commit 71a2cc1b3292d9b4c5128da2db008e843e3b608c Author: Ahmed Fakhry <afakhry@chromium.org> Date: Mon Apr 16 17:25:36 2018 Revert "Fix extensions badges and decorations positions" This reverts commit 94f048689670a751702fdd2e3c0b56e403d38964. Reason for revert: This doesn't work for PWAs, I will reland with a better fix so that it can be merged as a single CL. Original change's description: > Fix extensions badges and decorations positions > > The extensions browser actions badges and decorations should > be positioned at the corners of the icon area, rather than the > corners of the full button's area (i.e. ignoring the paddings). > > Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c11 > > BUG= 831946 > > Change-Id: I977883a3e6127e1f91779a7e73b4adb7fa7c6471 > Reviewed-on: https://chromium-review.googlesource.com/1013290 > Reviewed-by: Devlin <rdevlin.cronin@chromium.org> > Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> > Cr-Commit-Position: refs/heads/master@{#550855} TBR=thestig@chromium.org,rdevlin.cronin@chromium.org,afakhry@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 831946 Change-Id: I718ab6842d4bf55f5f1cf6f833e3638974c2fea8 Reviewed-on: https://chromium-review.googlesource.com/1012734 Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#551020} [modify] https://crrev.com/71a2cc1b3292d9b4c5128da2db008e843e3b608c/chrome/browser/ui/extensions/icon_with_badge_image_source.cc [modify] https://crrev.com/71a2cc1b3292d9b4c5128da2db008e843e3b608c/chrome/browser/ui/extensions/icon_with_badge_image_source.h
,
Apr 18 2018
Updated screenshots after discussing with sgabriel@ and bettes@ The page action decoration circle dimensions were changed to match those of the tab's attention indicator circle.
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b30852034d639c587d68f50b3859266056dae4de commit b30852034d639c587d68f50b3859266056dae4de Author: Ahmed Fakhry <afakhry@google.com> Date: Wed Apr 18 03:40:56 2018 Reland: Fix extensions badges and decorations positions The extensions browser actions badges and decorations should be positioned at the corners of the icon area, rather than the corners of the full button's area (i.e. ignoring the paddings). Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c17 BUG= 831946 Change-Id: Ib7f4123aa2349dcba5c10e34927d5ef787c53764 Reviewed-on: https://chromium-review.googlesource.com/1012868 Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#551570} [modify] https://crrev.com/b30852034d639c587d68f50b3859266056dae4de/chrome/browser/ui/extensions/icon_with_badge_image_source.cc [modify] https://crrev.com/b30852034d639c587d68f50b3859266056dae4de/chrome/browser/ui/extensions/icon_with_badge_image_source.h [modify] https://crrev.com/b30852034d639c587d68f50b3859266056dae4de/chrome/browser/ui/toolbar/toolbar_actions_bar.cc [modify] https://crrev.com/b30852034d639c587d68f50b3859266056dae4de/chrome/browser/ui/toolbar/toolbar_actions_bar.h [modify] https://crrev.com/b30852034d639c587d68f50b3859266056dae4de/chrome/browser/ui/views/frame/hosted_app_button_container.cc
,
Apr 19 2018
,
Apr 19 2018
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 19 2018
kbleicher@, to clarify the merge request, it is only for the CL in #18.
,
Apr 20 2018
That's a lot of modified lines for a UI change... Assume it's been tested on ToT and qualified as safe to go?
,
Apr 20 2018
That CL is only (+71 -34) lines of changes; it's quite small. The UI changes are necessary to fix an actual bug. This fix seems to be working fine on ToT.
,
Apr 23 2018
Guess it's perspective. That seems like a lot of lines from my point of view since our goal is stability ;-) Assume the ToT testing was against more than one board?
,
Apr 23 2018
This fix is not board-specific. However, it was tested on eve and chromeos linux build. To be clear, this Cl was fixing a regression and we can't release M-67 without this regression being fixed.
,
Apr 24 2018
Thanks for the added details. Approving merge to M67 Chrome OS.
,
Apr 24 2018
Issue 836456 has been merged into this issue.
,
Apr 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7 commit 7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7 Author: Ahmed Fakhry <afakhry@google.com> Date: Wed Apr 25 00:08:18 2018 [Merge to M-67] Reland: Fix extensions badges and decorations positions The extensions browser actions badges and decorations should be positioned at the corners of the icon area, rather than the corners of the full button's area (i.e. ignoring the paddings). Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c17 TBR=rdevlin.cronin@chromium.org, pkasting@chromium.org BUG= 831946 (cherry picked from commit b30852034d639c587d68f50b3859266056dae4de) Change-Id: Ib7f4123aa2349dcba5c10e34927d5ef787c53764 Reviewed-on: https://chromium-review.googlesource.com/1012868 Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#551570} Reviewed-on: https://chromium-review.googlesource.com/1026947 Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#275} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7/chrome/browser/ui/extensions/icon_with_badge_image_source.cc [modify] https://crrev.com/7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7/chrome/browser/ui/extensions/icon_with_badge_image_source.h [modify] https://crrev.com/7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7/chrome/browser/ui/toolbar/toolbar_actions_bar.cc [modify] https://crrev.com/7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7/chrome/browser/ui/toolbar/toolbar_actions_bar.h [modify] https://crrev.com/7c4c921453a7ccfcad8f2d766d59a8cd1f82d0d7/chrome/browser/ui/views/frame/hosted_app_button_container.cc
,
Apr 25 2018
,
Apr 30 2018
,
Apr 30 2018
Re#30: avi@ That is due to a different issue that is specific to Mac and is tracked by issue 825157 . This issue however is fixed. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by mgiuca@chromium.org
, Apr 12 2018