MD History: Use computed functions for card borders and time gaps

chrome/browser/resources/md_history/history_list.js

 // Copyright 2015 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.
 
 Polymer({
   is: 'history-list',
 
   properties: {
     // An array of history entries in reverse chronological order.
     historyData: {
@@ skipping 87 matching lines @@
 
     if (historyResults.length == 0)
       return;
 
     // Creates a copy of historyResults to prevent accidentally modifying this
     // field.
     var results = historyResults.slice();
 
     var currentDate = results[0].dateRelativeDay;
 
-    // Resets the last history item for the currentDate if new history results
-    // for currentDate is loaded.
-    if (this.historyData && this.historyData.length > 0) {
-      var lastHistoryItem = this.historyData[this.historyData.length - 1];
-      if (lastHistoryItem && lastHistoryItem.dateRelativeDay == currentDate) {
-        this.set('historyData.' + (this.historyData.length - 1) +
-            '.isLastItem', false);
-      }
-    }
-
     for (var i = 0; i < results.length; i++) {
       // Sets the default values for these fields to prevent undefined types.
       results[i].selected = false;
-      results[i].isLastItem = false;
-      results[i].isFirstItem = false;
-      results[i].needsTimeGap = this.needsTimeGap_(results, i);
       results[i].readableTimestamp =
           searchTerm == '' ? results[i].dateTimeOfDay : results[i].dateShort;
 
       if (results[i].dateRelativeDay != currentDate) {
-        results[i - 1].isLastItem = true;
-        results[i].isFirstItem = true;
         currentDate = results[i].dateRelativeDay;
       }
     }
-    results[i - 1].isLastItem = true;
-
-    if (!this.historyData || this.historyData.length == 0)
-      results[0].isFirstItem = true;
 
     if (this.historyData) {
       // If we have previously received data, push the new items onto the
       // existing array.
       results.unshift('historyData');
       this.push.apply(this, results);
     } else {
       // The first time we receive data, use set() to ensure the iron-list is
       // initialized correctly.
       this.set('historyData', results);
@@ skipping 25 matching lines @@
    * Remove all selected items from the overall array so that they are also
    * removed from view. Make sure that the card length and positioning is
    * updated accordingly.
    * @param {number} overallItemCount The number of items selected.
    */
   removeDeletedHistory: function(overallItemCount) {
     for (var i = this.historyData.length - 1; i >= 0; i--) {
       if (!this.historyData[i].selected)
         continue;
 
-      // TODO: Change to using computed properties to recompute the first and
-      // last cards.
-
-      // Resets the first history item.
-      if (this.historyData[i].isFirstItem &&
-          (i + 1) < this.historyData.length &&
-          this.historyData[i].dateRelativeDay ==
-          this.historyData[i + 1].dateRelativeDay) {
-        this.set('historyData.' + (i + 1) + '.isFirstItem', true);
-      }
-
-      // Resets the last history item.
-      if (this.historyData[i].isLastItem && i > 0 &&
-          this.historyData[i].dateRelativeDay ==
-          this.historyData[i - 1].dateRelativeDay) {
-        this.set('historyData.' + (i - 1) + '.isLastItem', true);
-
-        if (this.historyData[i - 1].needsTimeGap)
-          this.set('historyData.' + (i - 1) + '.needsTimeGap', false);
-      }
-
-      // Makes sure that the time gap separators are preserved.
-      if (this.historyData[i].needsTimeGap && i > 0)
-        this.set('historyData.' + (i - 1) + '.needsTimeGap', true);
-
-      // Removes the selected item from historyData.

[Dan Beam, 2016/04/12 19:08:29]

mayyyyyyyyyybe keep this comment?

[tsergeant, 2016/04/12 23:44:00]

Done.

       this.splice('historyData', i, 1);
 
       overallItemCount--;
       if (overallItemCount == 0)
         break;
     }
   },
 
   /**
    * Based on which items are selected, collect an array of the info required
@@ skipping 36 matching lines @@
         scrollElem.scrollTop + scrollElem.clientHeight + scrollOffset) {
       this.loading_ = true;
       chrome.send('queryHistory',
           [this.searchTerm, 0, 0, this.lastVisitedTime, RESULTS_PER_PAGE]);
     }
   },
 
   /**
    * Check whether the time difference between the given history item and the
    * next one is large enough for a spacer to be required.
-   * @param {Array<HistoryEntry>} results A list of history results.
-   * @param {number} index The index number of the first item being compared.
+   * @param {HistoryEntry} item
+   * @param {number} index The index of |item| in |historyData|.
+   * @param {number} length The length of |historyData|.
    * @return {boolean} Whether or not time gap separator is required.
    * @private
    */
-  needsTimeGap_: function(results, index) {
-    // TODO(tsergeant): Allow the final item from one batch of results to have a
-    // timegap once more results are added.
-    if (index == results.length - 1)
+  needsTimeGap_: function(item, index, length) {
+    if (index >= length - 1 || length == 0)
       return false;
 
-    var currentItem = results[index];
-    var nextItem = results[index + 1];
+    var currentItem = this.historyData[index];
+    var nextItem = this.historyData[index + 1];
 
     if (this.searchTerm)
       return currentItem.dateShort != nextItem.dateShort;
 
     return currentItem.time - nextItem.time > BROWSING_GAP_TIME &&
         currentItem.dateRelativeDay == nextItem.dateRelativeDay;
   },
 
   hasResults: function(historyDataLength) {
     return historyDataLength > 0;
   },
 
   noResultsMessage_: function(searchTerm, isLoading) {
     if (isLoading)
       return '';
     var messageId = searchTerm !== '' ? 'noSearchResults' : 'noResults';
     return loadTimeData.getString(messageId);
-  }
+  },
+
+  /**
+   * True if the given item is the beginning of a new card.
+   * @param {HistoryEntry} item
+   * @param {number} i

[Dan Beam, 2016/04/12 19:08:29]

d




ocument more for 1 character variables, I'd recommmend ;)

[tsergeant, 2016/04/12 23:44:00]

Done.

+   * @param {number} length
+   * @return {boolean}
+   * @private
+   */
+  isCardStart_: function(item, i, length) {
+    if (length == 0 || i > length - 1)
+      return false;
+    return i == 0 ||
+        this.historyData[i].dateRelativeDay !=
+        this.historyData[i - 1].dateRelativeDay;
+  },
+
+  /**
+   * True if the given item is the end of a card.
+   * @param {HistoryEntry} item
+   * @param {number} i

[Dan Beam, 2016/04/12 19:08:29]

s



ame

[tsergeant, 2016/04/12 23:44:00]

Done.

+   * @param {number} length
+   * @return {boolean}
+   * @private
+   */
+  isCardEnd_: function(item, i, length) {
+    if (length == 0 || i > length - 1)
+      return false;
+    return i == length - 1 ||
+        this.historyData[i].dateRelativeDay !=
+        this.historyData[i + 1].dateRelativeDay;
+  },
 });