Improve post stream

- Return all discussion post IDs from API requests which add/remove
posts, so the post stream updates appropriately. Related to #146
- Always unload posts that are two pages away, no matter how fast
you’re scrolling
- Retrieve posts from cache instead of reloading them
- Fix various bugs. Maybe #152, needs confirmation
This commit is contained in:
Toby Zerner 2015-07-06 16:26:27 +09:30
parent a70fff881d
commit eee5133d6e
11 changed files with 93 additions and 128 deletions

View File

@ -49,7 +49,7 @@ class PostStream extends mixin(Component, evented) {
m.redraw(true); m.redraw(true);
return promise.then(() => { return promise.then(() => {
anchorScroll(this.$('.item:'+(backwards ? 'last' : 'first')), () => m.redraw(true)); anchorScroll(this.$('.item:' + (backwards ? 'last' : 'first')), () => m.redraw(true));
this.scrollToIndex(index, noAnimation, backwards).done(this.unpause.bind(this)); this.scrollToIndex(index, noAnimation, backwards).done(this.unpause.bind(this));
}); });
@ -69,43 +69,15 @@ class PostStream extends mixin(Component, evented) {
return this.goToIndex(this.count() - 1, true); return this.goToIndex(this.count() - 1, true);
} }
/**
Update the stream to reflect any posts that have been added/removed from the
discussion.
*/
sync() {
var addedPosts = this.discussion.addedPosts();
if (addedPosts) addedPosts.forEach(this.pushPost.bind(this));
this.discussion.pushAttributes({links: {addedPosts: null}});
var removedPosts = this.discussion.removedPosts();
if (removedPosts) removedPosts.forEach(this.removePost.bind(this));
this.discussion.pushAttributes({removedPosts: null});
}
/** /**
Add a post to the end of the stream. Nothing will be done if the end of the Add a post to the end of the stream. Nothing will be done if the end of the
stream is not visible. stream is not visible.
*/ */
pushPost(post) {
if (this.visibleEnd >= this.count() - 1 && this.posts.indexOf(post) === -1) {
this.posts.push(post);
this.visibleEnd++; this.visibleEnd++;
} }
} }
/** /**
Search for and remove a specific post from the stream. Nothing will be done
if the post is not visible.
*/
removePost(id) {
this.posts.some((item, i) => {
if (item && item.id() == id) {
this.posts.splice(i, 1);
this.visibleEnd--;
return true;
}
});
} }
/** /**
@ -127,7 +99,6 @@ class PostStream extends mixin(Component, evented) {
Set up the stream with the given array of posts. Set up the stream with the given array of posts.
*/ */
setup(posts) { setup(posts) {
this.posts = posts;
this.visibleStart = posts.length ? this.discussion.postIds().indexOf(posts[0].id()) : 0; this.visibleStart = posts.length ? this.discussion.postIds().indexOf(posts[0].id()) : 0;
this.visibleEnd = this.visibleStart + posts.length; this.visibleEnd = this.visibleStart + posts.length;
} }
@ -137,11 +108,6 @@ class PostStream extends mixin(Component, evented) {
*/ */
clear(start, end) { clear(start, end) {
this.visibleStart = start || 0; this.visibleStart = start || 0;
this.visibleEnd = Math.min(this.count(), end || this.constructor.loadCount);
this.posts = [];
for (var i = this.visibleStart; i < this.visibleEnd; i++) {
this.posts.push(null);
}
} }
/** /**
@ -157,7 +123,6 @@ class PostStream extends mixin(Component, evented) {
var lastTime; var lastTime;
return m('div.discussion-posts.posts', {config: this.onload.bind(this)}, return m('div.discussion-posts.posts', {config: this.onload.bind(this)},
this.posts.map((post, i) => {
var content; var content;
var attributes = {}; var attributes = {};
attributes['data-index'] = this.visibleStart + i; attributes['data-index'] = this.visibleStart + i;
@ -172,7 +137,6 @@ class PostStream extends mixin(Component, evented) {
attributes['data-number'] = post.number(); attributes['data-number'] = post.number();
var dt = post.time() - lastTime; var dt = post.time() - lastTime;
if (dt > 1000 * 60 * 60 * 24 * 4) {
content = [ content = [
m('div.time-gap', m('span', moment.duration(dt).humanize(), ' later')), m('div.time-gap', m('span', moment.duration(dt).humanize(), ' later')),
content content
@ -193,7 +157,6 @@ class PostStream extends mixin(Component, evented) {
this.visibleEnd === this.count() && this.visibleEnd === this.count() &&
(!app.session.user() || this.discussion.canReply()) && (!app.session.user() || this.discussion.canReply()) &&
!app.composingReplyTo(this.discussion) !app.composingReplyTo(this.discussion)
? m('div.item', ReplyPlaceholder.component({discussion: this.discussion}))
: '' : ''
); );
} }
@ -233,7 +196,6 @@ class PostStream extends mixin(Component, evented) {
var marginTop = this.getMarginTop(); var marginTop = this.getMarginTop();
var viewportHeight = $(window).height() - marginTop; var viewportHeight = $(window).height() - marginTop;
var viewportTop = top + marginTop; var viewportTop = top + marginTop;
var loadAheadDistance = viewportHeight;
if (this.visibleStart > 0) { if (this.visibleStart > 0) {
var $item = this.$('.item[data-index='+this.visibleStart+']'); var $item = this.$('.item[data-index='+this.visibleStart+']');
@ -262,17 +224,7 @@ class PostStream extends mixin(Component, evented) {
var start = this.visibleEnd; var start = this.visibleEnd;
var end = this.visibleEnd = this.sanitizeIndex(this.visibleEnd + this.constructor.loadCount); var end = this.visibleEnd = this.sanitizeIndex(this.visibleEnd + this.constructor.loadCount);
for (var i = start; i < end; i++) {
this.posts.push(null);
}
// If the posts which are two pages back from the page we're currently
// loading still haven't loaded, we can assume that the user is scrolling
// pretty fast. Thus, we will unload them.
var twoPagesAway = start - this.constructor.loadCount * 2; var twoPagesAway = start - this.constructor.loadCount * 2;
if (twoPagesAway >= 0 && !this.posts[twoPagesAway - this.visibleStart]) {
this.posts.splice(0, twoPagesAway + this.constructor.loadCount - this.visibleStart);
this.visibleStart = twoPagesAway + this.constructor.loadCount;
clearTimeout(this.loadPageTimeouts[twoPagesAway]); clearTimeout(this.loadPageTimeouts[twoPagesAway]);
} }
@ -286,16 +238,7 @@ class PostStream extends mixin(Component, evented) {
var end = this.visibleStart; var end = this.visibleStart;
var start = this.visibleStart = this.sanitizeIndex(this.visibleStart - this.constructor.loadCount); var start = this.visibleStart = this.sanitizeIndex(this.visibleStart - this.constructor.loadCount);
for (var i = start; i < end; i++) {
this.posts.unshift(null);
}
// If the posts which are two pages back from the page we're currently
// loading still haven't loaded, we can assume that the user is scrolling
// pretty fast. Thus, we will unload them.
var twoPagesAway = start + this.constructor.loadCount * 2; var twoPagesAway = start + this.constructor.loadCount * 2;
if (twoPagesAway <= this.count() && !this.posts[twoPagesAway - this.visibleStart]) {
this.posts.splice(twoPagesAway - this.visibleStart);
this.visibleEnd = twoPagesAway; this.visibleEnd = twoPagesAway;
clearTimeout(this.loadPageTimeouts[twoPagesAway]); clearTimeout(this.loadPageTimeouts[twoPagesAway]);
} }
@ -310,8 +253,6 @@ class PostStream extends mixin(Component, evented) {
var redraw = () => { var redraw = () => {
if (start < this.visibleStart || end > this.visibleEnd) return; if (start < this.visibleStart || end > this.visibleEnd) return;
var anchorIndex = backwards && $(window).scrollTop() > 0 ? this.visibleEnd - 1 : this.visibleStart;
anchorScroll(this.$('.item[data-index='+anchorIndex+']'), () => m.redraw(true));
this.unpause(); this.unpause();
}; };
@ -332,10 +273,7 @@ class PostStream extends mixin(Component, evented) {
clearing it. clearing it.
*/ */
loadRange(start, end) { loadRange(start, end) {
return app.store.find('posts', this.discussion.postIds().slice(start, end)).then(posts => {
if (start < this.visibleStart || end > this.visibleEnd) return;
this.posts.splice.apply(this.posts, [start - this.visibleStart, end - start].concat(posts));
}); });
} }
@ -345,7 +283,6 @@ class PostStream extends mixin(Component, evented) {
resolved immediately. resolved immediately.
*/ */
loadNearNumber(number) { loadNearNumber(number) {
if (this.posts.some(post => post.number() == number)) {
return m.deferred().resolve().promise; return m.deferred().resolve().promise;
} }
@ -372,9 +309,6 @@ class PostStream extends mixin(Component, evented) {
this.clear(start, end); this.clear(start, end);
var ids = this.discussion.postIds().slice(start, end);
return app.store.find('posts', ids).then(this.setup.bind(this));
} }
/** /**
@ -419,7 +353,6 @@ class PostStream extends mixin(Component, evented) {
would consider a post to be the first one visible. would consider a post to be the first one visible.
*/ */
getMarginTop() { getMarginTop() {
return this.$() && $('.global-header').outerHeight() + parseInt(this.$().css('margin-top'));
} }
/** /**

View File

@ -2,7 +2,6 @@ import ItemList from 'flarum/utils/item-list';
import ComposerBody from 'flarum/components/composer-body'; import ComposerBody from 'flarum/components/composer-body';
import Alert from 'flarum/components/alert'; import Alert from 'flarum/components/alert';
import ActionButton from 'flarum/components/action-button'; import ActionButton from 'flarum/components/action-button';
import Composer from 'flarum/components/composer';
import icon from 'flarum/helpers/icon'; import icon from 'flarum/helpers/icon';
export default class ReplyComposer extends ComposerBody { export default class ReplyComposer extends ComposerBody {
@ -44,26 +43,11 @@ export default class ReplyComposer extends ComposerBody {
var data = this.data(); var data = this.data();
app.store.createRecord('posts').save(data).then((post) => { app.store.createRecord('posts').save(data).then(post => {
app.composer.hide();
discussion.pushAttributes({
relationships: {
lastUser: post.user(),
lastPost: post
},
lastTime: post.time(),
lastPostNumber: post.number(),
commentsCount: discussion.commentsCount() + 1,
readTime: post.time(),
readNumber: post.number()
});
discussion.data().relationships.posts.data.push({type: 'posts', id: post.id()});
// If we're currently viewing the discussion which this reply was made // If we're currently viewing the discussion which this reply was made
// in, then we can add the post to the end of the post stream. // in, then we can add the post to the end of the post stream.
if (app.current && app.current.discussion && app.current.discussion().id() === discussion.id()) { if (app.viewingDiscussion(discussion)) {
app.current.stream.pushPost(post); app.current.stream.update();
m.route(app.route('discussion.near', { m.route(app.route('discussion.near', {
id: discussion.id(), id: discussion.id(),
slug: discussion.slug(), slug: discussion.slug(),
@ -89,6 +73,8 @@ export default class ReplyComposer extends ComposerBody {
}) })
); );
} }
app.composer.hide();
}, errors => { }, errors => {
this.loading(false); this.loading(false);
m.redraw(); m.redraw();

View File

@ -55,20 +55,21 @@ export default function(app) {
app.history.back(); app.history.back();
} }
} }
} };
Discussion.prototype.renameAction = function() { Discussion.prototype.renameAction = function() {
var currentTitle = this.title(); const currentTitle = this.title();
var title = prompt('Enter a new title for this discussion:', currentTitle); const title = prompt('Enter a new title for this discussion:', currentTitle);
if (title && title !== currentTitle) { if (title && title !== currentTitle) {
this.save({title}).then(discussion => { this.save({title}).then(() => {
if (app.current instanceof DiscussionPage) { if (app.viewingDiscussion(this)) {
app.current.stream.sync(); app.current.stream.update();
} }
m.redraw(); m.redraw();
}); });
} }
} };
Discussion.prototype.userControls = function(context) { Discussion.prototype.userControls = function(context) {
var items = new ItemList(); var items = new ItemList();

View File

@ -23,10 +23,7 @@ export default function(app) {
function deleteAction() { function deleteAction() {
this.delete(); this.delete();
// this.discussion().pushAttributes({removedPosts: [this.id()]}); this.discussion().removePost(this.id());
if (app.current instanceof DiscussionPage) {
app.current.stream.removePost(this.id());
}
} }
Post.prototype.userControls = function(context) { Post.prototype.userControls = function(context) {

View File

@ -27,6 +27,20 @@ class Discussion extends Model {
// } // }
} }
removePost(id) {
const relationships = this.data().relationships;
const posts = relationships && relationships.posts;
if (posts) {
posts.data.some((data, i) => {
if (id === data.id) {
posts.data.splice(i, 1);
return true;
}
});
}
}
unreadCount() { unreadCount() {
var user = app.session.user(); var user = app.session.user();
if (user && user.readTime() < this.lastTime()) { if (user && user.readTime() < this.lastTime()) {

View File

@ -85,6 +85,17 @@ class UpdateAction extends SerializeResourceAction
$discussion = $state->discussion; $discussion = $state->discussion;
} }
if ($posts = $discussion->getModifiedPosts()) {
$discussion->posts_ids = $discussion->postsVisibleTo($actor)->orderBy('time')->lists('id');
$discussion->posts = array_filter($posts, function ($post) {
return $post->exists;
});
$request->include = array_merge($request->include, ['posts']);
$request->link = array_merge($request->include, ['posts', 'posts.discussion', 'posts.user']);
}
return $discussion; return $discussion;
} }
} }

View File

@ -22,13 +22,14 @@ class CreateAction extends BaseCreateAction
* @inheritdoc * @inheritdoc
*/ */
public static $include = [ public static $include = [
'user' => true 'user' => true,
'discussion' => true
]; ];
/** /**
* @inheritdoc * @inheritdoc
*/ */
public static $link = []; public static $link = ['discussion.posts'];
/** /**
* @inheritdoc * @inheritdoc
@ -84,6 +85,9 @@ class CreateAction extends BaseCreateAction
); );
} }
$discussion = $post->discussion;
$discussion->posts_ids = $discussion->postsVisibleTo($actor)->orderBy('time')->lists('id');
return $post; return $post;
} }
} }

View File

@ -46,6 +46,13 @@ class Discussion extends Model
'last_post_number' => 'integer' 'last_post_number' => 'integer'
]; ];
/**
* An array of posts that have been modified during this request.
*
* @var array
*/
protected $modifiedPosts = [];
/** /**
* The attributes that should be mutated to dates. * The attributes that should be mutated to dates.
* *
@ -140,7 +147,7 @@ class Discussion extends Model
/** /**
* Set the discussion's start post details. * Set the discussion's start post details.
* *
* @param \Flarum\Core\Posts\Post $post * @param Post $post
* @return $this * @return $this
*/ */
public function setStartPost(Post $post) public function setStartPost(Post $post)
@ -155,7 +162,7 @@ class Discussion extends Model
/** /**
* Set the discussion's last post details. * Set the discussion's last post details.
* *
* @param \Flarum\Core\Posts\Post $post * @param Post $post
* @return $this * @return $this
*/ */
public function setLastPost(Post $post) public function setLastPost(Post $post)
@ -214,16 +221,28 @@ class Discussion extends Model
* DiscussionRenamedPost, and delete if the title has been reverted * DiscussionRenamedPost, and delete if the title has been reverted
* completely.) * completely.)
* *
* @param \Flarum\Core\Posts\Post $post The post to save. * @param MergeablePost $post The post to save.
* @return \Flarum\Core\Posts\Post The resulting post. It may or may not be * @return Post The resulting post. It may or may not be the same post as
* the same post as was originally intended to be saved. It also may not * was originally intended to be saved. It also may not exist, if the
* exist, if the merge logic resulted in deletion. * merge logic resulted in deletion.
*/ */
public function mergePost(MergeablePost $post) public function mergePost(MergeablePost $post)
{ {
$lastPost = $this->posts()->latest('time')->first(); $lastPost = $this->posts()->latest('time')->first();
return $post->saveAfter($lastPost); $post = $post->saveAfter($lastPost);
return $this->modifiedPosts[] = $post;
}
/**
* Get the posts that have been modified during this request.
*
* @return array
*/
public function getModifiedPosts()
{
return $this->modifiedPosts;
} }
/** /**

View File

@ -129,26 +129,6 @@ class CommentPost extends Post
return $value; return $value;
} }
/**
* Define the relationship with the user who edited the post.
*
* @return \Illuminate\Database\Eloquent\Relations\BelongsTo
*/
public function editUser()
{
return $this->belongsTo('Flarum\Core\Users\User', 'edit_user_id');
}
/**
* Define the relationship with the user who hid the post.
*
* @return \Illuminate\Database\Eloquent\Relations\BelongsTo
*/
public function hideUser()
{
return $this->belongsTo('Flarum\Core\Users\User', 'hide_user_id');
}
/** /**
* Get text formatter instance. * Get text formatter instance.
* *

View File

@ -111,6 +111,26 @@ class Post extends Model
return $this->belongsTo('Flarum\Core\Users\User', 'user_id'); return $this->belongsTo('Flarum\Core\Users\User', 'user_id');
} }
/**
* Define the relationship with the user who edited the post.
*
* @return \Illuminate\Database\Eloquent\Relations\BelongsTo
*/
public function editUser()
{
return $this->belongsTo('Flarum\Core\Users\User', 'edit_user_id');
}
/**
* Define the relationship with the user who hid the post.
*
* @return \Illuminate\Database\Eloquent\Relations\BelongsTo
*/
public function hideUser()
{
return $this->belongsTo('Flarum\Core\Users\User', 'hide_user_id');
}
/** /**
* Get all posts, regardless of their type, by removing the * Get all posts, regardless of their type, by removing the
* `RegisteredTypesScope` global scope constraints applied on this model. * `RegisteredTypesScope` global scope constraints applied on this model.

View File

@ -8,7 +8,7 @@ class DiscussionAction extends IndexAction
{ {
$response = $this->apiClient->send(app('flarum.actor'), 'Flarum\Api\Actions\Discussions\ShowAction', [ $response = $this->apiClient->send(app('flarum.actor'), 'Flarum\Api\Actions\Discussions\ShowAction', [
'id' => $routeParams['id'], 'id' => $routeParams['id'],
'near' => $routeParams['near'] 'page.near' => $routeParams['near']
]); ]);
// TODO: return an object instead of an array? // TODO: return an object instead of an array?