[ntp4] Add a 'Manage Bookmarks' link at the top of the Bookmarks page. Also right-align links.

[ntp4] Add a 'Manage Bookmarks' link at the top of the Bookmarks page.  Also right-align links.

BUG=94785
TEST=Verify that a 'Manage Bookmarks' link appears at the top/right of bookmarks page in NTP4.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99700

[csilv, 2011-09-04 22:02:57]

+estade for review

FYI, this is the only CL I will have for review today (sunday).

[Evan Stade, 2011-09-05 01:59:20]

lgtm with comments addressed

http://codereview.chromium.org/7833021/diff/3001/chrome/app/generated_resourc...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/app/generated_resourc...
chrome/app/generated_resources.grd:8445: Show all...
no ellipses in either string (I asked UI leads specifically about this. The
ellipsis is just for actions that won't be completed when you click the thing.
The "Show all" action /is/ completed when you click "Show all". I guess it's a
bit hazier for "Manage bookmarks" but I think it's generally understood that a
link will take the user away from 'here' so the ellipsis is extraneous.)

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/bookmarks_page.css (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.css:109: #bookmarks-title-wrapper {
might as well fix crbug.com/94968 while we're here

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.css:165: text-align: right;
end not right

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.css:173: text-align: right;
also need to make this ok for RTL (I think you can just use text-align: end; and
margin: 0px -70px;)

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/bookmarks_page.js (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.js:236: // insert the top & bottom
links for the Bookmarks Manager page.
Insert

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/new_tab.html (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/new_tab.html:139: <a href="chrome://bookmarks"
i18n-content="bookmarksManagerLinkTitle"></a>
I think this should probably open the same URL as the bottom link. It's weird we
have two links that do the same thing but it would be weirder to have the top
link disappear, and probably it's not what the user wants if they're deep in
some folder and this link doesn't take them there.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/tile_page.js (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/tile_page.js:326: var TILE_GRID_EXTRA_PADDING =
40;
can you do this as a function on TilePage which children of TilePage can
override, like

ExtraBottomPadding: function() {
  return 40;
}

that way I think it's not such a hack.

[csilv, 2011-09-06 05:33:33]

http://codereview.chromium.org/7833021/diff/3001/chrome/app/generated_resourc...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/app/generated_resourc...
chrome/app/generated_resources.grd:8445: Show all...
On 2011/09/05 01:59:20, Evan Stade wrote:
> no ellipses in either string (I asked UI leads specifically about this. The
> ellipsis is just for actions that won't be completed when you click the thing.
> The "Show all" action /is/ completed when you click "Show all". I guess it's a
> bit hazier for "Manage bookmarks" but I think it's generally understood that a
> link will take the user away from 'here' so the ellipsis is extraneous.)

Done.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/bookmarks_page.css (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.css:109: #bookmarks-title-wrapper {
On 2011/09/05 01:59:20, Evan Stade wrote:
> might as well fix crbug.com/94968 while we're here

Could you clarify, do you mean the same colors (background gradient, line, text
colors) as the footer, or just the opacity?

Going to leave this to a separate CL.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.css:165: text-align: right;
On 2011/09/05 01:59:20, Evan Stade wrote:
> end not right

Done.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.css:173: text-align: right;
On 2011/09/05 01:59:20, Evan Stade wrote:
> also need to make this ok for RTL (I think you can just use text-align: end;
and
> margin: 0px -70px;)

Done.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/bookmarks_page.js (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/bookmarks_page.js:236: // insert the top & bottom
links for the Bookmarks Manager page.
On 2011/09/05 01:59:20, Evan Stade wrote:
> Insert

Done.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/new_tab.html (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/new_tab.html:139: <a href="chrome://bookmarks"
i18n-content="bookmarksManagerLinkTitle"></a>
On 2011/09/05 01:59:20, Evan Stade wrote:
> I think this should probably open the same URL as the bottom link. It's weird
we
> have two links that do the same thing but it would be weirder to have the top
> link disappear, and probably it's not what the user wants if they're deep in
> some folder and this link doesn't take them there.

Done.

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/tile_page.js (right):

http://codereview.chromium.org/7833021/diff/3001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/tile_page.js:326: var TILE_GRID_EXTRA_PADDING =
40;
On 2011/09/05 01:59:20, Evan Stade wrote:
> can you do this as a function on TilePage which children of TilePage can
> override, like
> 
> ExtraBottomPadding: function() {
>   return 40;
> }
> 
> that way I think it's not such a hack.

Done.