Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1849563002:
Re-order the BookmarkBarView to be behind the ToolbarView (Closed)
DescriptionThe Material Design ink drops on the ToolbarView are supposed to expand beyond the bounds of the ToolbarView and should be visible above the BookmarkBarView.
BUG=597757
TEST=Manual
Committed: https://crrev.com/52ef1ab0da46ea871271a0a2a6fa0ced541b0381
Cr-Commit-Position: refs/heads/master@{#384808}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Polished fix #
Total comments: 7
Patch Set 3 : Added comments to BrowserView::SetBookmarkBarParent(). #
Total comments: 12
Patch Set 4 : Updated comments as per pkasting@'s suggestions. #Patch Set 5 : Added TODO. #Messages
Total messages: 28 (6 generated)
https://codereview.chromium.org/1849563002/diff/1/chrome/browser/ui/views/fra... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/1/chrome/browser/ui/views/fra... chrome/browser/ui/views/frame/browser_view.cc:2230: // new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); This is the actual desired change here.
Description was changed from ========== Prototype fix. BUG=597757 ========== to ========== The Material Design ink drops on the ToolbarView are supposed to expand beyond the bounds of the ToolbarView and should be visible above the BookmarkBarView. BUG=597757 TEST=Manual ==========
bruthig@chromium.org changed reviewers: + pkasting@chromium.org
pkasting@, Can you PTAL?
https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); This may be better for sky than me; I'm not familiar with this function. It seems strange that the comment talks about Z-order but the functionality actually is about adding before or after other children, which impacts things like taborder and should normally match the visible hierarchy onscreen. I'm not familiar with child order affecting Z order. I would instead have expected changes to layout or painting to accomplish that.
bruthig@chromium.org changed reviewers: + sky@chromium.org
sky@, can you PTAL? https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); On 2016/03/30 21:15:28, Peter Kasting wrote: > This may be better for sky than me; I'm not familiar with this function. > > It seems strange that the comment talks about Z-order but the functionality > actually is about adding before or after other children, which impacts things > like taborder and should normally match the visible hierarchy onscreen. I'm not > familiar with child order affecting Z order. I would instead have expected > changes to layout or painting to accomplish that. As far as I can tell a user cannot tab between the ToolbarView and the BookmarkBarView, only between child Views within them. I will add sky@.
https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); On 2016/03/30 21:27:00, bruthig wrote: > On 2016/03/30 21:15:28, Peter Kasting wrote: > > This may be better for sky than me; I'm not familiar with this function. > > > > It seems strange that the comment talks about Z-order but the functionality > > actually is about adding before or after other children, which impacts things > > like taborder and should normally match the visible hierarchy onscreen. I'm > not > > familiar with child order affecting Z order. I would instead have expected > > changes to layout or painting to accomplish that. > > As far as I can tell a user cannot tab between the ToolbarView and the > BookmarkBarView, only between child Views within them. I will add sky@. Is the taborder between the toolbar children and the bookmark bar children unaffected by this change? Even if yes, I'm still a bit mystified how this actually achieves a Z-order adjustment.
On 2016/03/30 21:28:42, Peter Kasting wrote: > https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... > File chrome/browser/ui/views/frame/browser_view.cc (right): > > https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... > chrome/browser/ui/views/frame/browser_view.cc:2228: > new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); > On 2016/03/30 21:27:00, bruthig wrote: > > On 2016/03/30 21:15:28, Peter Kasting wrote: > > > This may be better for sky than me; I'm not familiar with this function. > > > > > > It seems strange that the comment talks about Z-order but the functionality > > > actually is about adding before or after other children, which impacts > things > > > like taborder and should normally match the visible hierarchy onscreen. I'm > > not > > > familiar with child order affecting Z order. I would instead have expected > > > changes to layout or painting to accomplish that. > > > > As far as I can tell a user cannot tab between the ToolbarView and the > > BookmarkBarView, only between child Views within them. I will add sky@. > > Is the taborder between the toolbar children and the bookmark bar children > unaffected by this change? > > Even if yes, I'm still a bit mystified how this actually achieves a Z-order > adjustment. because children are painted (z-ordered) based on their index, by default. See https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/view.h&l=...
https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); On 2016/03/30 21:27:00, bruthig wrote: > On 2016/03/30 21:15:28, Peter Kasting wrote: > > This may be better for sky than me; I'm not familiar with this function. > > > > It seems strange that the comment talks about Z-order but the functionality > > actually is about adding before or after other children, which impacts things > > like taborder and should normally match the visible hierarchy onscreen. I'm > not > > familiar with child order affecting Z order. I would instead have expected > > changes to layout or painting to accomplish that. > > As far as I can tell a user cannot tab between the ToolbarView and the > BookmarkBarView, only between child Views within them. I will add sky@. Tab does wrap within the bookmarkbar. You can cycle through panes with F6. First hit shift-alt-t, and then use f6 to cycle.
https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); On 2016/03/31 01:07:16, sky wrote: > On 2016/03/30 21:27:00, bruthig wrote: > > On 2016/03/30 21:15:28, Peter Kasting wrote: > > > This may be better for sky than me; I'm not familiar with this function. > > > > > > It seems strange that the comment talks about Z-order but the functionality > > > actually is about adding before or after other children, which impacts > things > > > like taborder and should normally match the visible hierarchy onscreen. I'm > > not > > > familiar with child order affecting Z order. I would instead have expected > > > changes to layout or painting to accomplish that. > > > > As far as I can tell a user cannot tab between the ToolbarView and the > > BookmarkBarView, only between child Views within them. I will add sky@. > > Tab does wrap within the bookmarkbar. You can cycle through panes with F6. First > hit shift-alt-t, and then use f6 to cycle. I'm slightly confused here, are there outstanding concerns? FTR The F6 pane focus order is defined by the list returned from BrowserView::GetAccessiblePanes() which is NOT dependent on the order of BrowserView's children Views.
Did you test that before/during/after animation the toolbar divider is not drawn incorrectly atop the bookmark bar? https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); On 2016/03/31 21:12:29, bruthig wrote: > On 2016/03/31 01:07:16, sky wrote: > > On 2016/03/30 21:27:00, bruthig wrote: > > > On 2016/03/30 21:15:28, Peter Kasting wrote: > > > > This may be better for sky than me; I'm not familiar with this function. > > > > > > > > It seems strange that the comment talks about Z-order but the > functionality > > > > actually is about adding before or after other children, which impacts > > things > > > > like taborder and should normally match the visible hierarchy onscreen. > I'm > > > not > > > > familiar with child order affecting Z order. I would instead have > expected > > > > changes to layout or painting to accomplish that. > > > > > > As far as I can tell a user cannot tab between the ToolbarView and the > > > BookmarkBarView, only between child Views within them. I will add sky@. > > > > Tab does wrap within the bookmarkbar. You can cycle through panes with F6. > First > > hit shift-alt-t, and then use f6 to cycle. > > I'm slightly confused here, are there outstanding concerns? > > FTR The F6 pane focus order is defined by the list returned from > BrowserView::GetAccessiblePanes() which is NOT dependent on the order of > BrowserView's children Views. As long as it's not possible to actually tab directly between toolbar and bookmark bar view elements (i.e. one has to hit a different key like F6), I'm not concerned. I'm still not familiar enough with the rest of this function to necessarily sign off on this change, though. What is the top container, and why could we sometimes be added as a sibling to it and sometimes as a child? I'd like to have a more coherent comment on all this stuff elaborating the different cases where there's a bookmark bar and what the child order needs to be in each one, mentioning how the child order becomes the paint order and thus the Z order.
On Thu, Mar 31, 2016 at 2:12 PM, <bruthig@chromium.org> wrote: > > https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... > File chrome/browser/ui/views/frame/browser_view.cc (right): > > https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... > chrome/browser/ui/views/frame/browser_view.cc:2228: > new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); > On 2016/03/31 01:07:16, sky wrote: >> On 2016/03/30 21:27:00, bruthig wrote: >> > On 2016/03/30 21:15:28, Peter Kasting wrote: >> > > This may be better for sky than me; I'm not familiar with this > function. >> > > >> > > It seems strange that the comment talks about Z-order but the > functionality >> > > actually is about adding before or after other children, which > impacts >> things >> > > like taborder and should normally match the visible hierarchy > onscreen. I'm >> > not >> > > familiar with child order affecting Z order. I would instead have > expected >> > > changes to layout or painting to accomplish that. >> > >> > As far as I can tell a user cannot tab between the ToolbarView and > the >> > BookmarkBarView, only between child Views within them. I will add > sky@. >> >> Tab does wrap within the bookmarkbar. You can cycle through panes with > F6. First >> hit shift-alt-t, and then use f6 to cycle. > > I'm slightly confused here, are there outstanding concerns? You added me to this review part way through. It isn't entirely clear what you need to comment on? > FTR The F6 pane focus order is defined by the list returned from > BrowserView::GetAccessiblePanes() which is NOT dependent on the order of > BrowserView's children Views. > > https://codereview.chromium.org/1849563002/ -- 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.
On 2016/03/31 21:31:17, sky wrote: > You added me to this review part way through. It isn't entirely clear > what you need to comment on? I was hoping you knew anything about the function being modified, which would be more than I know. If so, either responding to my latest questions, or in general thoughts about whether this is a sane way to fix, would be useful.
https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: new_parent->AddChildViewAt(bookmark_bar_view_.get(), 0); On 2016/03/31 21:20:41, Peter Kasting wrote: > On 2016/03/31 21:12:29, bruthig wrote: > > On 2016/03/31 01:07:16, sky wrote: > > > On 2016/03/30 21:27:00, bruthig wrote: > > > > On 2016/03/30 21:15:28, Peter Kasting wrote: > > > > > This may be better for sky than me; I'm not familiar with this function. > > > > > > > > > > It seems strange that the comment talks about Z-order but the > > functionality > > > > > actually is about adding before or after other children, which impacts > > > things > > > > > like taborder and should normally match the visible hierarchy onscreen. > > I'm > > > > not > > > > > familiar with child order affecting Z order. I would instead have > > expected > > > > > changes to layout or painting to accomplish that. > > > > > > > > As far as I can tell a user cannot tab between the ToolbarView and the > > > > BookmarkBarView, only between child Views within them. I will add sky@. > > > > > > Tab does wrap within the bookmarkbar. You can cycle through panes with F6. > > First > > > hit shift-alt-t, and then use f6 to cycle. > > > > I'm slightly confused here, are there outstanding concerns? > > > > FTR The F6 pane focus order is defined by the list returned from > > BrowserView::GetAccessiblePanes() which is NOT dependent on the order of > > BrowserView's children Views. > > As long as it's not possible to actually tab directly between toolbar and > bookmark bar view elements (i.e. one has to hit a different key like F6), I'm > not concerned. That is correct. Tab's will stay within the same "Pane" and only F6 will move between the Toolbar, BookmarkBarView, InfoBarContainerView, DownloadShelfView, ContentsWebView and dev tool view "Panes". > > I'm still not familiar enough with the rest of this function to necessarily sign > off on this change, though. What is the top container, and why could we > sometimes be added as a sibling to it and sometimes as a child? I'd like to > have a more coherent comment on all this stuff elaborating the different cases > where there's a bookmark bar and what the child order needs to be in each one, > mentioning how the child order becomes the paint order and thus the Z order. I've added some comments as requested however I feel like these could quickly drift from correctness and would be more than happy to purge some. Please let me know what you think is necessary. sky@, your input here is welcome too. As for why the |bookmark_bar_| is sometimes added as a sibling and sometimes as a child, I have no idea. sky@, do you have any idea?
Cool, these comments helped me understand more. I wrote some suggested improvements. I also proposed some functionality changes not necessary to fix this bug. I suggest making these in a separate CL to land after this, so we don't have to merge it back. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2220: // specific index because that is what determines z-order. Nit: How about this. TODO is optional, I don't know if you actually want to make that change someday or not. Because children are drawn in order, the child order also affects z-order: earlier children will appear "below" later ones. This is important for ink drops, which are drawn with the z-order of the view that parents them. Ink drops in the toolbar can spread beyond the toolbar bounds, so if the bookmark bar is attached, we want it to be below the toolbar so the ink drops draw atop it. This doesn't cause a problem for interactions with the bookmark bar, since it does not host any ink drops that spread beyond its bounds. If it did, we would need to change how ink drops are drawn so they use a transparent layer above all children here. TODO(bruthig): Use such a layer in all cases, eliminating the need for any complexity here. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: // |top_container_|. Nit: How about: |top_container_| contains the toolbar, so putting the bookmark bar ahead if it will ensure it's drawn before the toolbar. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2233: // is not visible so the z-order does not matter. Seems like if this is true we should remove this case and if necessary just DCHECK_GE(top_container_index, 0);. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2237: // BookmarkBarView is detached. Do you mean "attached"? https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2240: // |bookmark_bar_view_| first so it is behind the |toolbar_|. Nit: How about: The toolbar is a child of |top_container_|, so making the bookmark bar the first child ensures it's drawn before the toolbar. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2243: // This is not an expected case so we will default to no special stacking. Seems like if this is true we should remove this conditional arm and DCHECK(!new_parent); in the else below.
On 2016/03/31 21:32:51, Peter Kasting wrote: > On 2016/03/31 21:31:17, sky wrote: > > You added me to this review part way through. It isn't entirely clear > > what you need to comment on? > > I was hoping you knew anything about the function being modified, which would be > more than I know. If so, either responding to my latest questions, or in > general thoughts about whether this is a sane way to fix, would be useful. Have we considered hosting the ink drops at a higher level so that the z-order here doesn't matter?
On 2016/04/01 02:10:05, sky wrote: > On 2016/03/31 21:32:51, Peter Kasting wrote: > > On 2016/03/31 21:31:17, sky wrote: > > > You added me to this review part way through. It isn't entirely clear > > > what you need to comment on? > > > > I was hoping you knew anything about the function being modified, which would > be > > more than I know. If so, either responding to my latest questions, or in > > general thoughts about whether this is a sane way to fix, would be useful. > > Have we considered hosting the ink drops at a higher level so that the z-order > here doesn't matter? Right, see the TODO I suggested, which I think is basically that.
On 2016/04/01 04:55:38, Peter Kasting wrote: > On 2016/04/01 02:10:05, sky wrote: > > On 2016/03/31 21:32:51, Peter Kasting wrote: > > > On 2016/03/31 21:31:17, sky wrote: > > > > You added me to this review part way through. It isn't entirely clear > > > > what you need to comment on? > > > > > > I was hoping you knew anything about the function being modified, which > would > > be > > > more than I know. If so, either responding to my latest questions, or in > > > general thoughts about whether this is a sane way to fix, would be useful. > > > > Have we considered hosting the ink drops at a higher level so that the z-order > > here doesn't matter? > > Right, see the TODO I suggested, which I think is basically that. Any reason not to investigate that now? Seems like doing that would result in less z-order changes like this in the future.
On 2016/04/01 14:56:13, sky wrote: > On 2016/04/01 04:55:38, Peter Kasting wrote: > > On 2016/04/01 02:10:05, sky wrote: > > > On 2016/03/31 21:32:51, Peter Kasting wrote: > > > > On 2016/03/31 21:31:17, sky wrote: > > > > > You added me to this review part way through. It isn't entirely clear > > > > > what you need to comment on? > > > > > > > > I was hoping you knew anything about the function being modified, which > > would > > > be > > > > more than I know. If so, either responding to my latest questions, or in > > > > general thoughts about whether this is a sane way to fix, would be useful. > > > > > > Have we considered hosting the ink drops at a higher level so that the > z-order > > > here doesn't matter? > > > > Right, see the TODO I suggested, which I think is basically that. > > Any reason not to investigate that now? Seems like doing that would result in > less z-order changes like this in the future. I talked this through with Ben a bit and it seems to me it would be hard to restructure things to provide a general solution. I see a general solution as something like the following: RootView has a view that is responsible for drawing the background. This view draws to a layer. RootView has a background effect view. The contentview of RootView also draws to a layer. RootView has a top view that exists solely for effects that are to appear over everything. If we had this effects that need to be above the background but beneath the would be added to the background effect view. Effects that need to go above everything are added to the top view. Problem with this approach is that we don't necessarily have a clear owner of the 'background'. Both the toolbarview and bookmarkbar contributed to the background when using the glass frame. For the non-glass frame the background is drawn by the nonclientframeview. I think we restructure the views drawing background to fit this model, but then we would end up with a lot more layers, which I suspect negatively impacts memory.
> > > Have we considered hosting the ink drops at a higher level so that the > z-order > > > here doesn't matter? > > > > Right, see the TODO I suggested, which I think is basically that. > > Any reason not to investigate that now? Seems like doing that would result in > less z-order changes like this in the future. FTR sky@ and I chatted offline and decided to move forward with this as is for now and I've left a TODO to consider a more general mechanism. It should be noted that there is a somewhat competing requirement that the ripples are drawn behind the icons, (we also wanted it behind text but couldn't due to limitations of drawing text on a transparent background). pkasting@, PTAL. sky@, take a look if you wish but not required by me. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2220: // specific index because that is what determines z-order. On 2016/03/31 22:22:57, Peter Kasting wrote: > Nit: How about this. TODO is optional, I don't know if you actually want to > make that change someday or not. > > Because children are drawn in order, the child order also affects z-order: > earlier children will appear "below" later ones. This is important for ink > drops, which are drawn with the z-order of the view that parents them. Ink > drops in the toolbar can spread beyond the toolbar bounds, so if the bookmark > bar is attached, we want it to be below the toolbar so the ink drops draw atop > it. This doesn't cause a problem for interactions with the bookmark bar, since > it does not host any ink drops that spread beyond its bounds. If it did, we > would need to change how ink drops are drawn so they use a transparent layer > above all children here. TODO(bruthig): Use such a layer in all cases, > eliminating the need for any complexity here. Modified slightly but done. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2228: // |top_container_|. On 2016/03/31 22:22:56, Peter Kasting wrote: > Nit: How about: > > |top_container_| contains the toolbar, so putting the bookmark bar ahead if it > will ensure it's drawn before the toolbar. Done. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2233: // is not visible so the z-order does not matter. On 2016/03/31 22:22:57, Peter Kasting wrote: > Seems like if this is true we should remove this case and if necessary just > DCHECK_GE(top_container_index, 0);. Done. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2237: // BookmarkBarView is detached. On 2016/03/31 22:22:57, Peter Kasting wrote: > Do you mean "attached"? Done. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2240: // |bookmark_bar_view_| first so it is behind the |toolbar_|. On 2016/03/31 22:22:57, Peter Kasting wrote: > Nit: How about: > > The toolbar is a child of |top_container_|, so making the bookmark bar the first > child ensures it's drawn before the toolbar. Done. https://codereview.chromium.org/1849563002/diff/40001/chrome/browser/ui/views... chrome/browser/ui/views/frame/browser_view.cc:2243: // This is not an expected case so we will default to no special stacking. On 2016/03/31 22:22:57, Peter Kasting wrote: > Seems like if this is true we should remove this conditional arm and > DCHECK(!new_parent); in the else below. Done.
LGTM I can't think of any way to write a test to check that ink drops spread out atop the bookmark bar, but if you come up with one, feel free to add it.
The CQ bit was checked by bruthig@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1849563002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1849563002/80001
Message was sent while issue was closed.
Description was changed from ========== The Material Design ink drops on the ToolbarView are supposed to expand beyond the bounds of the ToolbarView and should be visible above the BookmarkBarView. BUG=597757 TEST=Manual ========== to ========== The Material Design ink drops on the ToolbarView are supposed to expand beyond the bounds of the ToolbarView and should be visible above the BookmarkBarView. BUG=597757 TEST=Manual ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== The Material Design ink drops on the ToolbarView are supposed to expand beyond the bounds of the ToolbarView and should be visible above the BookmarkBarView. BUG=597757 TEST=Manual ========== to ========== The Material Design ink drops on the ToolbarView are supposed to expand beyond the bounds of the ToolbarView and should be visible above the BookmarkBarView. BUG=597757 TEST=Manual Committed: https://crrev.com/52ef1ab0da46ea871271a0a2a6fa0ced541b0381 Cr-Commit-Position: refs/heads/master@{#384808} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/52ef1ab0da46ea871271a0a2a6fa0ced541b0381 Cr-Commit-Position: refs/heads/master@{#384808} |
