NTP5: Fine tuning of Apps implementation.

chrome/browser/resources/ntp_search/apps_page.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.
 
 cr.define('ntp', function() {
   'use strict';
 
   var Tile = ntp.Tile;
   var TilePage = ntp.TilePage;
   var APP_LAUNCH = ntp.APP_LAUNCH;
@@ skipping 558 matching lines @@
      * Reference to the Tile subclass that will be used to create the tiles.
      * @constructor
      * @extends {Tile}
      */
     TileClass: App,
 
     // The config object should be defined by a TilePage subclass if it
     // wants the non-default behavior.
     config: {
       // The width of a cell.
-      cellWidth: 77,
+      cellWidth: 70,
       // The start margin of a cell (left or right according to text direction).
       cellMarginStart: 12,
       // The maximum number of Tiles to be displayed.
-      maxTileCount: 20
+      maxTileCount: 20,
+      // Whether the TilePage content will be scrollable.
+      scrollable: true,
     },
 
     initialize: function() {
+      TilePage.prototype.initialize.apply(this, arguments);
+
       this.classList.add('apps-page');
 
       this.addEventListener('cardselected', this.onCardSelected_);
       // Add event listeners for two events, so we can temporarily suppress
       // the app notification bubbles when the app card slides in and out of
       // view.
       this.addEventListener('carddeselected', this.onCardDeselected_);
       this.addEventListener('cardSlider:card_change_ended',
                             this.onCardChangeEnded_);
 
       this.addEventListener('tilePage:tile_added', this.onTileAdded_);
-
-      this.content_.addEventListener('scroll', this.onScroll_.bind(this));
     },
 
     /**
      * Highlight a newly installed app as it's added to the NTP.
      * @param {Object} appData The data object that describes the app.
      */
     insertAndHighlightApp: function(appData) {
       ntp.getCardSlider().selectCardByValue(this);
-      this.content_.scrollTop = this.content_.scrollHeight;
-      this.insertApp(appData, true);
+      var app = this.insertApp(appData, true);
+      this.content_.scrollTop = app.tileCell.offsetTop;

[Dan Beam, 2012/11/30 23:41:45]

this might be a bit jerky... have you tried when this changes stuff?

[pedro (no code reviews), 2012/12/01 00:02:29]

What do you mean by "changes stuff"?

I've tested this and it does properly scroll to the proper place, revealing the
App that has just been added.

[Dan Beam, 2012/12/01 00:21:16]

sorry, "changes stuff" -> "an app is inserted".  if you've tried it and it
doesn't look so bad, that's fine, I just imagined it jerking to the new app's
spot rather than smoothly scrolling to it (desired, I'd think)

     },
 
     /**
-     * Similar to appendApp, but it respects the app_launch_ordinal field of
-     * |appData|.
+     * Inserts an App into the TilePage, preserving the alphabetical order.
      * @param {Object} appData The data that describes the app.
      * @param {boolean} animate Whether to animate the insertion.
      */
     insertApp: function(appData, animate) {
       var index = this.tiles_.length;
       for (var i = 0; i < this.tiles_.length; i++) {
-        if (appData.app_launch_ordinal <
-            this.tiles_[i].appData.app_launch_ordinal) {
+        if (appData.title.toLocaleLowerCase() <
+            this.tiles_[i].appData.title.toLocaleLowerCase()) {
           index = i;
           break;
         }
       }
 
-      this.addTileAt(new App(appData), index);
+      var app = new App(appData);
+      this.addTileAt(app, index);
       this.renderGrid_();
+
+      return app;
     },
 
     /**
      * Handler for 'cardselected' event, fired when |this| is selected. The
      * first time this is called, we load all the app icons.
      * @private
      */
     onCardSelected_: function(e) {
       var apps = this.querySelectorAll('.app.icon-loading');
       for (var i = 0; i < apps.length; i++) {
@@ skipping 37 matching lines @@
      */
     onCardDeselected_: function(e) {
       for (var i = 0; i < this.tiles_.length; i++) {
         var app = this.tiles_[i];
         assert(app instanceof App);
         if (app.currentBubbleShowing_)
           app.currentBubbleShowing_.suppressed = true;
       }
     },
 
-    /**
-     * A handler for when the apps page is scrolled (then we need to reposition
-     * the bubbles.
-     * @private
-     */
-    onScroll_: function(e) {
-      if (!this.selected)
-        return;
+    /** @override */
+    onScroll: function() {
+      TilePage.prototype.onScroll.apply(this, arguments);
+
       for (var i = 0; i < this.tiles_.length; i++) {
         var app = this.tiles_[i];
         assert(app instanceof App);
         if (app.currentBubbleShowing_)
           app.currentBubbleShowing_.resizeAndReposition();
         }
     },
 
     /** @override */
     doDragOver: function(e) {
@@ skipping 135 matching lines @@
     if (app && !app.appData.notifications_disabled)
       app.setupNotification_(notification);
   }
 
   return {
     appNotificationChanged: appNotificationChanged,
     AppsPage: AppsPage,
     launchAppAfterEnable: launchAppAfterEnable,
   };
 });