diff --git a/res/css/structures/_RoomView.scss b/res/css/structures/_RoomView.scss index 50d412ad58..5e826306c6 100644 --- a/res/css/structures/_RoomView.scss +++ b/res/css/structures/_RoomView.scss @@ -221,6 +221,9 @@ hr.mx_RoomView_myReadMarker { position: relative; top: -1px; z-index: 1; + transition: width 400ms easeInSine 1s, opacity 400ms easeInSine 1s; + width: 99%; + opacity: 1; } .mx_RoomView_callStatusBar .mx_UploadBar_uploadProgressInner { diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 912b865b9f..d1cc1b7caf 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -16,8 +16,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -/* global Velocity */ - import React from 'react'; import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; @@ -111,14 +109,12 @@ export default class MessagePanel extends React.Component { constructor() { super(); - // the event after which we put a visible unread marker on the last - // render cycle; null if readMarkerVisible was false or the RM was - // suppressed (eg because it was at the end of the timeline) - this.currentReadMarkerEventId = null; - // the event after which we are showing a disappearing read marker - // animation - this.currentGhostEventId = null; + this.state = { + // previous positions the read marker has been in, so we can + // display 'ghost' read markers that are animating away + ghostReadMarkers: [], + }; // opaque readreceipt info for each userId; used by ReadReceiptMarker // to manage its animations @@ -157,10 +153,6 @@ export default class MessagePanel extends React.Component { // displayed event in the current render cycle. this._readReceiptsByUserId = {}; - // Remember the read marker ghost node so we can do the cleanup that - // Velocity requires - this._readMarkerGhostNode = null; - // Cache hidden events setting on mount since Settings is expensive to // query, and we check this in a hot code path. this._showHiddenEventsInTimeline = @@ -177,6 +169,16 @@ export default class MessagePanel extends React.Component { this._isMounted = false; } + componentDidUpdate(prevProps, prevState) { + if (prevProps.readMarkerVisible && this.props.readMarkerEventId !== prevProps.readMarkerEventId) { + const ghostReadMarkers = this.state.ghostReadMarkers; + ghostReadMarkers.push(prevProps.readMarkerEventId); + this.setState({ + ghostReadMarkers, + }); + } + } + /* get the DOM node representing the given event */ getNodeForEventId(eventId) { if (!this.eventNodes) { @@ -325,6 +327,78 @@ export default class MessagePanel extends React.Component { return !shouldHideEvent(mxEv); } + _readMarkerForEvent(eventId, isLastEvent) { + const visible = !isLastEvent && this.props.readMarkerVisible; + + if (this.props.readMarkerEventId === eventId) { + let hr; + // if the read marker comes at the end of the timeline (except + // for local echoes, which are excluded from RMs, because they + // don't have useful event ids), we don't want to show it, but + // we still want to create the
  • for it so that the + // algorithms which depend on its position on the screen aren't + // confused. + if (visible) { + hr =
    ; + } + + return ( +
  • + { hr } +
    • + ); + } else if (this.state.ghostReadMarkers.includes(eventId)) { + // We render 'ghost' read markers in the DOM while they + // transition away. This allows the actual read marker + // to be in the right place straight away without having + // to wait for the transition to finish. + // There are probably much simpler ways to do this transition, + // possibly using react-transition-group which handles keeping + // elements in the DOM whilst they transition out, although our + // case is a little more complex because only some of the items + // transition (ie. the read markers do but the event tiles do not) + // and TransitionGroup requires that all its children are Transitions. + const hr =
    ; + + // give it a key which depends on the event id. That will ensure that + // we get a new DOM node (restarting the animation) when the ghost + // moves to a different event. + return ( +
  • + { hr } +
    • + ); + } + + return null; + } + + _collectGhostReadMarker = (node) => { + if (node) { + // now the element has appeared, change the style which will trigger the CSS transition + requestAnimationFrame(() => { + node.style.width = '10%'; + node.style.opacity = '0'; + }); + } + }; + + _onGhostTransitionEnd = (ev) => { + // we can now clean up the ghost element + const finishedEventId = ev.target.dataset.eventid; + this.setState({ + ghostReadMarkers: this.state.ghostReadMarkers.filter(eid => eid !== finishedEventId), + }); + }; + _getEventTiles() { const DateSeparator = sdk.getComponent('messages.DateSeparator'); const EventListSummary = sdk.getComponent('views.elements.EventListSummary'); @@ -332,7 +406,6 @@ export default class MessagePanel extends React.Component { this.eventNodes = {}; - let visible = false; let i; // first figure out which is the last event in the list which we're @@ -367,16 +440,6 @@ export default class MessagePanel extends React.Component { let prevEvent = null; // the last event we showed - // assume there is no read marker until proven otherwise - let readMarkerVisible = false; - - // if the readmarker has moved, cancel any active ghost. - if (this.currentReadMarkerEventId && this.props.readMarkerEventId && - this.props.readMarkerVisible && - this.currentReadMarkerEventId !== this.props.readMarkerEventId) { - this.currentGhostEventId = null; - } - this._readReceiptsByEvent = {}; if (this.props.showReadReceipts) { this._readReceiptsByEvent = this._getReadReceiptsByShownEvent(); @@ -401,7 +464,7 @@ export default class MessagePanel extends React.Component { return false; }; if (mxEv.getType() === "m.room.create") { - let readMarkerInSummary = false; + let summaryReadMarker = null; const ts1 = mxEv.getTs(); if (this._wantsDateSeparator(prevEvent, mxEv.getDate())) { @@ -410,9 +473,7 @@ export default class MessagePanel extends React.Component { } // If RM event is the first in the summary, append the RM after the summary - if (mxEv.getId() === this.props.readMarkerEventId) { - readMarkerInSummary = true; - } + summaryReadMarker = summaryReadMarker || this._readMarkerForEvent(mxEv.getId()); // If this m.room.create event should be shown (room upgrade) then show it before the summary if (this._shouldShowEvent(mxEv)) { @@ -427,9 +488,7 @@ export default class MessagePanel extends React.Component { // Ignore redacted/hidden member events if (!this._shouldShowEvent(collapsedMxEv)) { // If this hidden event is the RM and in or at end of a summary put RM after the summary. - if (collapsedMxEv.getId() === this.props.readMarkerEventId) { - readMarkerInSummary = true; - } + summaryReadMarker = summaryReadMarker || this._readMarkerForEvent(collapsedMxEv.getId()); continue; } @@ -438,9 +497,7 @@ export default class MessagePanel extends React.Component { } // If RM event is in the summary, mark it as such and the RM will be appended after the summary. - if (collapsedMxEv.getId() === this.props.readMarkerEventId) { - readMarkerInSummary = true; - } + summaryReadMarker = summaryReadMarker || this._readMarkerForEvent(collapsedMxEv.getId()); summarisedEvents.push(collapsedMxEv); } @@ -468,8 +525,8 @@ export default class MessagePanel extends React.Component { { eventTiles } ); - if (readMarkerInSummary) { - ret.push(this._getReadMarkerTile(visible)); + if (summaryReadMarker) { + ret.push(summaryReadMarker); } prevEvent = mxEv; @@ -480,7 +537,7 @@ export default class MessagePanel extends React.Component { // Wrap consecutive member events in a ListSummary, ignore if redacted if (isMembershipChange(mxEv) && wantTile) { - let readMarkerInMels = false; + let summaryReadMarker = null; const ts1 = mxEv.getTs(); // Ensure that the key of the MemberEventListSummary does not change with new // member events. This will prevent it from being re-created unnecessarily, and @@ -498,9 +555,7 @@ export default class MessagePanel extends React.Component { } // If RM event is the first in the MELS, append the RM after MELS - if (mxEv.getId() === this.props.readMarkerEventId) { - readMarkerInMels = true; - } + summaryReadMarker = summaryReadMarker || this._readMarkerForEvent(mxEv.getId()); const summarisedEvents = [mxEv]; for (;i + 1 < this.props.events.length; i++) { @@ -509,9 +564,7 @@ export default class MessagePanel extends React.Component { // Ignore redacted/hidden member events if (!this._shouldShowEvent(collapsedMxEv)) { // If this hidden event is the RM and in or at end of a MELS put RM after MELS. - if (collapsedMxEv.getId() === this.props.readMarkerEventId) { - readMarkerInMels = true; - } + summaryReadMarker = summaryReadMarker || this._readMarkerForEvent(collapsedMxEv.getId()); continue; } @@ -521,9 +574,7 @@ export default class MessagePanel extends React.Component { } // If RM event is in MELS mark it as such and the RM will be appended after MELS. - if (collapsedMxEv.getId() === this.props.readMarkerEventId) { - readMarkerInMels = true; - } + summaryReadMarker = summaryReadMarker || this._readMarkerForEvent(collapsedMxEv.getId()); summarisedEvents.push(collapsedMxEv); } @@ -554,8 +605,8 @@ export default class MessagePanel extends React.Component { { eventTiles } ); - if (readMarkerInMels) { - ret.push(this._getReadMarkerTile(visible)); + if (summaryReadMarker) { + ret.push(summaryReadMarker); } prevEvent = mxEv; @@ -570,40 +621,10 @@ export default class MessagePanel extends React.Component { prevEvent = mxEv; } - let isVisibleReadMarker = false; - - if (eventId === this.props.readMarkerEventId) { - visible = this.props.readMarkerVisible; - - // if the read marker comes at the end of the timeline (except - // for local echoes, which are excluded from RMs, because they - // don't have useful event ids), we don't want to show it, but - // we still want to create the
  • for it so that the - // algorithms which depend on its position on the screen aren't - // confused. - if (i >= lastShownNonLocalEchoIndex) { - visible = false; - } - ret.push(this._getReadMarkerTile(visible)); - readMarkerVisible = visible; - isVisibleReadMarker = visible; - } - - // XXX: there should be no need for a ghost tile - we should just use a - // a dispatch (user_activity_end) to start the RM animation. - if (eventId === this.currentGhostEventId) { - // if we're showing an animation, continue to show it. - ret.push(this._getReadMarkerGhostTile()); - } else if (!isVisibleReadMarker && - eventId === this.currentReadMarkerEventId) { - // there is currently a read-up-to marker at this point, but no - // more. Show an animation of it disappearing. - ret.push(this._getReadMarkerGhostTile()); - this.currentGhostEventId = eventId; - } + const readMarker = this._readMarkerForEvent(eventId, i >= lastShownNonLocalEchoIndex); + if (readMarker) ret.push(readMarker); } - this.currentReadMarkerEventId = readMarkerVisible ? this.props.readMarkerEventId : null; return ret; } @@ -797,53 +818,6 @@ export default class MessagePanel extends React.Component { return receiptsByEvent; } - _getReadMarkerTile(visible) { - let hr; - if (visible) { - hr =
    ; - } - - return ( -
  • - { hr } -
    • - ); - } - - _startAnimation = (ghostNode) => { - if (this._readMarkerGhostNode) { - Velocity.Utilities.removeData(this._readMarkerGhostNode); - } - this._readMarkerGhostNode = ghostNode; - - if (ghostNode) { - // eslint-disable-next-line new-cap - Velocity(ghostNode, {opacity: '0', width: '10%'}, - {duration: 400, easing: 'easeInSine', - delay: 1000}); - } - }; - - _getReadMarkerGhostTile() { - const hr =
    ; - - // give it a key which depends on the event id. That will ensure that - // we get a new DOM node (restarting the animation) when the ghost - // moves to a different event. - return ( -
  • - { hr } -
    • - ); - } - _collectEventNode = (eventId, node) => { this.eventNodes[eventId] = node; } diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index f58f1b925c..7c52512bc2 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -1,5 +1,6 @@ /* Copyright 2016 OpenMarket Ltd +Copyright 2019 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -81,6 +82,7 @@ describe('MessagePanel', function() { // HACK: We assume all settings want to be disabled SettingsStore.getValue = sinon.stub().returns(false); + SettingsStore.getValue.withArgs('showDisplaynameChanges').returns(true); // This option clobbers the duration of all animations to be 1ms // which makes unit testing a lot simpler (the animation doesn't @@ -109,6 +111,44 @@ describe('MessagePanel', function() { return events; } + + // make a collection of events with some member events that should be collapsed + // with a MemberEventListSummary + function mkMelsEvents() { + const events = []; + const ts0 = Date.now(); + + let i = 0; + events.push(test_utils.mkMessage({ + event: true, room: "!room:id", user: "@user:id", + ts: ts0 + ++i*1000, + })); + + for (i = 0; i < 10; i++) { + events.push(test_utils.mkMembership({ + event: true, room: "!room:id", user: "@user:id", + target: { + userId: "@user:id", + name: "Bob", + getAvatarUrl: () => { + return "avatar.jpeg"; + }, + }, + ts: ts0 + i*1000, + mship: 'join', + prevMship: 'join', + name: 'A user', + })); + } + + events.push(test_utils.mkMessage({ + event: true, room: "!room:id", user: "@user:id", + ts: ts0 + ++i*1000, + })); + + return events; + } + it('should show the events', function() { const res = TestUtils.renderIntoDocument( , @@ -120,6 +160,23 @@ describe('MessagePanel', function() { expect(tiles.length).toEqual(10); }); + it('should collapse adjacent member events', function() { + const res = TestUtils.renderIntoDocument( + , + ); + + // just check we have the right number of tiles for now + const tiles = TestUtils.scryRenderedComponentsWithType( + res, sdk.getComponent('rooms.EventTile'), + ); + expect(tiles.length).toEqual(2); + + const summaryTiles = TestUtils.scryRenderedComponentsWithType( + res, sdk.getComponent('elements.MemberEventListSummary'), + ); + expect(summaryTiles.length).toEqual(1); + }); + it('should show the read-marker in the right place', function() { const res = TestUtils.renderIntoDocument( , + ); + + const summary = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_EventListSummary'); + + // find the
  • which wraps the read marker + const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container'); + + expect(rm.previousSibling).toEqual(summary); + }); + it('shows a ghost read-marker when the read-marker moves', function(done) { // fake the clock so that we can test the velocity animation. clock.install(); @@ -191,50 +263,4 @@ describe('MessagePanel', function() { }, 100); }, 100); }); - - it('shows only one ghost when the RM moves twice', function() { - const parentDiv = document.createElement('div'); - - // first render with the RM in one place - let mp = ReactDOM.render( - , parentDiv); - - const tiles = TestUtils.scryRenderedComponentsWithType( - mp, sdk.getComponent('rooms.EventTile')); - const tileContainers = tiles.map(function(t) { - return ReactDOM.findDOMNode(t).parentNode; - }); - - // now move the RM - mp = ReactDOM.render( - , parentDiv); - - // now there should be two RM containers - let found = TestUtils.scryRenderedDOMComponentsWithClass(mp, 'mx_RoomView_myReadMarker_container'); - expect(found.length).toEqual(2); - - // the first should be the ghost - expect(tileContainers.indexOf(found[0].previousSibling)).toEqual(4); - - // the second should be the real RM - expect(tileContainers.indexOf(found[1].previousSibling)).toEqual(6); - - // and move the RM again - mp = ReactDOM.render( - , parentDiv); - - // still two RM containers - found = TestUtils.scryRenderedDOMComponentsWithClass(mp, 'mx_RoomView_myReadMarker_container'); - expect(found.length).toEqual(2); - - // they should have moved - expect(tileContainers.indexOf(found[0].previousSibling)).toEqual(6); - expect(tileContainers.indexOf(found[1].previousSibling)).toEqual(8); - }); });