|
|
Description[Material][Mac] Fix for Bookmark Subfolders Hover State
Ensures that the bookmark item's highlight gets drawn when
showsBorderOnlyWhileMouseInside is set to false
BUG=613261
Committed: https://crrev.com/a1272f22cff24d0ffe945665dbc4338d39107466
Cr-Commit-Position: refs/heads/master@{#404767}
Patch Set 1 #
Total comments: 2
Patch Set 2 : comment nit #Messages
Total messages: 22 (7 generated)
Description was changed from ========== [Material][Mac] Fix for Bookmark Subfolders Hover State BUG=613261 ========== to ========== [Material][Mac] Fix for Bookmark Subfolders Hover State Ensures that the bookmark item's highlight gets drawn when showsBorderOnlyWhileMouseInside is set to false BUG=613261 ==========
spqchan@chromium.org changed reviewers: + shrike@chromium.org
PTAL
lgtm When you mouse over a subfolder, its list of items appears to the right. It's a little unfortunate now that when you move the mouse away from a subfolder its row stays highlighted until its list of items gets hidden. Previously the row would get unhighlighted immediately even though the item row remained visible.
https://codereview.chromium.org/2134823002/diff/1/chrome/browser/ui/cocoa/gra... File chrome/browser/ui/cocoa/gradient_button_cell.mm (right): https://codereview.chromium.org/2134823002/diff/1/chrome/browser/ui/cocoa/gra... chrome/browser/ui/cocoa/gradient_button_cell.mm:597: // actually a highlight, which should be drawn if nit: I think you mean it to read "is actually a highlight..."
In regards to the row staying highlighted until its list of items get hidden, isn't that the expected behavior? https://codereview.chromium.org/2134823002/diff/1/chrome/browser/ui/cocoa/gra... File chrome/browser/ui/cocoa/gradient_button_cell.mm (right): https://codereview.chromium.org/2134823002/diff/1/chrome/browser/ui/cocoa/gra... chrome/browser/ui/cocoa/gradient_button_cell.mm:597: // actually a highlight, which should be drawn if On 2016/07/08 22:19:19, shrike wrote: > nit: I think you mean it to read "is actually a highlight..." Done.
On 2016/07/08 23:45:04, spqchan wrote: > In regards to the row staying highlighted until its list of items get hidden, > isn't that the expected behavior? Yes - sorry, what I forgot to add to my comment is that if you mouse over a subfolder so that its item appears, if you then move the mouse over another item you now have two items (the subfolder and the new item) highlighted until the bookmark code decides to hide the subfolder's item list. Previously the list would remain visible for the same amount of time, but there was only ever one row highlighted when you moved the mouse away from the subfolder.
On 2016/07/08 23:55:38, shrike wrote: > On 2016/07/08 23:45:04, spqchan wrote: > > In regards to the row staying highlighted until its list of items get hidden, > > isn't that the expected behavior? > > Yes - sorry, what I forgot to add to my comment is that if you mouse over a > subfolder so that its item appears, if you then move the mouse over another item > you now have two items (the subfolder and the new item) highlighted until the > bookmark code decides to hide the subfolder's item list. Previously the list > would remain visible for the same amount of time, but there was only ever one > row highlighted when you moved the mouse away from the subfolder. Ah, I see what you mean. Yeah, it doesn't look as responsive because of it. Is this material exclusive? I'm able to replicate this on Non-Material too
On 2016/07/11 18:07:20, spqchan wrote: > Ah, I see what you mean. Yeah, it doesn't look as responsive because of it. > Is this material exclusive? I'm able to replicate this on Non-Material too No. I don't think it's exclusive - I think it's the way it's always worked. I tested on Windows and it behaves the same way. However I don't think this really a good user experience. Would you please file a bug on this? At some point I will use the bug to check in with UX to see if they are OK with us making the menu handling more smooth.
On 2016/07/11 21:25:57, shrike wrote: > On 2016/07/11 18:07:20, spqchan wrote: > > Ah, I see what you mean. Yeah, it doesn't look as responsive because of it. > > Is this material exclusive? I'm able to replicate this on Non-Material too > > No. I don't think it's exclusive - I think it's the way it's always worked. > > I tested on Windows and it behaves the same way. However I don't think this > really a good user experience. Would you please file a bug on this? At some > point I will use the bug to check in with UX to see if they are OK with us > making the menu handling more smooth. Sure thing. I created crbug.com/627280
spqchan@chromium.org changed reviewers: + avi@chromium.org
+avi for OWNERS
lgtm
The CQ bit was checked by spqchan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from shrike@chromium.org Link to the patchset: https://codereview.chromium.org/2134823002/#ps20001 (title: "comment nit")
thanks!
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.
Description was changed from ========== [Material][Mac] Fix for Bookmark Subfolders Hover State Ensures that the bookmark item's highlight gets drawn when showsBorderOnlyWhileMouseInside is set to false BUG=613261 ========== to ========== [Material][Mac] Fix for Bookmark Subfolders Hover State Ensures that the bookmark item's highlight gets drawn when showsBorderOnlyWhileMouseInside is set to false BUG=613261 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== [Material][Mac] Fix for Bookmark Subfolders Hover State Ensures that the bookmark item's highlight gets drawn when showsBorderOnlyWhileMouseInside is set to false BUG=613261 ========== to ========== [Material][Mac] Fix for Bookmark Subfolders Hover State Ensures that the bookmark item's highlight gets drawn when showsBorderOnlyWhileMouseInside is set to false BUG=613261 Committed: https://crrev.com/a1272f22cff24d0ffe945665dbc4338d39107466 Cr-Commit-Position: refs/heads/master@{#404767} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a1272f22cff24d0ffe945665dbc4338d39107466 Cr-Commit-Position: refs/heads/master@{#404767} |