Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(193)

Issue 3228003: Sidebar view, implementation for Mac. (Closed)

Created:
10 years, 3 months ago by alekseys
Modified:
9 years, 7 months ago
Reviewers:
Nico
CC:
chromium-reviews, John Grabowski, Paweł Hajdan Jr., pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Sidebar view, Mac implementation (common sidebar code and Windows version is already submitted). TabContents.xib changes: it was converted to the latest IB version and vertical NSSplitView was added to it as a child for existing horizontal NSSplitView hosting devTools; new one hosts page content and sidebar content. BrowserWindowCocoa instance listen to SIDEBAR_CHANGED notification and updates sidebar content view according to the sidebar state linked to the current tab (adds or removes the corresponding view). Sidebar API design doc: http://www.chromium.org/developers/design-documents/extensions/sidebar-extension-api. BUG=51084 TEST=Run browser_tests and interactive_ui_tests. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=57898

Patch Set 1 #

Total comments: 10

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+936 lines, -50 lines) Patch
M chrome/app/nibs/TabContents.xib View 13 chunks +764 lines, -34 lines 0 comments Download
M chrome/browser/cocoa/browser_window_cocoa.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/cocoa/browser_window_cocoa.mm View 1 4 chunks +14 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/browser_window_controller.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/browser_window_controller.mm View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/tab_contents_controller.h View 2 chunks +13 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/tab_contents_controller.mm View 1 2 7 chunks +78 lines, -10 lines 0 comments Download
M chrome/browser/cocoa/tab_strip_controller.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/cocoa/tab_strip_controller.mm View 1 2 3 6 chunks +42 lines, -3 lines 0 comments Download
M chrome/browser/cocoa/view_id_util_browsertest.mm View 1 2 3 6 chunks +15 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
alekseys
10 years, 3 months ago (2010-08-27 22:02:45 UTC) #1
Nico
BUG= goes below issue description Please describe your xib changes in the CL description, reading ...
10 years, 3 months ago (2010-08-27 22:06:08 UTC) #2
Nico
Also, the CL description is a bit short – give a high-level overview of the ...
10 years, 3 months ago (2010-08-27 22:06:42 UTC) #3
Nico
http://codereview.chromium.org/3228003/diff/1/4 File chrome/browser/cocoa/browser_window_cocoa.mm (right): http://codereview.chromium.org/3228003/diff/1/4#newcode599 chrome/browser/cocoa/browser_window_cocoa.mm:599: [controller_ updateSidebarForContents: tab_contents]; no space after : http://codereview.chromium.org/3228003/diff/1/8 File ...
10 years, 3 months ago (2010-08-27 22:11:20 UTC) #4
alekseys
http://codereview.chromium.org/3228003/diff/1/4 File chrome/browser/cocoa/browser_window_cocoa.mm (right): http://codereview.chromium.org/3228003/diff/1/4#newcode599 chrome/browser/cocoa/browser_window_cocoa.mm:599: [controller_ updateSidebarForContents: tab_contents]; On 2010/08/27 22:11:20, Nico wrote: > ...
10 years, 3 months ago (2010-08-27 22:39:25 UTC) #5
Nico
Please add a comment like // This function is very similar to $OTHER_FN_NAME. I (alekseys) ...
10 years, 3 months ago (2010-08-27 22:45:22 UTC) #6
alekseys
10 years, 3 months ago (2010-08-28 16:05:23 UTC) #7
Done.

On 2010/08/27 22:45:22, Nico wrote:
> Please add a comment like
> 
> // This function is very similar to $OTHER_FN_NAME. I (alekseys) intend to
move
> // this functionality to browser window soon, so I didn't refactor this just
> yet.
> 
> Otherwise, lg.

Powered by Google App Engine
This is Rietveld 408576698