Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Make RoomListStore aware of Room.timeline events #1756

Merged
merged 3 commits into from
Feb 20, 2018
Merged

Make RoomListStore aware of Room.timeline events #1756

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94a0a90
Make RoomListStore aware of Room.timeline events
lukebarnard1 Feb 16, 2018
3f6c155
Remove unused `room` parameter of MatrixActions.Room.timeline
lukebarnard1 Feb 16, 2018
32130fb
Don't regenerate RoomListStore state for notifs/scrollback/etc.
lukebarnard1 Feb 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/actions/MatrixActionCreators.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ function createRoomTagsAction(matrixClient, roomTagsEvent, room) {
return { action: 'MatrixActions.Room.tags', room };
}

function createRoomTimelineAction(matrixClient, timelineEvent, room, toStartOfTimeline, removed, data) {
return {
action: 'MatrixActions.Room.timeline',
event: timelineEvent,
isLiveEvent: data.liveEvent,
room,
};
}

function createRoomMembershipAction(matrixClient, membershipEvent, member, oldMembership) {
return { action: 'MatrixActions.RoomMember.membership', member };
}
Expand All @@ -87,6 +96,7 @@ export default {
this._addMatrixClientListener(matrixClient, 'sync', createSyncAction);
this._addMatrixClientListener(matrixClient, 'accountData', createAccountDataAction);
this._addMatrixClientListener(matrixClient, 'Room.tags', createRoomTagsAction);
this._addMatrixClientListener(matrixClient, 'Room.timeline', createRoomTimelineAction);
this._addMatrixClientListener(matrixClient, 'RoomMember.membership', createRoomMembershipAction);
},

Expand Down
9 changes: 0 additions & 9 deletions src/components/views/rooms/RoomList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ module.exports = React.createClass({

cli.on("Room", this.onRoom);
cli.on("deleteRoom", this.onDeleteRoom);
cli.on("Room.timeline", this.onRoomTimeline);
cli.on("Room.name", this.onRoomName);
cli.on("Room.receipt", this.onRoomReceipt);
cli.on("RoomState.events", this.onRoomStateEvents);
Expand Down Expand Up @@ -177,7 +176,6 @@ module.exports = React.createClass({
if (MatrixClientPeg.get()) {
MatrixClientPeg.get().removeListener("Room", this.onRoom);
MatrixClientPeg.get().removeListener("deleteRoom", this.onDeleteRoom);
MatrixClientPeg.get().removeListener("Room.timeline", this.onRoomTimeline);
MatrixClientPeg.get().removeListener("Room.name", this.onRoomName);
MatrixClientPeg.get().removeListener("Room.receipt", this.onRoomReceipt);
MatrixClientPeg.get().removeListener("RoomState.events", this.onRoomStateEvents);
Expand Down Expand Up @@ -236,13 +234,6 @@ module.exports = React.createClass({
this._updateStickyHeaders(true, scrollToPosition);
},

onRoomTimeline: function(ev, room, toStartOfTimeline, removed, data) {
if (toStartOfTimeline) return;
if (!room) return;
if (data.timeline.getTimelineSet() !== room.getUnfilteredTimelineSet()) return;
this._delayedRefreshRoomList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks seem to have disappeared, as well as the _delayedRefreshRoomList?

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 Feb 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So:

  • the _delayedRefreshRoomList is done when we get an update from RoomListStore;
  • the timeline set check I thought would be handle by isLiveEvent? Unless I've got that confused?
  • toStartOfTimeline this is handled by isLiveEvent, which includes !toStartOfTimeline.
  • !room nothing uses the room, so I'll remove it from the MatrixAction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, OK. I think this almost all true apart from the timeline set check, where live timeline !== unfiltered timeline set. There are multiple timeline sets, the first is the unfiltered one (the normal one that isn't a search result). Each timeline set has many timelines, one of which is live. liveEvent is telling you if it's to the live timeline in a timeline set, not if it's to the timeline set you're interested in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in theory we shouldn't get live events for search result or any other timeline set apart from the unfiltered one. We're not sure how true this is though. Investigation of exactly when Room.timeline is emitted will have to be done.

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 Feb 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemingly this isn't true, as @dbkr has said, each set (be it a room timeline, notification timeline, rooms timeline) has a live timeline, and the only way of us knowing which is the room's live timeline if by comparing references.

},

onRoomReceipt: function(receiptEvent, room) {
// because if we read a notification, it will affect notification count
// only bother updating if there's a receipt from us
Expand Down
43 changes: 29 additions & 14 deletions src/stores/RoomListStore.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ import Unread from '../Unread';
* the RoomList.
*/
class RoomListStore extends Store {

static _listOrders = {
"m.favourite": "manual",
"im.vector.fake.invite": "recent",
"im.vector.fake.recent": "recent",
"im.vector.fake.direct": "recent",
"m.lowpriority": "recent",
"im.vector.fake.archived": "recent",
};

constructor() {
super(dis);

Expand Down Expand Up @@ -68,6 +78,14 @@ class RoomListStore extends Store {
this._generateRoomLists();
}
break;
case 'MatrixActions.Room.timeline': {
if (!this._state.ready ||
!payload.isLiveEvent ||
!this._eventTriggersRecentReorder(payload.event)
) break;
this._generateRoomLists();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't _generateRoomLists have a ratelimiter on it? surely this is quite a heavy operation, and we shouldn't be doing it each and every time a live timeline event happens?

}
break;
case 'MatrixActions.accountData': {
if (payload.event_type !== 'm.direct') break;
this._generateRoomLists();
Expand Down Expand Up @@ -159,18 +177,9 @@ class RoomListStore extends Store {
}
});

const listOrders = {
"m.favourite": "manual",
"im.vector.fake.invite": "recent",
"im.vector.fake.recent": "recent",
"im.vector.fake.direct": "recent",
"m.lowpriority": "recent",
"im.vector.fake.archived": "recent",
};

Object.keys(lists).forEach((listKey) => {
let comparator;
switch (listOrders[listKey]) {
switch (RoomListStore._listOrders[listKey]) {
case "recent":
comparator = this._recentsComparator;
break;
Expand All @@ -188,13 +197,17 @@ class RoomListStore extends Store {
});
}

_eventTriggersRecentReorder(ev) {
return ev.getTs() && (
Unread.eventTriggersUnreadCount(ev) ||
ev.getSender() === this._matrixClient.credentials.userId
);
}

_tsOfNewestEvent(room) {
for (let i = room.timeline.length - 1; i >= 0; --i) {
const ev = room.timeline[i];
if (ev.getTs() &&
(Unread.eventTriggersUnreadCount(ev) ||
(ev.getSender() === this._matrixClient.credentials.userId))
) {
if (this._eventTriggersRecentReorder(ev)) {
return ev.getTs();
}
}
Expand All @@ -210,6 +223,8 @@ class RoomListStore extends Store {
}

_recentsComparator(roomA, roomB) {
// XXX: We could use a cache here and update it when we see new
// events that trigger a reorder
return this._tsOfNewestEvent(roomB) - this._tsOfNewestEvent(roomA);
}

Expand Down