From d3cf78ff5abd3297cb1f51a3017d5236db114af5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 8 Jun 2017 14:17:49 +0100 Subject: [PATCH 01/18] Control currently viewied event via RoomViewStore Fix for https://github.com/vector-im/riot-web/issues/4224 Due to the way `MatrixChat` does a state update when the `view_room` dispatch fires and a second update when `RoomViewStore` sends an update, the current event ID and room ID were becoming out of sync. The solution devised was to have the event ID managed by the `RoomViewStore` itself and do any defaulting there (for when we revisit a room that we saved scroll state for previously). This required a few changes: - The addition of `update_scroll_state` in `RoomViewStore` allows the `RoomView` to save scroll state for a room before swapping to another one. Previously the caching of scroll state was done in `RoomView`. - The `view_room` dispatch now accepts an `event_id`, which dictates which event is supposed to be scrolled to in the `MessagePanel` when a new room is viewed. It also accepts `event_offset`, but currently, this isn't passed in by a dispatch in the app, but it is clobbered when loading the default position when an `event_id` isn't specified. Finally, `highlighted` was added to distinguish whether the initial event being scrolled to is also highlighted. This flag is also used by `viewRoom` in `MatrixChat` in order to decide whether to `notifyNewScreen` with the specified `event_id`. --- src/components/structures/LoggedInView.js | 2 - src/components/structures/MatrixChat.js | 19 +---- src/components/structures/RoomView.js | 93 +++++++++-------------- src/components/views/rooms/EventTile.js | 1 + src/stores/RoomViewStore.js | 72 +++++++++++++++++- 5 files changed, 109 insertions(+), 78 deletions(-) diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index a201a0bea7..8fa35e84d7 100644 --- a/src/components/structures/LoggedInView.js +++ b/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} diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index efb2b38d6e..b65fa0be1c 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -618,9 +618,6 @@ 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, @@ -632,18 +629,6 @@ module.exports = React.createClass({ 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; - } - } - // Wait for the first sync to complete so that if a room does have an alias, // it would have been retrieved. let waitFor = q(null); @@ -669,7 +654,7 @@ module.exports = React.createClass({ } } - if (roomInfo.event_id) { + if (roomInfo.event_id && roomInfo.highlighted) { presentedId += "/" + roomInfo.event_id; } this.notifyNewScreen('room/' + presentedId); @@ -1124,8 +1109,10 @@ module.exports = React.createClass({ }; const payload = { + id: '#mylovelyid', action: 'view_room', event_id: eventId, + highlighted: Boolean(eventId), third_party_invite: thirdPartyInvite, oob_data: oobData, }; diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index df21715d75..a4c589187b 100644 --- a/src/components/structures/RoomView.js +++ b/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() { @@ -180,13 +152,28 @@ 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(), - }, () => { + eventId: RoomViewStore.getEventId(), + eventPixelOffset: RoomViewStore.getEventPixelOffset(), + isEventHighlighted: RoomViewStore.isEventHighlighted(), + }; + + if (this.state.eventId !== newState.eventId) { + 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, () => { this._onHaveRoom(); this.onRoom(MatrixClientPeg.get().getRoom(this.state.roomId)); }); @@ -287,13 +274,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 +299,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 +578,14 @@ module.exports = React.createClass({ }); }, + _updateScrollMap(roomId) { + 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 +1228,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 +1650,14 @@ module.exports = React.createClass({ hideMessagePanel = true; } + const shouldHighlight = this.state.isEventHighlighted; + let highlightedEventId = null; + if (this.state.forwardingEvent) { + highlightedEventId = this.state.forwardingEvent.getId(); + } else if (shouldHighlight) { + highlightedEventId = this.state.eventId; + } + // console.log("ShowUrlPreview for %s is %s", this.state.room.roomId, this.state.showUrlPreview); var messagePanel = (