Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
152843004:
Move BookmarkBubble with anchor (Closed)
|
Created:
6 years, 10 months ago by oshima Modified:
6 years, 10 months ago Reviewers:
msw CC:
chromium-reviews, msw+watch_chromium.org, alicet1, tfarina Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionMove BookmarkBubble with anchor
BUG=343049
R=msw@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251019
Patch Set 1 #
Total comments: 3
Patch Set 2 : move the bubble with anchor #Messages
Total messages: 13 (0 generated)
mike, windows virtual keyboard seems to resize the window automatically, which is causing this bug. This is a bit hacky, so let me know if you have a better idea. I'll add test if this is ok with you. Thanks!
https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... File ui/views/bubble/bubble_delegate.cc (right): https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... ui/views/bubble/bubble_delegate.cc:182: // Do not close if the widget gets shrinked vertically without move. This seems odd, and I don't think this is the right fix. The widget bounds may have the same origin and height, but the anchor bounds within that widget still may have changed (the anchor view may have moved within the bounds of the widget, due to layout changes). I think we could probably just close if the widget origin changes, or the anchor bounds have changed; that should solve your bug. LMK if you need help.
https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... File ui/views/bubble/bubble_delegate.cc (right): https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... ui/views/bubble/bubble_delegate.cc:182: // Do not close if the widget gets shrinked vertically without move. On 2014/02/13 00:32:48, msw wrote: > This seems odd, and I don't think this is the right fix. The widget bounds may > have the same origin and height, but the anchor bounds within that widget still > may have changed (the anchor view may have moved within the bounds of the > widget, due to layout changes). I think we could probably just close if the > widget origin changes, or the anchor bounds have changed; that should solve your > bug. LMK if you need help. You're right that anchor position may change even if only the height changes, but for the same reason, we need to close when the width changes because the location of star icon does change when width changes. I actually wonder why bookmark bubble isn't set to move with anchor. Do you know why?
https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... File ui/views/bubble/bubble_delegate.cc (right): https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... ui/views/bubble/bubble_delegate.cc:182: // Do not close if the widget gets shrinked vertically without move. On 2014/02/13 01:28:29, oshima wrote: > On 2014/02/13 00:32:48, msw wrote: > > This seems odd, and I don't think this is the right fix. The widget bounds may > > have the same origin and height, but the anchor bounds within that widget > still > > may have changed (the anchor view may have moved within the bounds of the > > widget, due to layout changes). I think we could probably just close if the > > widget origin changes, or the anchor bounds have changed; that should solve > your > > bug. LMK if you need help. > > You're right that anchor position may change even if only the height changes, > but for the same reason, we need to close when the width changes because the > location of star icon does change when width changes. > > I actually wonder why bookmark bubble isn't set to move with anchor. Do you know > why? I don't see a reason offhand why the bookmark bubble doesn't move with its anchor (just tried this and it doesn't seem too broken). If that fixes your issue and doesn't cause other issues, let's go with that.
On 2014/02/13 01:38:07, msw wrote: > https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... > File ui/views/bubble/bubble_delegate.cc (right): > > https://codereview.chromium.org/152843004/diff/1/ui/views/bubble/bubble_deleg... > ui/views/bubble/bubble_delegate.cc:182: // Do not close if the widget gets > shrinked vertically without move. > On 2014/02/13 01:28:29, oshima wrote: > > On 2014/02/13 00:32:48, msw wrote: > > > This seems odd, and I don't think this is the right fix. The widget bounds > may > > > have the same origin and height, but the anchor bounds within that widget > > still > > > may have changed (the anchor view may have moved within the bounds of the > > > widget, due to layout changes). I think we could probably just close if the > > > widget origin changes, or the anchor bounds have changed; that should solve > > your > > > bug. LMK if you need help. > > > > You're right that anchor position may change even if only the height changes, > > but for the same reason, we need to close when the width changes because the > > location of star icon does change when width changes. > > > > I actually wonder why bookmark bubble isn't set to move with anchor. Do you > know > > why? > > I don't see a reason offhand why the bookmark bubble doesn't move with its > anchor (just tried this and it doesn't seem too broken). If that fixes your > issue and doesn't cause other issues, let's go with that. Ok done.
LGTM
The CQ bit was checked by oshima@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/oshima@chromium.org/152843004/60001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on win for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
The CQ bit was checked by oshima@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/oshima@chromium.org/152843004/60001
Message was sent while issue was closed.
Change committed as 251019 |
