Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1937673002:
Close mash shelf tooltips on touches outside the shelf. (Closed)
DescriptionClose mash shelf tooltips on touches outside the shelf.
Make ShelfTooltipManager a views::PointerWatcher.
Close on mouse/touch press (including outside shelf root).
BUG=595853
TEST=Touches outside the shelf area close shelf tooltips.
R=jamescook@chromium.org,sky@chromium.org
Committed: https://crrev.com/b53df9e2ff6a08ab6203129e8be41583712099f9
Cr-Commit-Position: refs/heads/master@{#391647}
Patch Set 1 #Patch Set 2 : Sync and rebase; cleanup. #
Total comments: 4
Patch Set 3 : Address comments. #
Messages
Total messages: 23 (13 generated)
Description was changed from ========== Close mash shelf tooltips on touches outside the shelf. BUG= ========== to ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org ==========
msw@chromium.org changed reviewers: + jamescook@chromium.org
Patchset #2 (id:20001) has been deleted
Patchset #2 (id:40001) has been deleted
Hey James, please take a look; thanks!
The CQ bit was checked by msw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1937673002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1937673002/60001
LGTM with nits https://codereview.chromium.org/1937673002/diff/60001/ash/shelf/shelf_tooltip... File ash/shelf/shelf_tooltip_manager.cc (right): https://codereview.chromium.org/1937673002/diff/60001/ash/shelf/shelf_tooltip... ash/shelf/shelf_tooltip_manager.cc:178: // PointerWatcher supports tracking events outside the root window in mash. nit: I might say something explicitly that any click/touch, even one inside the tooltip, is supposed to close it. https://codereview.chromium.org/1937673002/diff/60001/ash/shelf/shelf_tooltip... ash/shelf/shelf_tooltip_manager.cc:189: if (event->type() == ui::ET_MOUSE_PRESSED || Eliminate this one, as it's confusing that there are two mouse pressed handlers. The PointerWatcher will give you press events that hit the tooltip.
Description was changed from ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org ========== to ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org,sky@chromium.org ==========
msw@chromium.org changed reviewers: + sky@chromium.org
Scott, please take a look for OWNERS; thanks! https://codereview.chromium.org/1937673002/diff/60001/ash/shelf/shelf_tooltip... File ash/shelf/shelf_tooltip_manager.cc (right): https://codereview.chromium.org/1937673002/diff/60001/ash/shelf/shelf_tooltip... ash/shelf/shelf_tooltip_manager.cc:178: // PointerWatcher supports tracking events outside the root window in mash. On 2016/05/04 19:17:49, James Cook wrote: > nit: I might say something explicitly that any click/touch, even one inside the > tooltip, is supposed to close it. Done. https://codereview.chromium.org/1937673002/diff/60001/ash/shelf/shelf_tooltip... ash/shelf/shelf_tooltip_manager.cc:189: if (event->type() == ui::ET_MOUSE_PRESSED || On 2016/05/04 19:17:49, James Cook wrote: > Eliminate this one, as it's confusing that there are two mouse pressed handlers. > The PointerWatcher will give you press events that hit the tooltip. Done. (Added early return for touch/mouse pressed)
The CQ bit was checked by msw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1937673002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1937673002/80001
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 msw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jamescook@chromium.org Link to the patchset: https://codereview.chromium.org/1937673002/#ps80001 (title: "Address comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1937673002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1937673002/80001
Message was sent while issue was closed.
Description was changed from ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org,sky@chromium.org ========== to ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org,sky@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org,sky@chromium.org ========== to ========== Close mash shelf tooltips on touches outside the shelf. Make ShelfTooltipManager a views::PointerWatcher. Close on mouse/touch press (including outside shelf root). BUG=595853 TEST=Touches outside the shelf area close shelf tooltips. R=jamescook@chromium.org,sky@chromium.org Committed: https://crrev.com/b53df9e2ff6a08ab6203129e8be41583712099f9 Cr-Commit-Position: refs/heads/master@{#391647} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/b53df9e2ff6a08ab6203129e8be41583712099f9 Cr-Commit-Position: refs/heads/master@{#391647} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
