Merge pull request #1058 from matrix-org/luke/fix-event-id-state

Control currently viewed event via RoomViewStore

src/components/structures/LoggedInView.js

@@ -223,10 +223,8 @@ export default React.createClass({
                         ref='roomView'
                         autoJoin={this.props.autoJoin}
                         onRegistered={this.props.onRegistered}
-                        eventId={this.props.initialEventId}
                         thirdPartyInvite={this.props.thirdPartyInvite}
                         oobData={this.props.roomOobData}
-                        highlightedEventId={this.props.highlightedEventId}
                         eventPixelOffset={this.props.initialEventPixelOffset}
                         key={this.props.currentRoomId || 'roomview'}
                         opacity={this.props.middleOpacity}

src/components/structures/MatrixChat.js

@@ -607,6 +607,8 @@ module.exports = React.createClass({
     // @param {boolean=} roomInfo.show_settings Makes RoomView show the room settings dialog.
     // @param {string=} roomInfo.event_id ID of the event in this room to show: this will cause a switch to the
     //                                    context of that particular event.
+    // @param {boolean=} roomInfo.highlighted If true, add event_id to the hash of the URL
+    //                                        and alter the EventTile to appear highlighted.
     // @param {Object=} roomInfo.third_party_invite Object containing data about the third party
     //                                    we received to join the room, if any.
     // @param {string=} roomInfo.third_party_invite.inviteSignUrl 3pid invite sign URL
@@ -618,40 +620,20 @@ module.exports = React.createClass({
         this.focusComposer = true;
 
         const newState = {
-            initialEventId: roomInfo.event_id,
-            highlightedEventId: roomInfo.event_id,
-            initialEventPixelOffset: undefined,
             page_type: PageTypes.RoomView,
             thirdPartyInvite: roomInfo.third_party_invite,
             roomOobData: roomInfo.oob_data,
-            currentRoomAlias: roomInfo.room_alias,
             autoJoin: roomInfo.auto_join,
         };
 
-        if (!roomInfo.room_alias) {
-            newState.currentRoomId = roomInfo.room_id;
-        }
-
-        // if we aren't given an explicit event id, look for one in the
-        // scrollStateMap.
-        //
-        // TODO: do this in RoomView rather than here
-        if (!roomInfo.event_id && this.refs.loggedInView) {
-            const scrollState = this.refs.loggedInView.getScrollStateForRoom(roomInfo.room_id);
-            if (scrollState) {
-                newState.initialEventId = scrollState.focussedEvent;
-                newState.initialEventPixelOffset = scrollState.pixelOffset;
-            }
-        }
-
         if (roomInfo.room_alias) {
             console.log(
                 `Switching to room alias ${roomInfo.room_alias} at event ` +
-                newState.initialEventId,
+                roomInfo.event_id,
             );
         } else {
             console.log(`Switching to room id ${roomInfo.room_id} at event ` +
-                newState.initialEventId,
+                roomInfo.event_id,
             );
         }
 
@@ -680,7 +662,7 @@ module.exports = React.createClass({
                 }
             }
 
-            if (roomInfo.event_id) {
+            if (roomInfo.event_id && roomInfo.highlighted) {
                 presentedId += "/" + roomInfo.event_id;
             }
             this.notifyNewScreen('room/' + presentedId);
@@ -1137,6 +1119,10 @@ module.exports = React.createClass({
             const payload = {
                 action: 'view_room',
                 event_id: eventId,
+                // If an event ID is given in the URL hash, notify RoomViewStore to mark
+                // it as highlighted, which will propagate to RoomView and highlight the
+                // associated EventTile.
+                highlighted: Boolean(eventId),
                 third_party_invite: thirdPartyInvite,
                 oob_data: oobData,
             };

src/components/structures/RoomView.js

@@ -83,36 +83,8 @@ module.exports = React.createClass({
         //  *                      invited us tovthe room
         oobData: React.PropTypes.object,
 
-        // id of an event to jump to. If not given, will go to the end of the
-        // live timeline.
-        eventId: React.PropTypes.string,
-
-        // where to position the event given by eventId, in pixels from the
-        // bottom of the viewport. If not given, will try to put the event
-        // 1/3 of the way down the viewport.
-        eventPixelOffset: React.PropTypes.number,
-
-        // ID of an event to highlight. If undefined, no event will be highlighted.
-        // Typically this will either be the same as 'eventId', or undefined.
-        highlightedEventId: React.PropTypes.string,
-
         // is the RightPanel collapsed?
         collapsedRhs: React.PropTypes.bool,
-
-        // a map from room id to scroll state, which will be updated on unmount.
-        //
-        // If there is no special scroll state (ie, we are following the live
-        // timeline), the scroll state is null. Otherwise, it is an object with
-        // the following properties:
-        //
-        //    focussedEvent: the ID of the 'focussed' event. Typically this is
-        //        the last event fully visible in the viewport, though if we
-        //        have done an explicit scroll to an explicit event, it will be
-        //        that event.
-        //
-        //    pixelOffset: the number of pixels the window is scrolled down
-        //        from the focussedEvent.
-        scrollStateMap: React.PropTypes.object,
     },
 
     getInitialState: function() {
@@ -122,6 +94,13 @@ module.exports = React.createClass({
             roomLoading: true,
             peekLoading: false,
 
+            // The event to be scrolled to initially
+            initialEventId: null,
+            // The offset in pixels from the event with which to scroll vertically
+            initialEventPixelOffset: null,
+            // Whether to highlight the event scrolled to
+            isInitialEventHighlighted: null,
+
             forwardingEvent: null,
             editingRoomSettings: false,
             uploadingRoomSettings: false,
@@ -180,13 +159,32 @@ module.exports = React.createClass({
         if (this.unmounted) {
             return;
         }
-        this.setState({
+        const newState = {
             roomId: RoomViewStore.getRoomId(),
             roomAlias: RoomViewStore.getRoomAlias(),
             roomLoading: RoomViewStore.isRoomLoading(),
             roomLoadError: RoomViewStore.getRoomLoadError(),
             joining: RoomViewStore.isJoining(),
-        }, () => {
+            initialEventId: RoomViewStore.getInitialEventId(),
+            initialEventPixelOffset: RoomViewStore.getInitialEventPixelOffset(),
+            isInitialEventHighlighted: RoomViewStore.isInitialEventHighlighted(),
+        };
+
+        // Clear the search results when clicking a search result (which changes the
+        // currently scrolled to event, this.state.initialEventId).
+        if (this.state.initialEventId !== newState.initialEventId) {
+            newState.searchResults = null;
+        }
+
+        // Store the scroll state for the previous room so that we can return to this
+        // position when viewing this room in future.
+        if (this.state.roomId !== newState.roomId) {
+            this._updateScrollMap(this.state.roomId);
+        }
+
+        this.setState(newState, () => {
+            // At this point, this.state.roomId could be null (e.g. the alias might not
+            // have been resolved yet) so anything called here must handle this case.
             this._onHaveRoom();
             this.onRoom(MatrixClientPeg.get().getRoom(this.state.roomId));
         });
@@ -287,13 +285,6 @@ module.exports = React.createClass({
         }
     },
 
-    componentWillReceiveProps: function(newProps) {
-        if (newProps.eventId != this.props.eventId) {
-            // when we change focussed event id, hide the search results.
-            this.setState({searchResults: null});
-        }
-    },
-
     shouldComponentUpdate: function(nextProps, nextState) {
         return (!ObjectUtils.shallowEqual(this.props, nextProps) ||
                 !ObjectUtils.shallowEqual(this.state, nextState));
@@ -319,7 +310,7 @@ module.exports = React.createClass({
         this.unmounted = true;
 
         // update the scroll map before we get unmounted
-        this._updateScrollMap();
+        this._updateScrollMap(this.state.roomId);
 
         if (this.refs.roomView) {
             // disconnect the D&D event listeners from the room view. This
@@ -598,6 +589,18 @@ module.exports = React.createClass({
         });
     },
 
+    _updateScrollMap(roomId) {
+        // No point updating scroll state if the room ID hasn't been resolved yet
+        if (!roomId) {
+            return;
+        }
+        dis.dispatch({
+            action: 'update_scroll_state',
+            room_id: roomId,
+            scroll_state: this._getScrollState(),
+        });
+    },
+
     onRoom: function(room) {
         if (!room || room.roomId !== this.state.roomId) {
             return;
@@ -1240,21 +1243,6 @@ module.exports = React.createClass({
         }
     },
 
-    // update scrollStateMap on unmount
-    _updateScrollMap: function() {
-        if (!this.state.room) {
-            // we were instantiated on a room alias and haven't yet joined the room.
-            return;
-        }
-        if (!this.props.scrollStateMap) return;
-
-        var roomId = this.state.room.roomId;
-
-        var state = this._getScrollState();
-        this.props.scrollStateMap[roomId] = state;
-    },
-
-
     // get the current scroll position of the room, so that it can be
     // restored when we switch back to it.
     //
@@ -1677,6 +1665,14 @@ module.exports = React.createClass({
             hideMessagePanel = true;
         }
 
+        const shouldHighlight = this.state.isInitialEventHighlighted;
+        let highlightedEventId = null;
+        if (this.state.forwardingEvent) {
+            highlightedEventId = this.state.forwardingEvent.getId();
+        } else if (shouldHighlight) {
+            highlightedEventId = this.state.initialEventId;
+        }
+
         // console.log("ShowUrlPreview for %s is %s", this.state.room.roomId, this.state.showUrlPreview);
         var messagePanel = (
             <TimelinePanel ref={this._gatherTimelinePanelRef}
@@ -1684,9 +1680,9 @@ module.exports = React.createClass({
                 manageReadReceipts={!UserSettingsStore.getSyncedSetting('hideReadReceipts', false)}
                 manageReadMarkers={true}
                 hidden={hideMessagePanel}
-                highlightedEventId={this.state.forwardingEvent ? this.state.forwardingEvent.getId() : this.props.highlightedEventId}
+                highlightedEventId={highlightedEventId}
-                eventId={this.props.eventId}
+                eventId={this.state.initialEventId}
-                eventPixelOffset={this.props.eventPixelOffset}
+                eventPixelOffset={this.state.initialEventPixelOffset}
                 onScroll={ this.onMessageListScroll }
                 onReadMarkerUpdated={ this._updateTopUnreadMessagesBar }
                 showUrlPreview = { this.state.showUrlPreview }

src/components/views/rooms/EventTile.js

@@ -381,6 +381,7 @@ module.exports = WithMatrixClient(React.createClass({
         dis.dispatch({
             action: 'view_room',
             event_id: this.props.mxEvent.getId(),
+            highlighted: true,
             room_id: this.props.mxEvent.getRoomId(),
         });
     },

src/stores/RoomViewStore.js

@@ -23,16 +23,38 @@ import { _t } from '../languageHandler';
 const INITIAL_STATE = {
     // Whether we're joining the currently viewed room
     joining: false,
-    // Any error occurred during joining
+    // Any error that has occurred during joining
     joinError: null,
-    // The room ID of the room
+    // The room ID of the room currently being viewed
     roomId: null,
+
+    // The event to scroll to when the room is first viewed
+    initialEventId: null,
+    // The offset to display the initial event at (see scrollStateMap)
+    initialEventPixelOffset: null,
+    // Whether to highlight the initial event
+    isInitialEventHighlighted: false,
+
     // The room alias of the room (or null if not originally specified in view_room)
     roomAlias: null,
     // Whether the current room is loading
     roomLoading: false,
     // Any error that has occurred during loading
     roomLoadError: null,
+    // A map from room id to scroll state.
+    //
+    // If there is no special scroll state (ie, we are following the live
+    // timeline), the scroll state is null. Otherwise, it is an object with
+    // the following properties:
+    //
+    //    focussedEvent: the ID of the 'focussed' event. Typically this is
+    //        the last event fully visible in the viewport, though if we
+    //        have done an explicit scroll to an explicit event, it will be
+    //        that event.
+    //
+    //    pixelOffset: the number of pixels the window is scrolled down
+    //        from the focussedEvent.
+    scrollStateMap: {},
 };
 
 /**
@@ -58,6 +80,9 @@ class RoomViewStore extends Store {
             // view_room:
             //      - room_alias:   '#somealias:matrix.org'
             //      - room_id:      '!roomid123:matrix.org'
+            //      - event_id:     '$213456782:matrix.org'
+            //      - event_offset: 100
+            //      - highlighted:  true
             case 'view_room':
                 this._viewRoom(payload);
                 break;
@@ -88,20 +113,41 @@ class RoomViewStore extends Store {
             case 'on_logged_out':
                 this.reset();
                 break;
+            case 'update_scroll_state':
+                this._updateScrollState(payload);
+                break;
         }
     }
 
     _viewRoom(payload) {
-        // Always set the room ID if present
         if (payload.room_id) {
-            this._setState({
+            const newState = {
                 roomId: payload.room_id,
+                initialEventId: payload.event_id,
+                initialEventPixelOffset: undefined,
+                isInitialEventHighlighted: payload.highlighted,
                 roomLoading: false,
                 roomLoadError: null,
-            });
+            };
+
+            // If an event ID wasn't specified, default to the one saved for this room
+            // via update_scroll_state. Assume initialEventPixelOffset should be set.
+            if (!newState.initialEventId) {
+                const roomScrollState = this._state.scrollStateMap[payload.room_id];
+                if (roomScrollState) {
+                    newState.initialEventId = roomScrollState.focussedEvent;
+                    newState.initialEventPixelOffset = roomScrollState.pixelOffset;
+                }
+            }
+
+            this._setState(newState);
         } else if (payload.room_alias) {
+            // Resolve the alias and then do a second dispatch with the room ID acquired
             this._setState({
                 roomId: null,
+                initialEventId: null,
+                initialEventPixelOffset: null,
+                isInitialEventHighlighted: null,
                 roomAlias: payload.room_alias,
                 roomLoading: true,
                 roomLoadError: null,
@@ -111,6 +157,8 @@ class RoomViewStore extends Store {
                 dis.dispatch({
                     action: 'view_room',
                     room_id: result.room_id,
+                    event_id: payload.event_id,
+                    highlighted: payload.highlighted,
                     room_alias: payload.room_alias,
                 });
             }, (err) => {
@@ -168,34 +216,63 @@ class RoomViewStore extends Store {
         });
     }
 
+    _updateScrollState(payload) {
+        // Clobber existing scroll state for the given room ID
+        const newScrollStateMap = this._state.scrollStateMap;
+        newScrollStateMap[payload.room_id] = payload.scroll_state;
+        this._setState({
+            scrollStateMap: newScrollStateMap,
+        });
+    }
+
     reset() {
         this._state = Object.assign({}, INITIAL_STATE);
     }
 
+    // The room ID of the room currently being viewed
     getRoomId() {
         return this._state.roomId;
     }
 
+    // The event to scroll to when the room is first viewed
+    getInitialEventId() {
+        return this._state.initialEventId;
+    }
+
+    // The offset to display the initial event at (see scrollStateMap)
+    getInitialEventPixelOffset() {
+        return this._state.initialEventPixelOffset;
+    }
+
+    // Whether to highlight the initial event
+    isInitialEventHighlighted() {
+        return this._state.isInitialEventHighlighted;
+    }
+
+    // The room alias of the room (or null if not originally specified in view_room)
     getRoomAlias() {
         return this._state.roomAlias;
     }
 
+    // Whether the current room is loading (true whilst resolving an alias)
     isRoomLoading() {
         return this._state.roomLoading;
     }
 
+    // Any error that has occurred during loading
     getRoomLoadError() {
         return this._state.roomLoadError;
     }
 
+    // Whether we're joining the currently viewed room
     isJoining() {
         return this._state.joining;
     }
 
+    // Any error that has occurred during joining
     getJoinError() {
         return this._state.joinError;
     }
-
 }
 
 let singletonRoomViewStore = null;