Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2524223002:
MacViews: Anchor bubbles according to the MD spec. (Closed)
DescriptionMacViews: Anchor bubbles according to the MD spec.
MD bubbles no longer use arrows. Instead the top outer-side corner of
the bubble should align with the bottom outer-side corner of the
location bar decoration.
BUG=657263
Committed: https://crrev.com/e917fcdbb7701c1abc3c42d5d08ef8bf347b22ad
Cr-Commit-Position: refs/heads/master@{#435206}
Patch Set 1 #Patch Set 2 : use GetBackgroundView() #Patch Set 3 : Fix BubbleBorder::GetBounds() #
Total comments: 2
Patch Set 4 : Zap unnecessary patchset dep #
Messages
Total messages: 33 (23 generated)
The CQ bit was checked by tapted@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by tapted@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...
Description was changed from ========== MacViews: Anchor bubbles according to the MD spec. BUG=657263 ========== to ========== MacViews: Anchor bubbles according to the MD spec. MD bubbles no longer use arrows. Instead the top outer-side corner of the bubble should align with the bottom outer-side corner of the location bar decoration. BUG=657263 ==========
tapted@chromium.org changed reviewers: + ellyjones@chromium.org
Patchset #4 (id:80001) has been deleted
Patchset #3 (id:60001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
Elly - could you review the cocoa stuff before I send to msw for OWNERS - thanks!
lgtm https://codereview.chromium.org/2524223002/diff/100001/ui/views/bubble/bubble... File ui/views/bubble/bubble_border.cc (right): https://codereview.chromium.org/2524223002/diff/100001/ui/views/bubble/bubble... ui/views/bubble/bubble_border.cc:226: shadow_ == NO_ASSETS ? gfx::Insets() : gfx::Insets(kBorderThicknessDip); what's up with this?
tapted@chromium.org changed reviewers: + msw@chromium.org
+msw for ui/views/bubble/OWNERS (and thanks elly!) https://codereview.chromium.org/2524223002/diff/100001/ui/views/bubble/bubble... File ui/views/bubble/bubble_border.cc (right): https://codereview.chromium.org/2524223002/diff/100001/ui/views/bubble/bubble... ui/views/bubble/bubble_border.cc:226: shadow_ == NO_ASSETS ? gfx::Insets() : gfx::Insets(kBorderThicknessDip); On 2016/11/29 22:25:35, Elly Fong-Jones wrote: > what's up with this? This is needed to get the alignment right - With UseMd() the `NO_ASSETS` border type was still adding borders. I fixed most of it in r434069, but the stuff here was throwing off the alignment slightly. r434069 ensured that the border wasn't painted for NO_ASSETS, but the size shouldn't be adjusted here assiming there is always a border. In theory the GetInsets() on the line below should have taken care of it (it already has an early exit for NO_ASSETS which was added in r434069). However, the `border` local variable there was dead/unused -- I think it needs the separate treatment here to deal with HiDPI stuff anyway.
ui/views/bubble lgtm
The CQ bit was checked by tapted@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2524213002 Patch 20001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by tapted@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org, ellyjones@chromium.org Link to the patchset: https://codereview.chromium.org/2524223002/#ps120001 (title: "Zap unnecessary patchset dep")
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
Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1480499079451400,
"parent_rev": "77b2a4891db6524edb7f9f23bd1924de6270b272", "commit_rev":
"1adf622daac78bb19ee771d7cb96d22d17bc7809"}
Message was sent while issue was closed.
Description was changed from ========== MacViews: Anchor bubbles according to the MD spec. MD bubbles no longer use arrows. Instead the top outer-side corner of the bubble should align with the bottom outer-side corner of the location bar decoration. BUG=657263 ========== to ========== MacViews: Anchor bubbles according to the MD spec. MD bubbles no longer use arrows. Instead the top outer-side corner of the bubble should align with the bottom outer-side corner of the location bar decoration. BUG=657263 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== MacViews: Anchor bubbles according to the MD spec. MD bubbles no longer use arrows. Instead the top outer-side corner of the bubble should align with the bottom outer-side corner of the location bar decoration. BUG=657263 ========== to ========== MacViews: Anchor bubbles according to the MD spec. MD bubbles no longer use arrows. Instead the top outer-side corner of the bubble should align with the bottom outer-side corner of the location bar decoration. BUG=657263 Committed: https://crrev.com/e917fcdbb7701c1abc3c42d5d08ef8bf347b22ad Cr-Commit-Position: refs/heads/master@{#435206} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e917fcdbb7701c1abc3c42d5d08ef8bf347b22ad Cr-Commit-Position: refs/heads/master@{#435206} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
