NTP5: Making Apps page taller.

chrome/browser/resources/ntp_search/new_tab.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview New tab page
  * This is the main code for the new tab page. NewTabView manages page list,
  * dot list and handles apps pages callbacks from backend. It also handles
  * the layout of the Bottom Panel and the global UI states of the New Tab Page.
  */
@@ skipping 118 matching lines @@
    * @type {number|undefined}
    * @private
    */
   var headerHeight;
 
   /**
    * The height of the Bottom Panel Footer, in pixels.
    * @type {number|undefined}
    * @private
    */
   var footerHeight;

[Dan Beam, 2012/12/14 03:44:42]

remove this now that it can be a local, not a global

[pedro (no code reviews), 2012/12/14 05:06:33]

Done.

 
   /**
    * The time in milliseconds for most transitions.  This should match what's
    * in new_tab.css.  Unfortunately there's no better way to try to time
    * something to occur until after a transition has completed.
    * @type {number}
    * @const
    */
   var DEFAULT_TRANSITION_TIME = 500;
 
@@ skipping 578 matching lines @@
       if (width != this.contentWidth_) {
         this.contentWidth_ = width;
         $('bottom-panel-footer').style.width = width + 'px';
       }
 
       // Finally, dispatch the layout method to the current page.
       var currentPage = opt_page || this.cardSlider.currentCardValue;
 
       var contentHeight = TILE_ROW_HEIGHT;
       if (!opt_page && currentPage.config.scrollable) {
+        footerHeight = $('bottom-panel-footer').offsetHeight;

[Dan Beam, 2012/12/14 03:44:42]

nit: var footerHeight; (make a local)

[pedro (no code reviews), 2012/12/14 05:06:33]

I would normally create a variable, just for readability purposes, but since
$('bottom-panel-footer').offsetHeight is also pretty readable, I'll accept your
second suggestion.

         contentHeight = viewHeight - bottomPanelOffsetTop -
             headerHeight - footerHeight;

[Dan Beam, 2012/12/14 03:44:42]

or just inline $('bottom-panel-footer').offsetHeight

[pedro (no code reviews), 2012/12/14 05:06:33]

Done.

         contentHeight = Math.max(TILE_ROW_HEIGHT, contentHeight);
-        contentHeight = Math.min(2 * TILE_ROW_HEIGHT, contentHeight);
       }
       this.contentHeight_ = contentHeight;
 
       $('card-slider-frame').style.height = contentHeight + 'px';
 
       currentPage.layout(opt_animate);
     },
 
     /**
      * @return {number} The height of the Bottom Panel's content.
@@ skipping 49 matching lines @@
    * Invoked at startup once the DOM is available to initialize the app.
    */
   function onLoad() {
     // Load the current theme colors.
     themeChanged();
 
     newTabView = new NewTabView();
 
     bottomPanelOffsetTop = $('bottom-panel').offsetTop;
     headerHeight = $('bottom-panel-header').offsetHeight;
-    footerHeight = $('bottom-panel-footer').offsetHeight;
 
     notificationContainer = getRequiredElement('notification-container');
     notificationContainer.addEventListener(
         'webkitTransitionEnd', onNotificationTransitionEnd);
 
     var mostVisited = new ntp.MostVisitedPage();
     newTabView.appendTilePage(mostVisited,
                               loadTimeData.getString('mostvisited'));
     chrome.send('getMostVisited');
 
@@ skipping 183 matching lines @@
     document.querySelector('#notification button').onclick = closeFunc;
     document.addEventListener('dragstart', closeFunc);
 
     notificationContainer.hidden = false;
     window.setTimeout(function() {
       notificationContainer.classList.remove('inactive');
     }, 0);
 
     var timeout = opt_timeout || 10000;
     notificationTimeout = window.setTimeout(hideNotification, timeout);
+
+    layout();
   }
 
   /**
    * Hide the notification bubble.
    */
   function hideNotification() {
     notificationContainer.classList.add('inactive');
+
+    layout();
   }
 
   /**
    * When done fading out, set hidden to true so the notification can't be
    * tabbed to or clicked.
    * @param {Event} e The webkitTransitionEnd event.
    */
   function onNotificationTransitionEnd(e) {
     if (notificationContainer.classList.contains('inactive'))
       notificationContainer.hidden = true;
@@ skipping 65 matching lines @@
   }
 
   function getCardSlider() {
     return newTabView.cardSlider;
   }
 
   function setAppToBeHighlighted(appId) {
     newTabView.highlightAppId = appId;
   }
 
   function layout(opt_animate) {

[Dan Beam, 2012/12/14 03:44:42]

shouldn't this also take opt_page? why aren't use using the

  function forward() {
    newTabView.forward.apply(newTabView, arguments);
  }

forwarding pattern?

[Dan Beam, 2012/12/14 03:45:02]

why aren't you using**

[pedro (no code reviews), 2012/12/14 05:06:33]

Done.

[pedro (no code reviews), 2012/12/14 05:06:33]

Using the forwarding pattern now.

     newTabView.layout(opt_animate);
   }
 
   function getContentHeight() {
     return newTabView.contentHeight;
   }
 
   function getContentWidth() {
     return newTabView.contentWidth;
   }
@@ skipping 27 matching lines @@
     setRecentlyClosedTabs: noop,
     showNotification: showNotification,
     themeChanged: themeChanged,
     updateLogin: noop,
   };
 });
 
 document.addEventListener('DOMContentLoaded', ntp.onLoad);
 
 var toCssPx = cr.ui.toCssPx;