diff --git a/build-system/server.js b/build-system/server.js index ca46bed51587..e9b3c4a6b731 100644 --- a/build-system/server.js +++ b/build-system/server.js @@ -24,11 +24,12 @@ var bodyParser = require('body-parser'); var clr = require('connect-livereload'); var finalhandler = require('finalhandler'); var fs = BBPromise.promisifyAll(require('fs')); +var jsdom = require('jsdom'); var path = require('path'); -var url = require('url'); var request = require('request'); var serveIndex = require('serve-index'); var serveStatic = require('serve-static'); +var url = require('url'); var args = Array.prototype.slice.call(process.argv, 2, 4); var paths = args[0]; @@ -103,6 +104,34 @@ app.use('/examples.build/live-list.amp.max.html', function(req, res) { }); }); +var liveListUpdateFile = '/examples.build/live-list-update.amp.max.html'; +var liveListUpdateFullPath = `${process.cwd()}${liveListUpdateFile}`; +var liveListFile = fs.readFileSync(liveListUpdateFullPath); +var ctr = 0; +var doc = jsdom.jsdom(liveListFile); +var win = doc.defaultView; +app.use(liveListUpdateFile, function(req, res) { + var doctype = '\n'; + res.setHeader('Content-Type', 'text/html'); + res.statusCode = 200; + if (ctr != 0) { + if (Math.random() < .7) { + var item1 = doc.querySelector('#list-item-1'); + var item1Content = item1.querySelectorAll('.content'); + item1.setAttribute('data-update-time', Date.now()); + item1Content[0].textContent = Math.floor(Math.random() * 10); + item1Content[1].textContent = Math.floor(Math.random() * 10); + } else { + // Sometimes we want an empty response to simulate no changes. + res.end(`${doctype}`); + return; + } + } + var outerHTML = doc.documentElement./*OK*/outerHTML; + res.end(`${doctype}${outerHTML}`); + ctr++; +}); + // Proxy with unminified JS. // Example: // http://localhost:8000/max/s/www.washingtonpost.com/amphtml/news/post-politics/wp/2016/02/21/bernie-sanders-says-lower-turnout-contributed-to-his-nevada-loss-to-hillary-clinton/ diff --git a/examples/img/ampicon.png b/examples/img/ampicon.png new file mode 100644 index 000000000000..a2e8b92ae879 Binary files /dev/null and b/examples/img/ampicon.png differ diff --git a/examples/live-list-update.amp.html b/examples/live-list-update.amp.html new file mode 100644 index 000000000000..2f42f787e3bc --- /dev/null +++ b/examples/live-list-update.amp.html @@ -0,0 +1,109 @@ + + + + + Lorem Ipsum | PublisherName + + + + + + + + + + +
+ Scoreboard refresh takes 15 seconds. +
+ + +
+
+
+
+ +
2
+
+
+
2
+ +
+
+
+
+
+
+ +
2
+
+
+
2
+ +
+
+
+
+ + + + diff --git a/extensions/amp-live-list/0.1/amp-live-list.js b/extensions/amp-live-list/0.1/amp-live-list.js index 27512f92e2f5..3c20537ef35a 100644 --- a/extensions/amp-live-list/0.1/amp-live-list.js +++ b/extensions/amp-live-list/0.1/amp-live-list.js @@ -38,7 +38,7 @@ const classes = { /** * @typedef {{ * insert: !Array, - * update: !Array, + * replace: !Array, * tombstone: !Array * }} */ @@ -54,6 +54,7 @@ export class LiveListInterface { * Update the underlying live list dom structure. * * @param {?Element} element + * @return {time} */ update(unusedElement) { } @@ -85,6 +86,7 @@ export function getNumberMaxOrDefault(value, defaultValue) { /** * Component class that handles updates to its underlying children dom * structure. + * * @implements {LiveListInterface} */ export class AmpLiveList extends AMP.BaseElement { @@ -140,7 +142,11 @@ export class AmpLiveList extends AMP.BaseElement { this.manager_.register(this.liveListId_, this); - this.insertFragment_ = this.win.document.createDocumentFragment(); + /** @private @const {!Array} */ + this.pendingItemsInsert_ = []; + + /** @private @const {!Array} */ + this.pendingItemsReplace_ = []; this.updateSlot_ = user.assert( this.getUpdateSlot_(this.element), @@ -158,6 +164,9 @@ export class AmpLiveList extends AMP.BaseElement { this.validateLiveListItems_(this.itemsSlot_, true); this.registerAction('update', this.updateAction_.bind(this)); + + /** @private @const {function(!Element, !Element): number} */ + this.comparator_ = this.sortByDataSortTime_.bind(this); } /** @override */ @@ -167,22 +176,17 @@ export class AmpLiveList extends AMP.BaseElement { this.validateLiveListItems_(container); const mutateItems = this.getUpdates_(container); - // Insert/new items will be contiguous at the top even though they - // weren't in the actual request DOM structure. - const comparator = this.sortByDataSortTime_.bind(this); - mutateItems.insert.sort(comparator).forEach(child => { - child.classList.add(classes.ITEM); - child.classList.add(classes.NEW_ITEM); - // Since we only manipulate the DocumentFragment instance and not the - // live dom we don't need to be inside a vsync.mutate context. - this.insertFragment_.insertBefore(child, - this.insertFragment_.firstElementChild); - }); + this.preparePendingItemsInsert_(mutateItems.insert); + this.preparePendingItemsReplace_(mutateItems.replace); - if (mutateItems.insert.length > 0) { + // We prefer user interaction if we have pending items to insert at the + // top of the component. + if (this.pendingItemsInsert_.length > 0) { this.deferMutate(() => { this.updateSlot_.classList.remove('-amp-hidden'); }); + } else if (this.pendingItemsReplace_.length > 0) { + this.updateAction_(); } return this.updateTime_; @@ -191,39 +195,148 @@ export class AmpLiveList extends AMP.BaseElement { /** * Mutates the current elements dom and compensates for scroll * change if necessary. + * Makes sure to zero out the pending items arrays after flushing + * server DOM to live client DOM. + * * @return {!Promise} * @private */ updateAction_() { - if (this.insertFragment_.childElementCount == 0) { - return Promise.resolve(); - } + const hasNewInsert = this.pendingItemsInsert_.length > 0; // TODO(erwinm): do in place update as well as sorting, // correct insertion, tombstoning etc. - return this.mutateElement(() => { - // Remove the new class from the previously inserted items. - this.eachChildElement_(this.itemsSlot_, child => { - child.classList.remove(classes.NEW_ITEM); - }); - // Items are reparented from the document fragment to the live DOM - // by the `insertBefore` and `appendChild` operations, so we can reuse - // the same document fragment instance safely. - this.itemsSlot_.insertBefore(this.insertFragment_, - this.itemsSlot_.firstElementChild); + let promise = this.mutateElement(() => { - // Hide the update button in case we previously displayed it. + if (hasNewInsert) { + // Remove the new class from the previously inserted items if + // we are inserting new items. + this.eachChildElement_(this.itemsSlot_, child => { + child.classList.remove(classes.NEW_ITEM); + }); + + this.insert_(this.itemsSlot_, this.pendingItemsInsert_); + this.pendingItemsInsert_.length = 0; + } + + if (this.pendingItemsReplace_.length > 0) { + this.replace_(this.itemsSlot_, this.pendingItemsReplace_); + this.pendingItemsReplace_.length = 0; + } + + // Always hide update slot after mutation operation. this.updateSlot_.classList.add('-amp-hidden'); + }); - // TODO(erwinm): Handle updates - }).then(() => { - this.getVsync().mutate(() => { - // Should scroll into view be toggleable - this.viewport_./*OK*/scrollIntoView(this.element); + if (hasNewInsert) { + promise = promise.then(() => { + this.getVsync().mutate(() => { + // Should scroll into view be toggleable + this.viewport_./*OK*/scrollIntoView(this.element); + }); }); + } + return promise; + } + + /** + * Reparents the html from the server to the live DOM. + * + * @param {!Element} parent + * @param {!Array} orphans + * @private + */ + insert_(parent, orphans) { + const fragment = this.win.document.createDocumentFragment(); + orphans.forEach(elem => { + fragment.insertBefore(elem, fragment.firstElementChild); + }); + parent.insertBefore(fragment, parent.firstElementChild); + } + + /** + * Does an inline replace of a list item using the element ID. + * Does nothing if item has already been tombstoned or removed from the + * live DOM. + * + * @param {!Element} parent + * @param {!Array} orphans + * @private + */ + replace_(parent, orphans) { + orphans.forEach(orphan => { + const orphanId = orphan.getAttribute('id'); + const liveElement = parent.querySelector(`#${orphanId}`); + // Don't bother updating if live element is tombstoned or + // if we can't find it. + if (!liveElement || liveElement.hasAttribute('data-tombstone')) { + return; + } + parent.replaceChild(orphan, liveElement); }); } + /** + * Prepares the items from the server to be inserted into the DOM + * by sorting using `data-sort-time` and adding the needed list items + * classes for styling. + * + * @param {!Array} items + * @private + */ + preparePendingItemsInsert_(items) { + // Insert/new items will be contiguous at the top even though they + // weren't in the actual request DOM structure as it doesn't make sense + // to insert new items between old items. + // Order matters as this is how it will be appended into the DOM. + items.sort(this.comparator_).forEach(elem => { + elem.classList.add(classes.ITEM); + elem.classList.add(classes.NEW_ITEM); + }); + this.pendingItemsInsert_.push.apply(this.pendingItemsInsert_, items); + } + + /** + * Prepares the items from the server to directly replace an item in the + * live DOM. If item has a counterpart in the current pending changes + * that hasn't been flushed yet we just swap it out directly, else + * we push it into the array. + * Makes sure to add the `amp-live-list-item` class for items styling. + * + * @param {!Array} items + * @private + */ + preparePendingItemsReplace_(items) { + // Order doesn't matter since we do an in place replacement. + items.forEach(elem => { + const hasPendingCounterpart = this.hasMatchingPendingElement_( + this.pendingItemsReplace_, elem); + elem.classList.add('amp-live-list-item'); + if (hasPendingCounterpart == -1) { + this.pendingItemsReplace_.push(elem); + } else { + this.pendingItemsReplace_[hasPendingCounterpart] = elem; + } + }); + } + + /** + * Returns the index of the matching element from the queue using the + * element id. Otherwise returns -1 for not found. + * + * @param {!Array} pendingQueue + * @param {!Element} elem + * @return {number} + */ + hasMatchingPendingElement_(pendingQueue, elem) { + for (let i = 0; i < pendingQueue.length; i++) { + if (pendingQueue[i].getAttribute('id') == elem.getAttribute('id')) { + return i; + } + } + return -1; + } + /** @override */ getInterval() { return this.pollInterval_; @@ -243,7 +356,7 @@ export class AmpLiveList extends AMP.BaseElement { // trigger `createdCallback`s even though we won't actually be // reparenting those nodes to this tree. const insert = []; - const updates = []; + const replace = []; const tombstone = []; for (let child = updatedElement.firstElementChild; child; @@ -261,15 +374,16 @@ export class AmpLiveList extends AMP.BaseElement { if (updateTime > this.updateTime_) { this.updateTime_ = updateTime; } - updates.push(orphan); + replace.push(orphan); } } - return {insert, updates, tombstone}; + return {insert, replace, tombstone}; } /** * Predicate to check if the child passed in is new. + * * @param {!Element} elem * @return {boolean} * @private @@ -282,6 +396,7 @@ export class AmpLiveList extends AMP.BaseElement { /** * Predicate to check if the child passed in is an update, determined * by data-update-time attribute. + * * @param {!Element} elem * @return {boolean} * @private @@ -370,6 +485,7 @@ export class AmpLiveList extends AMP.BaseElement { /** * Iterates over the child elements and invokes the callback with * the current child element passed in as the first argument. + * * @param {!Element} parent * @param {function(!Element)} cb * @private @@ -400,16 +516,17 @@ export class AmpLiveList extends AMP.BaseElement { /** * @param {!Element} a * @param {!Element} b - * @return {number} + * @return {time} * @private */ sortByDataSortTime_(a, b) { - return this.getSortTime_(a) - this.getSortTime_(b); + // Sort from newest to oldest so we don't have to reverse + return this.getSortTime_(b) - this.getSortTime_(a); } /** * @param {!Element} elem - * @return {number} + * @return {time} * @private */ getSortTime_(elem) { @@ -418,7 +535,7 @@ export class AmpLiveList extends AMP.BaseElement { /** * @param {!Element} elem - * @return {number} + * @return {time} * @private */ getUpdateTime_(elem) { @@ -430,7 +547,7 @@ export class AmpLiveList extends AMP.BaseElement { /** * @param {!Element} elem - * @return {number} + * @return {time} * @private */ getTimeAttr_(elem, attr) { diff --git a/extensions/amp-live-list/0.1/test/test-amp-live-list.js b/extensions/amp-live-list/0.1/test/test-amp-live-list.js index 65889f40d535..de1239f9ea3e 100644 --- a/extensions/amp-live-list/0.1/test/test-amp-live-list.js +++ b/extensions/amp-live-list/0.1/test/test-amp-live-list.js @@ -259,7 +259,7 @@ describe('amp-live-list', () => { const fromServer1 = createFromServer([{id: 'id0'}]); liveList.update(fromServer1); - expect(liveList.insertFragment_.childElementCount).to.equal(0); + expect(liveList.pendingItemsInsert_).to.have.length(0); const fromServer2 = createFromServer([ {id: 'id0'}, {id: 'id1'}, {id: 'id2'}, @@ -267,9 +267,9 @@ describe('amp-live-list', () => { const fromServer2ItemsCont = fromServer2 .querySelector('[items]'); expect(fromServer2ItemsCont.childElementCount).to.equal(3); - expect(liveList.insertFragment_.childElementCount).to.equal(0); + expect(liveList.pendingItemsInsert_).to.have.length(0); liveList.update(fromServer2); - expect(liveList.insertFragment_.childElementCount).to.equal(2); + expect(liveList.pendingItemsInsert_).to.have.length(2); }); it('should wait for user interaction before inserting', () => { @@ -278,9 +278,9 @@ describe('amp-live-list', () => { expect(liveList.itemsSlot_.childElementCount).to.equal(0); const fromServer1 = createFromServer([{id: 'id0'}]); liveList.update(fromServer1); - expect(liveList.insertFragment_.childElementCount).to.equal(1); + expect(liveList.pendingItemsInsert_).to.have.length(1); return liveList.updateAction_().then(() => { - expect(liveList.insertFragment_.childElementCount).to.equal(0); + expect(liveList.pendingItemsInsert_).to.have.length(0); expect(liveList.itemsSlot_.childElementCount).to.equal(1); }); }); @@ -371,7 +371,7 @@ describe('amp-live-list', () => { const fromServer1 = createFromServer([{id: 'id0'}]); liveList.update(fromServer1); - expect(liveList.insertFragment_.childElementCount).to.equal(1); + expect(liveList.pendingItemsInsert_).to.have.length(1); expect(liveList.updateSlot_).to.not.have.class('-amp-hidden'); return liveList.updateAction_(fromServer1).then(() => { @@ -395,11 +395,11 @@ describe('amp-live-list', () => { liveList.update(fromServer1); - expect(liveList.insertFragment_.children[0].getAttribute('id')) + expect(liveList.pendingItemsInsert_[0].getAttribute('id')) .to.equal('unique-id-num-4'); - expect(liveList.insertFragment_.children[1].getAttribute('id')) + expect(liveList.pendingItemsInsert_[1].getAttribute('id')) .to.equal('unique-id-num-3'); - expect(liveList.insertFragment_.children[2].getAttribute('id')) + expect(liveList.pendingItemsInsert_[2].getAttribute('id')) .to.equal('unique-id-num-5'); }); }); @@ -454,6 +454,131 @@ describe('amp-live-list', () => { expect(liveList.update(fromServer5)).to.equal(600); }); + it('should be able to accumulate insert items', () => { + const child1 = document.createElement('div'); + const child2 = document.createElement('div'); + child1.setAttribute('id', 'id1'); + child2.setAttribute('id', 'id2'); + child1.setAttribute('data-sort-time', '123'); + child2.setAttribute('data-sort-time', '124'); + itemsSlot.appendChild(child1); + itemsSlot.appendChild(child2); + buildElement(elem, dftAttrs); + liveList.buildCallback(); + + const fromServer1 = createFromServer([ + {id: 'id1', updateTime: 125}, + {id: 'id3'}, + ]); + + const spy = sandbox.spy(liveList, 'updateAction_'); + liveList.update(fromServer1); + + expect(liveList.pendingItemsInsert_).to.have.length(1); + expect(spy.callCount).to.equal(0); + + const fromServer2 = createFromServer([ + {id: 'id4'}, + {id: 'id7'}, + {id: 'id9'}, + ]); + liveList.update(fromServer2); + expect(liveList.pendingItemsInsert_).to.have.length(4); + expect(spy.callCount).to.equal(0); + }); + + it('should have pending replace items', () => { + const child1 = document.createElement('div'); + const child2 = document.createElement('div'); + child1.setAttribute('id', 'id1'); + child2.setAttribute('id', 'id2'); + child1.setAttribute('data-sort-time', '123'); + child2.setAttribute('data-sort-time', '124'); + itemsSlot.appendChild(child1); + itemsSlot.appendChild(child2); + buildElement(elem, dftAttrs); + liveList.buildCallback(); + + const fromServer1 = createFromServer([ + {id: 'id1', updateTime: 125}, + {id: 'id3'}, + ]); + + const spy = sandbox.spy(liveList, 'updateAction_'); + liveList.update(fromServer1); + + expect(liveList.pendingItemsInsert_).to.have.length(1); + expect(liveList.pendingItemsReplace_).to.have.length(1); + // Should wait for user action until `updateAction_` + expect(spy.callCount).to.equal(0); + }); + + it('should have pending replace items even w/o new inserts', () => { + const child1 = document.createElement('div'); + const child2 = document.createElement('div'); + child1.setAttribute('id', 'id1'); + child2.setAttribute('id', 'id2'); + child1.setAttribute('data-sort-time', '123'); + child2.setAttribute('data-sort-time', '124'); + itemsSlot.appendChild(child1); + itemsSlot.appendChild(child2); + buildElement(elem, dftAttrs); + liveList.buildCallback(); + + const fromServer1 = createFromServer([ + {id: 'id1', updateTime: 125}, + ]); + + const spy = sandbox.spy(liveList, 'updateAction_'); + liveList.update(fromServer1); + + expect(liveList.pendingItemsInsert_).to.have.length(0); + expect(liveList.pendingItemsReplace_).to.have.length(1); + // If there is no pending items to insert, flush the replace items + // right away. + expect(spy.callCount).to.equal(1); + }); + + it('should always use latest update to replace when in pending state', () => { + const child1 = document.createElement('div'); + const child2 = document.createElement('div'); + child1.setAttribute('id', 'id1'); + child2.setAttribute('id', 'id2'); + child1.setAttribute('data-sort-time', '123'); + child2.setAttribute('data-sort-time', '124'); + itemsSlot.appendChild(child1); + itemsSlot.appendChild(child2); + buildElement(elem, dftAttrs); + liveList.buildCallback(); + + const fromServer1 = createFromServer([ + {id: 'id1', updateTime: 125}, + {id: 'id3'}, + ]); + + const spy = sandbox.spy(liveList, 'updateAction_'); + liveList.update(fromServer1); + + expect(liveList.pendingItemsReplace_).to.have.length(1); + expect(liveList.pendingItemsReplace_[0].getAttribute('id')).to.equal('id1'); + expect(liveList.pendingItemsReplace_[0].getAttribute('data-update-time')) + .to.equal('125'); + // Should wait for user action until `updateAction_` + expect(spy.callCount).to.equal(0); + + const fromServer2 = createFromServer([ + {id: 'id1', updateTime: 127}, + ]); + liveList.update(fromServer2); + + expect(liveList.pendingItemsReplace_).to.have.length(1); + expect(liveList.pendingItemsReplace_[0].getAttribute('id')).to.equal('id1'); + expect(liveList.pendingItemsReplace_[0].getAttribute('data-update-time')) + .to.equal('127'); + + expect(spy.callCount).to.equal(0); + }); + describe('#getNumberMaxOrDefault', () => { it('should return correct value', () => { diff --git a/gulpfile.js b/gulpfile.js index 4d657e7ec15f..1ab2f51ee40c 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -314,6 +314,7 @@ function buildExamples(watch) { buildExample('csp.amp.html'); buildExample('layout-flex-item.amp.html'); buildExample('live-list.amp.html'); + buildExample('live-list-update.amp.html'); buildExample('metadata-examples/article-json-ld.amp.html'); buildExample('metadata-examples/article-microdata.amp.html'); buildExample('metadata-examples/recipe-json-ld.amp.html'); diff --git a/package.json b/package.json index 70fb14ebab59..e231f63125e5 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,8 @@ "heroku-postbuild": "gulp clean && gulp build --fortesting && gulp dist --fortesting" }, "dependencies": { - "promise-pjs": "1.1.2", - "document-register-element": "0.5.4" + "document-register-element": "0.5.4", + "promise-pjs": "1.1.2" }, "devDependencies": { "autoprefixer": "6.1.1", @@ -62,6 +62,7 @@ "gulp-wrap": "0.11.0", "gzip-size": "3.0.0", "jison": "0.4.15", + "jsdom": "9.1.0", "karma": "0.13.21", "karma-browserify": "5.0.2", "karma-chai": "0.1.0",