Remove content border images in opaque frame for refresh |
||||||||
Issue descriptionThe opaque frame expects there to be a stroke atop the toolbar. Without one in refresh, the strokes on the other three sides below that point look odd. Possibilities: (1) Draw a stroke atop the toolbar and around only the active tab (and maybe hovered and/or selected tabs, I dunno?). Would maybe want to share code with the solution for bug 848546 in this case. (2) Kick opaque frame fully into the "bug 848546" camp and draw strokes everywhere, assuming that's the solution we take on that bug. (3) Remove the strokes around the other three sides (but in this case those might start looking weird). (4) Fade out the stroke through the toolbar height portion, or some other similar transition. (5) Draw the stroke all the way to the top, or up to a (top border)-size height below the top that we draw the stroke across. I think probably (4) is both easiest and "good enough". Otherwise I might try to do (2). (3) and (5) are going to look bad and (1) might look nice but has design gotchas that aren't worth worrying about over (2), plus it's probably more complex to implement.
,
Jun 7 2018
,
Jun 26 2018
Note that this is pretty close to how the frame looked before -- only the tabstrip area changed. If you think the sides of the windows look wrong, could you share a screenshot of your pre-refresh state and note why the differences make refresh look like it has scrollbars?
,
Jun 26 2018
I think part of why it looks so bad in #3 may be because of the repainting issue with --disable-gpu on linux. Maybe the whole titlebar isn't meant to be white.. or maybe it is when there's one tab, and it looks better with > 1 tab, it's hard for me to tell.
,
Jun 26 2018
The titlebar is meant to be white when there is 1 tab. Your diagnosis reassures me that this is the right bug and comment 0 is the right description of the bug, but makes me slightly less confident that solution 4 would address this. I still think it's a P3 visual polish bug, though, rather than a P2 "we really shouldn't ship this to stable users". I might be wrong.
,
Jun 28 2018
,
Jul 7
In revisiting this in mocks, comment 0 solution (3) actually looks good. Just take off the 4 DIP-thick border images around the content on the left/right/bottom and paint the frame color there. I think we should fix before shipping. This looks really bad.
,
Jul 7
,
Jul 9
Load balancing
,
Jul 11
,
Jul 18
I'm gonna P1 this because it looks horrible and we should not ship without it addressed.
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900 commit 5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900 Author: Peter Boström <pbos@chromium.org> Date: Thu Jul 19 03:43:52 2018 Remove content border for opaque frame in Refresh Under Refresh there's no top border drawn by the tabstrip. As such the three other strokes that form the content border look cut off and out of place. This change removes them when Refresh is turned on. Bug: chromium:848549 Change-Id: If77903b5b275f6d0deee6d22d88a3725885137f2 Reviewed-on: https://chromium-review.googlesource.com/1132543 Commit-Queue: Peter Boström <pbos@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#576355} [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/browser_non_client_frame_view.h [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/browser_view.cc [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/glass_browser_frame_view.cc [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/glass_browser_frame_view.h [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/opaque_browser_frame_view.h [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h [modify] https://crrev.com/5e35d8a8cb7c2547cd2d13e1b5ab283a112cf900/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
,
Jul 19
Fixed? |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by pkasting@chromium.org
, Jun 1 2018