Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2439793002:
Harmony - Draw bubble/dialog borders just outside the content area (Closed)
|
Created:
4 years, 2 months ago by Evan Stade Modified:
4 years, 2 months ago Reviewers:
sky CC:
chromium-reviews, tfarina, groby+bubble_chromium.org, rouslan+bubble_chromium.org, msw+watch_chromium.org, hcarmona+bubble_chromium.org, Peter Kasting Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionHarmony - Draw bubble/dialog borders just outside the content area
instead of just inside of it.
As noted in code comments, this creates minor graphical bugs at scale
factors greater than 1, but I don't think they can be addressed at the
moment.
BUG=655589, 656662
Committed: https://crrev.com/b1448a03ddd9c30a6711cb485ba0614a5e41579e
Cr-Commit-Position: refs/heads/master@{#426570}
Patch Set 1 #Patch Set 2 : self review #
Total comments: 2
Patch Set 3 : s/px/dip #Messages
Total messages: 18 (9 generated)
estade@chromium.org changed reviewers: + sky@chromium.org
+sky for review +pkasting for FYI --- note px/dip positioning bug. At scale=2, the bookmark bubble's border is 1px below the omnibox border instead of right on top. At scale=1, it looks right.
The CQ bit was checked by estade@chromium.org 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...
https://codereview.chromium.org/2439793002/diff/20001/ui/views/bubble/bubble_... File ui/views/bubble/bubble_border.cc (right): https://codereview.chromium.org/2439793002/diff/20001/ui/views/bubble/bubble_... ui/views/bubble/bubble_border.cc:222: const gfx::Insets border_insets = gfx::Insets(kBorderThicknessPx); I find usage of a Px here confusing as it is dips here.
https://codereview.chromium.org/2439793002/diff/20001/ui/views/bubble/bubble_... File ui/views/bubble/bubble_border.cc (right): https://codereview.chromium.org/2439793002/diff/20001/ui/views/bubble/bubble_... ui/views/bubble/bubble_border.cc:222: const gfx::Insets border_insets = gfx::Insets(kBorderThicknessPx); On 2016/10/20 16:23:17, sky wrote: > I find usage of a Px here confusing as it is dips here. I tried to address this in the comment above. What would you suggest?
Not using Px, with your comment that you want it to be pixel but can't. On Thu, Oct 20, 2016 at 9:26 AM, <estade@chromium.org> wrote: > > https://codereview.chromium.org/2439793002/diff/20001/ui/views/bubble/bubble_... > File ui/views/bubble/bubble_border.cc (right): > > https://codereview.chromium.org/2439793002/diff/20001/ui/views/bubble/bubble_... > ui/views/bubble/bubble_border.cc:222: const gfx::Insets border_insets = > gfx::Insets(kBorderThicknessPx); > On 2016/10/20 16:23:17, sky wrote: >> I find usage of a Px here confusing as it is dips here. > > I tried to address this in the comment above. What would you suggest? > > https://codereview.chromium.org/2439793002/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by estade@chromium.org to run a CQ dry run
something like this?
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Yes, thanks. LGTM
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 estade@chromium.org
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.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Harmony - Draw bubble/dialog borders just outside the content area instead of just inside of it. As noted in code comments, this creates minor graphical bugs at scale factors greater than 1, but I don't think they can be addressed at the moment. BUG=655589,656662 ========== to ========== Harmony - Draw bubble/dialog borders just outside the content area instead of just inside of it. As noted in code comments, this creates minor graphical bugs at scale factors greater than 1, but I don't think they can be addressed at the moment. BUG=655589,656662 Committed: https://crrev.com/b1448a03ddd9c30a6711cb485ba0614a5e41579e Cr-Commit-Position: refs/heads/master@{#426570} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/b1448a03ddd9c30a6711cb485ba0614a5e41579e Cr-Commit-Position: refs/heads/master@{#426570} |
