Issue 12208060: Alternate NTP: Don't overlap dev tools with bookmark bar

Issue 12208060: Alternate NTP: Don't overlap dev tools with bookmark bar (Closed)

Created:
7 years, 10 months ago by sail

Modified:
7 years, 10 months ago

Reviewers:
yurys, pfeldman, dhollowa

CC:
chromium-reviews, sail+watch_chromium.org, vsevik, yurys, pfeldman

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Alternate NTP: Don't overlap dev tools with bookmark bar When docked vertically the dev tools view overlaps the bookmark bar. This was a bug I introduced in r178367 when I changed the bookmark bar to overlap web contents. To fix this I've made the following changes: - implement constrainSplitPosition: to prevent the splitter from being dragged past the bookmark bar - move the dev tools view into a container view. The container view resizes the dev tools view to be below the bookmark bar BUG=173205 TEST=Open dev tools and verified that it never went under the bookmark bar. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181301

Patch Set 1 #

Total comments: 2

Patch Set 2 : address review comments #

Patch Set 3 : fix test #

Patch Set 4 : fix linux #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+141 lines, -1 line)			Patch
M	chrome/browser/devtools/devtools_window.h	View	1 2 3	2 chunks	+2 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/browser_window_controller.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/browser_window_controller.mm	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/dev_tools_controller.mm	View		5 chunks	+45 lines, -1 line	0 comments	Download
A	chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm	View	1 2	1 chunk	+87 lines, -0 lines	0 comments	Download
M	chrome/chrome_tests.gypi	View		1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

dhollowa

LGTM. Thanks for the tests. https://codereview.chromium.org/12208060/diff/1/chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm File chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm (right): https://codereview.chromium.org/12208060/diff/1/chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm#newcode1 chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm:1: // Copyright (c) 2013 ...

7 years, 10 months ago (2013-02-07 02:43:40 UTC) #2

sail

pfeldman or yurys: could I get a devtools OWNERS review? https://codereview.chromium.org/12208060/diff/1/chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm File chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm (right): https://codereview.chromium.org/12208060/diff/1/chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm#newcode1 ...

7 years, 10 months ago (2013-02-07 02:50:49 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sail@chromium.org/12208060/8001

7 years, 10 months ago (2013-02-07 11:33:09 UTC) #5

commit-bot: I haz the power

Sorry for I got bad news for ya. Compile failed with a clobber build on ...

7 years, 10 months ago (2013-02-07 12:01:30 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sail@chromium.org/12208060/4004

7 years, 10 months ago (2013-02-07 12:07:52 UTC) #7

Message was sent while issue was closed.

Change committed as 181301

Expand Messages | Collapse Messages