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

Issue 2243002: Resize the "For quick access..." and "Import bookmarks now..." components sho... (Closed)

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

Description

Resize the "For quick access..." and "Import bookmarks now..." components shown in the bookmark bar when there are no bookmarks in the bar, when the window resizes. Show ellipses when each gets trimmed and hide them when they get too small. BookmarkBar.xib changes: Connect the "Import bookmarks now..." button up to the importBookmarksButton_ outlet in the BookmarkBarView. BUG=32557 TEST=1) Start up a browser with no bookmarks on the bookmarks bar. 2) Verify that the "For quick access..." and "Import bookmarks now..." test fully shows. 3) Start shrinking the width of the browser window. 4) Verify that the "Import bookmarks now..." text is shrunk/clipped and finally disappears as the window gets narrower. 5) Continue shrinking the width and verify that the "For quick access..." text is shrunk/clipped with ellipses as the window gets narrower. 6) Grow the window and verify that the "For quick access..." reappears and then the "Import book now..." does. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=48316

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+114 lines, -12 lines) Patch
M chrome/app/nibs/BookmarkBar.xib View 8 chunks +33 lines, -5 lines 0 comments Download
M chrome/browser/cocoa/bookmark_bar_controller.h View 2 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/cocoa/bookmark_bar_controller.mm View 1 3 chunks +42 lines, -0 lines 6 comments Download
M chrome/browser/cocoa/bookmark_bar_controller_unittest.mm View 1 chunk +19 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/bookmark_bar_view.h View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/bookmark_bar_view.mm View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/hyperlink_button_cell.mm View 1 chunk +9 lines, -6 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
mrossetti
The change to the hyperlink button cell drawing renders the text exactly as before, only ...
10 years, 7 months ago (2010-05-26 15:19:30 UTC) #1
John Grabowski
Be sure to document explicitly your xib changes. LGTM http://codereview.chromium.org/2243002/diff/4001/5003 File chrome/browser/cocoa/bookmark_bar_controller.mm (right): http://codereview.chromium.org/2243002/diff/4001/5003#newcode1223 chrome/browser/cocoa/bookmark_bar_controller.mm:1223: ...
10 years, 7 months ago (2010-05-26 17:39:34 UTC) #2
mrossetti
10 years, 7 months ago (2010-05-26 20:10:31 UTC) #3
XIB changes also documented.

http://codereview.chromium.org/2243002/diff/4001/5003
File chrome/browser/cocoa/bookmark_bar_controller.mm (right):

http://codereview.chromium.org/2243002/diff/4001/5003#newcode1223
chrome/browser/cocoa/bookmark_bar_controller.mm:1223: +
(BOOL)shrinkOrHideView:(NSView*)view forMaxX:(CGFloat)maxViewX {
On 2010/05/26 17:39:34, John Grabowski wrote:
> name issue: forMaxX --> forMaxWidth
> 

I updated the comment instead since it really is the x extent and not the width.

http://codereview.chromium.org/2243002/diff/4001/5003#newcode1223
chrome/browser/cocoa/bookmark_bar_controller.mm:1223: +
(BOOL)shrinkOrHideView:(NSView*)view forMaxX:(CGFloat)maxViewX {
On 2010/05/26 17:39:34, John Grabowski wrote:
> class method?  Really?  Who uses this but us?

Changed.

http://codereview.chromium.org/2243002/diff/4001/5003#newcode1288
chrome/browser/cocoa/bookmark_bar_controller.mm:1288: // While we're here,
adjust the horizontal width and the visibility
On 2010/05/26 17:39:34, John Grabowski wrote:
> split into distinct function.
> Only call it if ![buttons_ count].

Done.

Powered by Google App Engine
This is Rietveld 408576698