From 40c8d391379a7fd72e6bbd3fc2ff3a2ce3174af3 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 1 Dec 2015 17:27:32 -0500 Subject: [PATCH] FEATURE: Reply Placeholders in Stream --- .../discourse/controllers/composer.js.es6 | 2 +- .../discourse/controllers/topic.js.es6 | 15 +- .../javascripts/discourse/lib/url.js.es6 | 47 ++- .../discourse/models/post-stream.js.es6 | 270 +++++++++++------- .../javascripts/discourse/models/topic.js.es6 | 6 +- .../discourse/routes/topic-unsubscribe.js.es6 | 7 +- .../discourse/templates/post-placeholder.hbs | 13 + .../javascripts/discourse/templates/topic.hbs | 5 +- .../discourse/views/cloaked-collection.js.es6 | 26 +- .../discourse/views/cloaked.js.es6 | 27 +- .../discourse/views/post-placeholder.js.es6 | 1 + .../stylesheets/common/base/topic-post.scss | 16 ++ .../stylesheets/desktop/topic-post.scss | 1 + .../models/post-stream-test.js.es6 | 61 +++- 14 files changed, 312 insertions(+), 185 deletions(-) create mode 100644 app/assets/javascripts/discourse/templates/post-placeholder.hbs create mode 100644 app/assets/javascripts/discourse/views/post-placeholder.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6 index 822c833010c..1c7384c254b 100644 --- a/app/assets/javascripts/discourse/controllers/composer.js.es6 +++ b/app/assets/javascripts/discourse/controllers/composer.js.es6 @@ -115,7 +115,7 @@ export default Ember.Controller.extend({ // If there is no current post, use the first post id from the stream if (!postId && postStream) { - postId = postStream.get('firstPostId'); + postId = postStream.get('stream.firstObject'); } // If we're editing a post, fetch the reply when importing a quote diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 435418ecf00..a6e098ee2d2 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -668,8 +668,8 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { topVisibleChanged(post) { if (!post) { return; } - const postStream = this.get('model.postStream'), - firstLoadedPost = postStream.get('firstLoadedPost'); + const postStream = this.get('model.postStream'); + const firstLoadedPost = postStream.get('posts.firstObject'); this.set('model.currentPost', post.get('post_number')); @@ -680,15 +680,16 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { // trigger a scroll after a promise resolves in a controller? We need // to do this to preserve upwards infinte scrolling. const $body = $('body'); - let $elem = $('#post-cloak-' + post.get('post_number')); + const elemId = `#post_${post.get('post_number')}`; + const $elem = $(elemId).closest('.post-cloak'); const distToElement = $body.scrollTop() - $elem.position().top; postStream.prependMore().then(function() { Em.run.next(function () { - $elem = $('#post-cloak-' + post.get('post_number')); + const $refreshedElem = $(elemId).closest('.post-cloak'); // Quickly going back might mean the element is destroyed - const position = $elem.position(); + const position = $refreshedElem.position(); if (position && position.top) { $('html, body').scrollTop(position.top + distToElement); } @@ -706,8 +707,8 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { bottomVisibleChanged(post) { if (!post) { return; } - const postStream = this.get('model.postStream'), - lastLoadedPost = postStream.get('lastLoadedPost'); + const postStream = this.get('model.postStream'); + const lastLoadedPost = postStream.get('posts.lastObject'); this.set('controllers.topic-progress.progressPosition', postStream.progressIndexOfPost(post)); diff --git a/app/assets/javascripts/discourse/lib/url.js.es6 b/app/assets/javascripts/discourse/lib/url.js.es6 index 14e2392744e..7a39b5bdaa0 100644 --- a/app/assets/javascripts/discourse/lib/url.js.es6 +++ b/app/assets/javascripts/discourse/lib/url.js.es6 @@ -15,14 +15,13 @@ const DiscourseURL = Ember.Object.createWithMixins({ Jumps to a particular post in the stream **/ jumpToPost: function(postNumber, opts) { - const holderId = '#post-cloak-' + postNumber; + const holderId = `#post_${postNumber}`; + const offset = function() { - const offset = function(){ - - const $header = $('header'), - $title = $('#topic-title'), - windowHeight = $(window).height() - $title.height(), - expectedOffset = $title.height() - $header.find('.contents').height() + (windowHeight / 5); + const $header = $('header'); + const $title = $('#topic-title'); + const windowHeight = $(window).height() - $title.height(); + const expectedOffset = $title.height() - $header.find('.contents').height() + (windowHeight / 5); return $header.outerHeight(true) + ((expectedOffset < 0) ? 0 : expectedOffset); }; @@ -203,40 +202,40 @@ const DiscourseURL = Ember.Object.createWithMixins({ @param {String} oldPath the previous path we were on @param {String} path the path we're navigating to **/ - navigatedToPost: function(oldPath, path) { - const newMatches = this.TOPIC_REGEXP.exec(path), - newTopicId = newMatches ? newMatches[2] : null; + navigatedToPost(oldPath, path) { + const newMatches = this.TOPIC_REGEXP.exec(path); + const newTopicId = newMatches ? newMatches[2] : null; if (newTopicId) { - const oldMatches = this.TOPIC_REGEXP.exec(oldPath), - oldTopicId = oldMatches ? oldMatches[2] : null; + const oldMatches = this.TOPIC_REGEXP.exec(oldPath); + const oldTopicId = oldMatches ? oldMatches[2] : null; // If the topic_id is the same if (oldTopicId === newTopicId) { DiscourseURL.replaceState(path); - const container = Discourse.__container__, - topicController = container.lookup('controller:topic'), - opts = {}, - postStream = topicController.get('model.postStream'); + const container = Discourse.__container__; + const topicController = container.lookup('controller:topic'); + const opts = {}; + const postStream = topicController.get('model.postStream'); - if (newMatches[3]) opts.nearPost = newMatches[3]; + if (newMatches[3]) { opts.nearPost = newMatches[3]; } if (path.match(/last$/)) { opts.nearPost = topicController.get('model.highest_post_number'); } const closest = opts.nearPost || 1; - const self = this; - postStream.refresh(opts).then(function() { + postStream.refresh(opts).then(() => { topicController.setProperties({ 'model.currentPost': closest, enteredAt: new Date().getTime().toString() }); - const closestPost = postStream.closestPostForPostNumber(closest), - progress = postStream.progressIndexOfPost(closestPost), - progressController = container.lookup('controller:topic-progress'); + + const closestPost = postStream.closestPostForPostNumber(closest); + const progress = postStream.progressIndexOfPost(closestPost); + const progressController = container.lookup('controller:topic-progress'); progressController.set('progressPosition', progress); - self.appEvents.trigger('post:highlight', closest); - }).then(function() { + this.appEvents.trigger('post:highlight', closest); + }).then(() => { DiscourseURL.jumpToPost(closest, {skipIfOnScreen: true}); }); diff --git a/app/assets/javascripts/discourse/models/post-stream.js.es6 b/app/assets/javascripts/discourse/models/post-stream.js.es6 index 5178b55ffc3..8ee3893b1fc 100644 --- a/app/assets/javascripts/discourse/models/post-stream.js.es6 +++ b/app/assets/javascripts/discourse/models/post-stream.js.es6 @@ -1,6 +1,7 @@ import DiscourseURL from 'discourse/lib/url'; import RestModel from 'discourse/models/rest'; import { default as computed } from 'ember-addons/ember-computed-decorators'; +import { Placeholder } from 'discourse/views/cloaked'; function calcDayDiff(p1, p2) { if (!p1) { return; } @@ -17,7 +18,119 @@ function calcDayDiff(p1, p2) { } } -const PostStream = RestModel.extend({ +export function loadTopicView(topic, args) { + const topicId = topic.get('id'); + const data = _.merge({}, args); + const url = Discourse.getURL("/t/") + topicId; + const jsonUrl = (data.nearPost ? `${url}/${data.nearPost}` : url) + '.json'; + + delete data.nearPost; + delete data.__type; + delete data.store; + + return PreloadStore.getAndRemove(`topic_${topicId}`, () => { + return Discourse.ajax(jsonUrl, {data}); + }).then(json => { + topic.updateFromJson(json); + return json; + }); +} + +const PostsWithPlaceholders = Ember.Object.extend(Ember.Array, { + posts: null, + _appendingIds: null, + + init() { + this._appendingIds = {}; + }, + + @computed + length() { + return this.get('posts.length') + Object.keys(this._appendingIds || {}).length; + }, + + append(cb) { + const l = this.get('posts.length'); + this.arrayContentWillChange(l, 0, 1); + cb(); + this.arrayContentDidChange(l, 0, 1); + this.propertyDidChange('length'); + }, + + removePost(cb) { + const l = this.get('posts.length') - 1; + this.arrayContentWillChange(l, 1, 0); + cb(); + this.arrayContentDidChange(l, 1, 0); + this.propertyDidChange('length'); + }, + + appending(postIds) { + console.log('appending'); + const l = this.get('length'); + this.arrayContentWillChange(l, 0, postIds.length); + const appendingIds = this._appendingIds; + postIds.forEach(pid => appendingIds[pid] = true); + this.arrayContentDidChange(l, 0, postIds.length); + this.propertyDidChange('length'); + }, + + finishedAppending(postIds) { + const l = this.get('posts.length') - postIds.length; + this.arrayContentWillChange(l, postIds.length, postIds.length); + const appendingIds = this._appendingIds; + postIds.forEach(pid => delete appendingIds[pid]); + this.arrayContentDidChange(l, postIds.length, postIds.length); + this.propertyDidChange('length'); + }, + + finishedPrepending(postIds) { + this.arrayContentDidChange(0, 0, postIds.length); + this.propertyDidChange('length'); + }, + + objectAt(index) { + const posts = this.get('posts'); + if (index < posts.length) { + return posts[index]; + } else { + return new Placeholder('post-placeholder'); + } + }, +}); + +export default RestModel.extend({ + _identityMap: null, + posts: null, + stream: null, + userFilters: null, + summary: null, + loaded: null, + loadingAbove: null, + loadingBelow: null, + loadingFilter: null, + stagingPost: null, + postsWithPlaceholders: null, + + init() { + this._identityMap = {}; + const posts = []; + const postsWithPlaceholders = PostsWithPlaceholders.create({ posts, store: this.store }); + + this.setProperties({ + posts, + postsWithPlaceholders, + stream: [], + userFilters: [], + summary: false, + loaded: false, + loadingAbove: false, + loadingBelow: false, + loadingFilter: false, + stagingPost: false, + }); + }, + loading: Ember.computed.or('loadingAbove', 'loadingBelow', 'loadingFilter', 'stagingPost'), notLoading: Ember.computed.not('loading'), filteredPostsCount: Ember.computed.alias("stream.length"), @@ -27,7 +140,11 @@ const PostStream = RestModel.extend({ return this.get('posts.length') > 0; }, - hasStream: Ember.computed.gt('filteredPostsCount', 0), + @computed('hasPosts', 'filteredPostsCount') + hasLoadedData(hasPosts, filteredPostsCount) { + return hasPosts && filteredPostsCount > 0; + }, + canAppendMore: Ember.computed.and('notLoading', 'hasPosts', 'lastPostNotLoaded'), canPrependMore: Ember.computed.and('notLoading', 'hasPosts', 'firstPostNotLoaded'), @@ -38,26 +155,8 @@ const PostStream = RestModel.extend({ }, firstPostNotLoaded: Ember.computed.not('firstPostPresent'), - - @computed('posts.@each') - firstLoadedPost() { - return _.first(this.get('posts')); - }, - - @computed('posts.@each') - lastLoadedPost() { - return _.last(this.get('posts')); - }, - - @computed('stream.@each') - firstPostId() { - return this.get('stream')[0]; - }, - - @computed('stream.@each') - lastPostId() { - return _.last(this.get('stream')); - }, + firstPostId: Ember.computed.alias('stream.firstObject'), + lastPostId: Ember.computed.alias('stream.lastObject'), @computed('hasLoadedData', 'lastPostId', 'posts.@each.id') loadedAllPosts(hasLoadedData, lastPostId) { @@ -117,7 +216,7 @@ const PostStream = RestModel.extend({ Returns the window of posts below the current set in the stream, bound by the bottom of the stream. This is the collection we use when scrolling downwards. **/ - @computed('lastLoadedPost', 'stream.@each') + @computed('posts.lastObject', 'stream.@each') nextWindow(lastLoadedPost) { // If we can't find the last post loaded, bail if (!lastLoadedPost) { return []; } @@ -206,8 +305,7 @@ const PostStream = RestModel.extend({ opts = _.merge(opts, this.get('streamFilters')); // Request a topicView - return PostStream.loadTopicView(topic.get('id'), opts).then(json => { - topic.updateFromJson(json); + return loadTopicView(topic, opts).then(json => { this.updateFromJson(json.post_stream); this.setProperties({ loadingFilter: false, loaded: true }); }).catch(result => { @@ -215,7 +313,6 @@ const PostStream = RestModel.extend({ throw result; }); }, - hasLoadedData: Ember.computed.and('hasPosts', 'hasStream'), collapsePosts(from, to){ const posts = this.get('posts'); @@ -237,7 +334,6 @@ const PostStream = RestModel.extend({ this.get('stream').enumerableContentDidChange(); }, - // Fill in a gap of posts before a particular post fillGapBefore(post, gap) { const postId = post.get('id'), @@ -293,12 +389,15 @@ const PostStream = RestModel.extend({ this.set('loadingBelow', true); - const stopLoading = () => this.set('loadingBelow', false); - - return this.findPostsByIds(postIds).then((posts) => { + const postsWithPlaceholders = this.get('postsWithPlaceholders'); + postsWithPlaceholders.appending(postIds); + return this.findPostsByIds(postIds).then(posts => { posts.forEach(p => this.appendPost(p)); - stopLoading(); - }, stopLoading); + return posts; + }).finally(() => { + postsWithPlaceholders.finishedAppending(postIds); + this.set('loadingBelow', false); + }); }, // Prepend the previous window of posts to the stream. Call it when scrolling upwards. @@ -312,6 +411,9 @@ const PostStream = RestModel.extend({ this.set('loadingAbove', true); return this.findPostsByIds(postIds.reverse()).then(posts => { posts.forEach(p => this.prependPost(p)); + }).finally(() => { + const postsWithPlaceholders = this.get('postsWithPlaceholders'); + postsWithPlaceholders.finishedPrepending(postIds); this.set('loadingAbove', false); }); }, @@ -363,8 +465,7 @@ const PostStream = RestModel.extend({ } this.get('stream').removeObject(-1); - this.get('postIdentityMap').set(-1, null); - + this._identityMap[-1] = null; this.set('stagingPost', false); }, @@ -374,8 +475,8 @@ const PostStream = RestModel.extend({ **/ undoPost(post) { this.get('stream').removeObject(-1); - this.posts.removeObject(post); - this.get('postIdentityMap').set(-1, null); + this.get('postsWithPlaceholders').removePost(() => this.posts.removeObject(post)); + this._identityMap[-1] = null; const topic = this.get('topic'); this.set('stagingPost', false); @@ -405,7 +506,13 @@ const PostStream = RestModel.extend({ const posts = this.get('posts'); calcDayDiff(stored, this.get('lastAppended')); - posts.addObject(stored); + if (!posts.contains(stored)) { + if (!this.get('loadingBelow')) { + this.get('postsWithPlaceholders').append(() => posts.pushObject(stored)); + } else { + posts.pushObject(stored); + } + } if (stored.get('id') !== -1) { this.set('lastAppended', stored); @@ -418,16 +525,16 @@ const PostStream = RestModel.extend({ if (Ember.isEmpty(posts)) { return; } const postIds = posts.map(p => p.get('id')); - const identityMap = this.get('postIdentityMap'); + const identityMap = this._identityMap; this.get('stream').removeObjects(postIds); this.get('posts').removeObjects(posts); - postIds.forEach(id => identityMap.delete(id)); + postIds.forEach(id => delete identityMap[id]); }, // Returns a post from the identity map if it's been inserted. findLoadedPost(id) { - return this.get('postIdentityMap').get(id); + return this._identityMap[id]; }, loadPost(postId){ @@ -454,16 +561,13 @@ const PostStream = RestModel.extend({ this.get('stream').addObject(postId); if (loadedAllPosts) { this.set('loadingLastPost', true); - this.appendMore().finally( - ()=>this.set('loadingLastPost', true) - ); + this.appendMore().finally(()=> this.set('loadingLastPost', true)); } } }, triggerRecoveredPost(postId) { - const postIdentityMap = this.get('postIdentityMap'); - const existing = postIdentityMap.get(postId); + const existing = this._identityMap[postId]; if (existing) { this.triggerChangedPost(postId, new Date()); @@ -506,8 +610,7 @@ const PostStream = RestModel.extend({ }, triggerDeletedPost(postId){ - const postIdentityMap = this.get('postIdentityMap'); - const existing = postIdentityMap.get(postId); + const existing = this._identityMap[postId]; if (existing) { const url = "/posts/" + postId; @@ -524,8 +627,7 @@ const PostStream = RestModel.extend({ triggerChangedPost(postId, updatedAt) { if (!postId) { return; } - const postIdentityMap = this.get('postIdentityMap'); - const existing = postIdentityMap.get(postId); + const existing = this._identityMap[postId]; if (existing && existing.updated_at !== updatedAt) { const url = "/posts/" + postId; const store = this.store; @@ -625,19 +727,18 @@ const PostStream = RestModel.extend({ }, updateFromJson(postStreamData) { - const postStream = this, - posts = this.get('posts'); + const posts = this.get('posts'); posts.clear(); this.set('gaps', null); if (postStreamData) { // Load posts if present const store = this.store; - postStreamData.posts.forEach(p => postStream.appendPost(store.createRecord('post', p))); + postStreamData.posts.forEach(p => this.appendPost(store.createRecord('post', p))); delete postStreamData.posts; // Update our attributes - postStream.setProperties(postStreamData); + this.setProperties(postStreamData); } }, @@ -647,13 +748,12 @@ const PostStream = RestModel.extend({ than you supplied if the post has already been loaded. **/ storePost(post) { - // Calling `Ember.get(undefined` raises an error + // Calling `Ember.get(undefined)` raises an error if (!post) { return; } const postId = Ember.get(post, 'id'); if (postId) { - const postIdentityMap = this.get('postIdentityMap'), - existing = postIdentityMap.get(post.get('id')); + const existing = this._identityMap[post.get('id')]; // Update the `highest_post_number` if this post is higher. const postNumber = post.get('post_number'); @@ -668,31 +768,18 @@ const PostStream = RestModel.extend({ } post.set('topic', this.get('topic')); - postIdentityMap.set(post.get('id'), post); + this._identityMap[post.get('id')] = post; } return post; }, - /** - Given a list of postIds, returns a list of the posts we don't have in our - identity map and need to load. - **/ - listUnloadedIds(postIds) { - const unloaded = []; - const postIdentityMap = this.get('postIdentityMap'); - postIds.forEach(p => { - if (!postIdentityMap.has(p)) { unloaded.pushObject(p); } - }); - return unloaded; - }, - findPostsByIds(postIds) { - const unloaded = this.listUnloadedIds(postIds); - const postIdentityMap = this.get('postIdentityMap'); + const identityMap = this._identityMap; + const unloaded = postIds.filter(p => !identityMap[p]); // Load our unloaded posts by id return this.loadIntoIdentityMap(unloaded).then(() => { - return postIds.map(p => postIdentityMap.get(p)).compact(); + return postIds.map(p => identityMap[p]).compact(); }); }, @@ -747,40 +834,3 @@ const PostStream = RestModel.extend({ } }); - - -PostStream.reopenClass({ - create() { - const postStream = this._super.apply(this, arguments); - postStream.setProperties({ - posts: [], - stream: [], - userFilters: [], - postIdentityMap: Ember.Map.create(), - summary: false, - loaded: false, - loadingAbove: false, - loadingBelow: false, - loadingFilter: false, - stagingPost: false - }); - return postStream; - }, - - loadTopicView(topicId, args) { - const opts = _.merge({}, args); - let url = Discourse.getURL("/t/") + topicId; - if (opts.nearPost) { - url += "/" + opts.nearPost; - } - delete opts.nearPost; - delete opts.__type; - delete opts.store; - - return PreloadStore.getAndRemove("topic_" + topicId, () => { - return Discourse.ajax(url + ".json", {data: opts}); - }); - } -}); - -export default PostStream; diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6 index e3e651aa8d7..889349f1db1 100644 --- a/app/assets/javascripts/discourse/models/topic.js.es6 +++ b/app/assets/javascripts/discourse/models/topic.js.es6 @@ -319,11 +319,7 @@ const Topic = RestModel.extend({ keys.removeObject('details'); keys.removeObject('post_stream'); - const topic = this; - keys.forEach(function (key) { - topic.set(key, json[key]); - }); - + keys.forEach(key => this.set(key, json[key])); }, isPinnedUncategorized: function() { diff --git a/app/assets/javascripts/discourse/routes/topic-unsubscribe.js.es6 b/app/assets/javascripts/discourse/routes/topic-unsubscribe.js.es6 index 10e77c47b48..26e476af233 100644 --- a/app/assets/javascripts/discourse/routes/topic-unsubscribe.js.es6 +++ b/app/assets/javascripts/discourse/routes/topic-unsubscribe.js.es6 @@ -1,12 +1,9 @@ -import PostStream from "discourse/models/post-stream"; +import { loadTopicView } from "discourse/models/post-stream"; export default Discourse.Route.extend({ model(params) { const topic = this.store.createRecord("topic", { id: params.id }); - return PostStream.loadTopicView(params.id).then(json => { - topic.updateFromJson(json); - return topic; - }); + return loadTopicView(topic).then(() => topic); }, afterModel(topic) { diff --git a/app/assets/javascripts/discourse/templates/post-placeholder.hbs b/app/assets/javascripts/discourse/templates/post-placeholder.hbs new file mode 100644 index 00000000000..936863f191e --- /dev/null +++ b/app/assets/javascripts/discourse/templates/post-placeholder.hbs @@ -0,0 +1,13 @@ +
+
+
+
+
+ +
+
+
+
+
+
+
diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index f26ff34e2a0..6430a4ec5e4 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -68,9 +68,8 @@ {{#unless model.postStream.loadingFilter}} {{cloaked-collection itemViewClass="post" - idProperty="post_number" defaultHeight="200" - content=model.postStream.posts + content=model.postStream.postsWithPlaceholders slackRatio="15" loadingHTML="" preservesContext="true" @@ -78,8 +77,6 @@ offsetFixedTop="header" offsetFixedBottom="#reply-control"}} {{/unless}} - - {{conditional-loading-spinner condition=model.postStream.loadingBelow}}
diff --git a/app/assets/javascripts/discourse/views/cloaked-collection.js.es6 b/app/assets/javascripts/discourse/views/cloaked-collection.js.es6 index b15e5c60a64..7e5bb25e418 100644 --- a/app/assets/javascripts/discourse/views/cloaked-collection.js.es6 +++ b/app/assets/javascripts/discourse/views/cloaked-collection.js.es6 @@ -27,7 +27,6 @@ const CloakedCollectionView = Ember.CollectionView.extend({ init() { this._super(); - if (idProperty) { this.set('elementId', cloakView + '-cloak-' + this.get('content.' + idProperty)); } @@ -124,8 +123,9 @@ const CloakedCollectionView = Ember.CollectionView.extend({ const viewportTop = windowTop - slack, topView = this.findTopView(childViews, viewportTop, 0, childViews.length-1); - let windowBottom = windowTop + windowHeight, - viewportBottom = windowBottom + slack; + let windowBottom = windowTop + windowHeight; + let viewportBottom = windowBottom + slack; + if (windowBottom > bodyHeight) { windowBottom = bodyHeight; } if (viewportBottom > bodyHeight) { viewportBottom = bodyHeight; } @@ -139,22 +139,28 @@ const CloakedCollectionView = Ember.CollectionView.extend({ // Find the bottom view and what's onscreen let bottomView = topView; + let bottomVisible = null; while (bottomView < childViews.length) { - const view = childViews[bottomView], - $view = view.$(); + const view = childViews[bottomView]; + const $view = view.$(); if (!$view) { break; } // in case of not full-window scrolling - const scrollOffset = this.get('wrapperTop') || 0, - viewTop = $view.offset().top + scrollOffset, - viewBottom = viewTop + $view.height(); + const scrollOffset = this.get('wrapperTop') || 0; + const viewTop = $view.offset().top + scrollOffset; + const viewBottom = viewTop + $view.height(); if (viewTop > viewportBottom) { break; } toUncloak.push(view); if (viewBottom > windowTop && viewTop <= windowBottom) { - onscreen.push(view.get('content')); + const content = view.get('content'); + onscreen.push(content); + + if (!view.get('isPlaceholder')) { + bottomVisible = content; + } onscreenCloaks.push(view); } @@ -165,7 +171,7 @@ const CloakedCollectionView = Ember.CollectionView.extend({ // If our controller has a `sawObjects` method, pass the on screen objects to it. const controller = this.get('controller'); if (onscreen.length) { - this.setProperties({topVisible: onscreen[0], bottomVisible: onscreen[onscreen.length-1]}); + this.setProperties({topVisible: onscreen[0], bottomVisible }); if (controller && controller.sawObjects) { Em.run.schedule('afterRender', function() { controller.sawObjects(onscreen); diff --git a/app/assets/javascripts/discourse/views/cloaked.js.es6 b/app/assets/javascripts/discourse/views/cloaked.js.es6 index ddb3a00ae28..76cc16baa2f 100644 --- a/app/assets/javascripts/discourse/views/cloaked.js.es6 +++ b/app/assets/javascripts/discourse/views/cloaked.js.es6 @@ -1,9 +1,14 @@ +export function Placeholder(viewName) { + this.viewName = viewName; +} + export default Ember.View.extend({ attributeBindings: ['style'], _containedView: null, _scheduled: null, + isPlaceholder: null, - init: function() { + init() { this._super(); this._scheduled = false; this._childViews = []; @@ -15,6 +20,8 @@ export default Ember.View.extend({ this._childViews[0] = cv; } + this.set('isPlaceholder', cv && (cv.get('content') instanceof Placeholder)); + if (cv) { cv.set('_parentView', this); cv.set('templateData', this.get('templateData')); @@ -56,8 +63,8 @@ export default Ember.View.extend({ if (state !== 'inDOM' && state !== 'preRender') { return; } if (!this._containedView) { - const model = this.get('content'), - container = this.get('container'); + const model = this.get('content'); + const container = this.get('container'); let controller; @@ -80,8 +87,8 @@ export default Ember.View.extend({ controller = factory.create({ model, parentController, target: parentController }); } - const createArgs = {}, - target = controller || model; + const createArgs = {}; + const target = controller || model; if (this.get('preservesContext')) { createArgs.content = target; @@ -89,12 +96,10 @@ export default Ember.View.extend({ createArgs.context = target; } if (controller) { createArgs.controller = controller; } - this.setProperties({ - style: null, - loading: false - }); + this.setProperties({ style: null, loading: false }); - this.setContainedView(this.createChildView(this.get('cloaks'), createArgs)); + const cloaks = target && (target instanceof Placeholder) ? target.viewName : this.get('cloaks'); + this.setContainedView(this.createChildView(cloaks, createArgs)); } }, @@ -107,7 +112,7 @@ export default Ember.View.extend({ const self = this; if (this._containedView && (this._state || this.state) === 'inDOM') { - const style = 'height: ' + this.$().height() + 'px;'; + const style = `height: ${this.$().height()}px;`.htmlSafe(); this.set('style', style); this.$().prop('style', style); diff --git a/app/assets/javascripts/discourse/views/post-placeholder.js.es6 b/app/assets/javascripts/discourse/views/post-placeholder.js.es6 new file mode 100644 index 00000000000..d73c6ce7546 --- /dev/null +++ b/app/assets/javascripts/discourse/views/post-placeholder.js.es6 @@ -0,0 +1 @@ +export default Ember.View.extend({ templateName: 'post-placeholder' }); diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 02cacad3280..d50548fe411 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -1,3 +1,19 @@ +.placeholder-avatar { + display: inline-block; + background-color: dark-light-diff($primary, $secondary, 90%, -75%); + width: 45px; + height: 45px; + border-radius: 50%; +} + +.placeholder-text { + display: inline-block; + background-color: dark-light-diff($primary, $secondary, 90%, -75%); + width: 100%; + height: 1.5em; + margin-bottom: 0.6em; +} + .names { float: left; .username { diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index 9c00a3b482f..7d34cbe874c 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -685,6 +685,7 @@ blockquote { $topic-body-width: 690px; $topic-body-width-padding: 11px; $topic-avatar-width: 45px; + .topic-body { width: $topic-body-width; float: left; diff --git a/test/javascripts/models/post-stream-test.js.es6 b/test/javascripts/models/post-stream-test.js.es6 index 13ae84a961b..dde21e2d2bd 100644 --- a/test/javascripts/models/post-stream-test.js.es6 +++ b/test/javascripts/models/post-stream-test.js.es6 @@ -65,6 +65,7 @@ test('appending posts', function() { const postStream = buildStream(4567, [1, 3, 4]); const store = postStream.store; + equal(postStream.get('firstPostId'), 1); equal(postStream.get('lastPostId'), 4, "the last post id is 4"); ok(!postStream.get('hasPosts'), "there are no posts by default"); @@ -283,23 +284,26 @@ test("storePost", function() { const postWithoutId = store.createRecord('post', {raw: 'hello world'}); stored = postStream.storePost(postWithoutId); equal(stored, postWithoutId, "it returns the same post back"); - equal(postStream.get('postIdentityMap.size'), 1, "it does not add a new entry into the identity map"); }); test("identity map", function() { - const postStream = buildStream(1234), - store = postStream.store; + const postStream = buildStream(1234); + const store = postStream.store; const p1 = postStream.appendPost(store.createRecord('post', {id: 1, post_number: 1})); - postStream.appendPost(store.createRecord('post', {id: 3, post_number: 4})); + const p3 = postStream.appendPost(store.createRecord('post', {id: 3, post_number: 4})); equal(postStream.findLoadedPost(1), p1, "it can return cached posts by id"); blank(postStream.findLoadedPost(4), "it can't find uncached posts"); - deepEqual(postStream.listUnloadedIds([10, 11, 12]), [10, 11, 12], "it returns a list of all unloaded ids"); - blank(postStream.listUnloadedIds([1, 3]), "if we have loaded all posts it's blank"); - deepEqual(postStream.listUnloadedIds([1, 2, 3, 4]), [2, 4], "it only returns unloaded posts"); + // Find posts by ids uses the identity map + postStream.findPostsByIds([1, 2, 3]).then(result => { + equal(result.length, 3); + equal(result.objectAt(0), p1); + equal(result.objectAt(1).get('post_number'), 2); + equal(result.objectAt(2), p3); + }); }); test("loadIntoIdentityMap with no data", () => { @@ -439,7 +443,6 @@ test('triggerNewPostInStream', function() { ok(postStream.appendMore.calledOnce, "delegates to appendMore because the last post is loaded"); }); - test("loadedAllPosts when the id changes", function() { // This can happen in a race condition between staging a post and it coming through on the // message bus. If the id of a post changes we should reconsider the loadedAllPosts property. @@ -475,3 +478,45 @@ test("comitting and triggerNewPostInStream race condition", function() { equal(postStream.get('filteredPostsCount'), 1, "it does not add the same post twice"); }); +test("postsWithPlaceholders", () => { + const postStream = buildStream(4964, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + const postsWithPlaceholders = postStream.get('postsWithPlaceholders'); + const store = postStream.store; + + const testProxy = Ember.ArrayProxy.create({ content: postsWithPlaceholders }); + + const p1 = store.createRecord('post', {id: 1, post_number: 1}); + const p2 = store.createRecord('post', {id: 2, post_number: 2}); + const p3 = store.createRecord('post', {id: 3, post_number: 3}); + const p4 = store.createRecord('post', {id: 4, post_number: 4}); + + postStream.appendPost(p1); + postStream.appendPost(p2); + postStream.appendPost(p3); + + // Test enumerable and array access + equal(postsWithPlaceholders.get('length'), 3); + equal(testProxy.get('length'), 3); + equal(postsWithPlaceholders.nextObject(0), p1); + equal(postsWithPlaceholders.objectAt(0), p1); + equal(postsWithPlaceholders.nextObject(1, p1), p2); + equal(postsWithPlaceholders.objectAt(1), p2); + equal(postsWithPlaceholders.nextObject(2, p2), p3); + equal(postsWithPlaceholders.objectAt(2), p3); + + const promise = postStream.appendMore(); + equal(postsWithPlaceholders.get('length'), 8, 'we immediately have a larger placeholder window'); + equal(testProxy.get('length'), 8); + ok(!!postsWithPlaceholders.nextObject(3, p3)); + ok(!!postsWithPlaceholders.objectAt(4)); + ok(postsWithPlaceholders.objectAt(3) !== p4); + ok(testProxy.objectAt(3) !== p4); + + return promise.then(() => { + equal(postsWithPlaceholders.objectAt(3), p4); + equal(postsWithPlaceholders.get('length'), 8, 'have a larger placeholder window when loaded'); + equal(testProxy.get('length'), 8); + equal(testProxy.objectAt(3), p4); + }); + +});