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

Add a small indicator for when a new event is pinned #1486

Merged
merged 6 commits into from
Nov 7, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
19 changes: 19 additions & 0 deletions src/components/views/rooms/PinnedEventsPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,25 @@ module.exports = React.createClass({
this.setState({ loading: false, pinned });
});
}

this._updateReadState();
},

_updateReadState: function() {
const pinnedEvents = this.props.room.currentState.getStateEvents("m.room.pinned_events", "");
if (!pinnedEvents) return; // nothing to read

let lastReadEvent = null;
const readPinsEvent = this.props.room.getAccountData("im.vector.room.read_pins");
if (readPinsEvent) {
lastReadEvent = readPinsEvent.getContent().last_read_id;
}

if (lastReadEvent !== pinnedEvents.getId()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't make sure that the lastReadEvent is ahead in the DAG relative to pinnedEvents.getId(). This could lead to two clients (i.e. clients with potentially differing state event data) fighting over which pinned message is really the last one read.

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the best way to check for this? Just using .getTs()?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can't technically rely on the timestamp to compare events because it might not be accurate. If we want to do this with some guarantee of the event ID always "increasing" in the timeline, we'd need server-side support. This would look something like m.fully_read - the read marker.

As there's no generic API for setting a similar m.pinned_messages.fully_read or whatever, this would require server-side changes.

But if we assume that the TS on the events are accurate and comparable (which they probably are), then sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

m.fully_read doesn't seem to appear in the spec at all. Is it something that already exists or is that part of the server-side implementation request?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

(which is matrix-org/matrix-spec-proposals#910)

I'll take a look and give it a shot I suppose.

MatrixClientPeg.get().setRoomAccountData(this.props.room.roomId, "im.vector.room.read_pins", {
last_read_id: pinnedEvents.getId(),
});
}
},

_getPinnedTiles: function() {
Expand Down
36 changes: 35 additions & 1 deletion src/components/views/rooms/RoomHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ module.exports = React.createClass({
componentDidMount: function() {
const cli = MatrixClientPeg.get();
cli.on("RoomState.events", this._onRoomStateEvents);
cli.on("Room.accountData", this._onRoomAccountData);

// When a room name occurs, RoomState.events is fired *before*
// room.name is updated. So we have to listen to Room.name as well as
Expand All @@ -87,6 +88,7 @@ module.exports = React.createClass({
const cli = MatrixClientPeg.get();
if (cli) {
cli.removeListener("RoomState.events", this._onRoomStateEvents);
cli.removeListener("Room.accountData", this._onRoomAccountData);
}
},

Expand All @@ -99,6 +101,13 @@ module.exports = React.createClass({
this._rateLimitedUpdate();
},

_onRoomAccountData: function(event, room) {
if (!this.props.room || room.roomId !== this.props.room.roomId) return;
if (event.getType() !== "im.vector.room.read_pins") return;

this._rateLimitedUpdate();
},

_rateLimitedUpdate: new RateLimitedFunc(function() {
/* eslint-disable babel/no-invalid-this */
this.forceUpdate();
Expand Down Expand Up @@ -139,6 +148,25 @@ module.exports = React.createClass({
dis.dispatch({ action: 'show_right_panel' });
},

_hasUnreadPins: function() {
const currentPinEvent = this.props.room.currentState.getStateEvents("m.room.pinned_events", '');
if (!currentPinEvent) return false;
if (currentPinEvent.getContent().pinned && currentPinEvent.getContent().pinned.length <= 0) {
return false; // no pins == nothing to read
}

const readPinsEvent = this.props.room.getAccountData("im.vector.room.read_pins");
if (readPinsEvent) {
const lastReadEvent = readPinsEvent.getContent().last_read_id;
if (lastReadEvent) {
return currentPinEvent.getId() !== lastReadEvent;
}
}

// There's pins, and we haven't read any of them
return true;
},

/**
* After editing the settings, get the new name for the room
*
Expand Down Expand Up @@ -302,8 +330,14 @@ module.exports = React.createClass({
}

if (this.props.onPinnedClick && UserSettingsStore.isFeatureEnabled('feature_pinning')) {
let newPinsNotification = null;
if (this._hasUnreadPins()) {
newPinsNotification = (<div className="mx_RoomHeader_unreadPinsIndicator"></div>);
}
pinnedEventsButton =
<AccessibleButton className="mx_RoomHeader_button" onClick={this.props.onPinnedClick} title={_t("Pinned Messages")}>
<AccessibleButton className="mx_RoomHeader_button mx_RoomHeader_pinnedButton"
onClick={this.props.onPinnedClick} title={_t("Pinned Messages")}>
{ newPinsNotification }
<TintableSvg src="img/icons-pin.svg" width="16" height="16" />
</AccessibleButton>;
}
Expand Down