diff --git a/res/css/_components.scss b/res/css/_components.scss index f627fe3a29..4891fd90c0 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -88,12 +88,12 @@ @import "./views/elements/_Dropdown.scss"; @import "./views/elements/_EditableItemList.scss"; @import "./views/elements/_ErrorBoundary.scss"; +@import "./views/elements/_EventListSummary.scss"; @import "./views/elements/_Field.scss"; @import "./views/elements/_ImageView.scss"; @import "./views/elements/_InlineSpinner.scss"; @import "./views/elements/_InteractiveTooltip.scss"; @import "./views/elements/_ManageIntegsButton.scss"; -@import "./views/elements/_MemberEventListSummary.scss"; @import "./views/elements/_PowerSelector.scss"; @import "./views/elements/_ProgressBar.scss"; @import "./views/elements/_ReplyThread.scss"; diff --git a/res/css/views/elements/_MemberEventListSummary.scss b/res/css/views/elements/_EventListSummary.scss similarity index 75% rename from res/css/views/elements/_MemberEventListSummary.scss rename to res/css/views/elements/_EventListSummary.scss index 02ecb5d84a..99a5c06a5f 100644 --- a/res/css/views/elements/_MemberEventListSummary.scss +++ b/res/css/views/elements/_EventListSummary.scss @@ -14,28 +14,28 @@ See the License for the specific language governing permissions and limitations under the License. */ -.mx_MemberEventListSummary { +.mx_EventListSummary { position: relative; } -.mx_TextualEvent.mx_MemberEventListSummary_summary { +.mx_TextualEvent.mx_EventListSummary_summary { font-size: 14px; display: inline-flex; } -.mx_MemberEventListSummary_avatars { +.mx_EventListSummary_avatars { display: inline-block; margin-right: 8px; padding-top: 8px; line-height: 12px; } -.mx_MemberEventListSummary_avatars .mx_BaseAvatar { +.mx_EventListSummary_avatars .mx_BaseAvatar { margin-right: -4px; cursor: pointer; } -.mx_MemberEventListSummary_toggle { +.mx_EventListSummary_toggle { color: $accent-color; cursor: pointer; float: right; @@ -43,29 +43,29 @@ limitations under the License. margin-top: 8px; } -.mx_MemberEventListSummary_line { +.mx_EventListSummary_line { border-bottom: 1px solid $primary-hairline-color; margin-left: 63px; line-height: 30px; } .mx_MatrixChat_useCompactLayout { - .mx_MemberEventListSummary { + .mx_EventListSummary { font-size: 13px; .mx_EventTile_line { line-height: 20px; } } - .mx_MemberEventListSummary_line { + .mx_EventListSummary_line { line-height: 22px; } - .mx_MemberEventListSummary_toggle { + .mx_EventListSummary_toggle { margin-top: 3px; } - .mx_TextualEvent.mx_MemberEventListSummary_summary { + .mx_TextualEvent.mx_EventListSummary_summary { font-size: 13px; } } diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 33ae0c3f74..7254059734 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -28,6 +28,7 @@ import sdk from '../../index'; import MatrixClientPeg from '../../MatrixClientPeg'; import SettingsStore from '../../settings/SettingsStore'; +import {_t} from "../../languageHandler"; const CONTINUATION_MAX_INTERVAL = 5 * 60 * 1000; // 5 minutes const continuedTypes = ['m.sticker', 'm.room.message']; @@ -323,6 +324,7 @@ module.exports = createReactClass({ _getEventTiles: function() { const DateSeparator = sdk.getComponent('messages.DateSeparator'); + const EventListSummary = sdk.getComponent('views.elements.EventListSummary'); const MemberEventListSummary = sdk.getComponent('views.elements.MemberEventListSummary'); this.eventNodes = {}; @@ -382,6 +384,87 @@ module.exports = createReactClass({ const eventId = mxEv.getId(); const last = (mxEv === lastShownEvent); + // Wrap initial room creation events into an EventListSummary + // Grouping only events sent by the same user that sent the `m.room.create` and only until + // the first non-state event or membership event which is not regarding the sender of the `m.room.create` event + const shouldGroup = (ev) => { + if (ev.getType() === "m.room.member" + && (ev.getStateKey() !== mxEv.getSender() || ev.getContent()["membership"] !== "join")) { + return false; + } + if (ev.isState() && ev.getSender() === mxEv.getSender()) { + return true; + } + return false; + }; + if (mxEv.getType() === "m.room.create") { + let readMarkerInSummary = false; + const ts1 = mxEv.getTs(); + + if (this._wantsDateSeparator(prevEvent, mxEv.getDate())) { + const dateSeparator =
    • ; + ret.push(dateSeparator); + } + + // If RM event is the first in the summary, append the RM after the summary + if (mxEv.getId() === this.props.readMarkerEventId) { + readMarkerInSummary = true; + } + + const summarisedEvents = []; // Don't add m.room.create here as we don't want it inside the summary + for (;i + 1 < this.props.events.length; i++) { + const collapsedMxEv = this.props.events[i + 1]; + + // 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; + } + continue; + } + + if (!shouldGroup(collapsedMxEv) || this._wantsDateSeparator(mxEv, collapsedMxEv.getDate())) { + break; + } + + // 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; + } + + summarisedEvents.push(collapsedMxEv); + } + + // At this point, i = the index of the last event in the summary sequence + const eventTiles = summarisedEvents.map((e) => { + // In order to prevent DateSeparators from appearing in the expanded form + // of EventListSummary, render each member event as if the previous + // one was itself. This way, the timestamp of the previous event === the + // timestamp of the current event, and no DateSeparator is inserted. + return this._getTilesForEvent(e, e, e === lastShownEvent); + }).reduce((a, b) => a.concat(b)); + + ret.push( + { eventTiles } + ); + + if (readMarkerInSummary) { + ret.push(this._getReadMarkerTile(visible)); + } + + prevEvent = mxEv; + continue; + } + const wantTile = this._shouldShowEvent(mxEv); // Wrap consecutive member events in a ListSummary, ignore if redacted diff --git a/src/components/views/elements/EventListSummary.js b/src/components/views/elements/EventListSummary.js new file mode 100644 index 0000000000..d6971334d4 --- /dev/null +++ b/src/components/views/elements/EventListSummary.js @@ -0,0 +1,95 @@ +/* +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. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React, {useEffect} from 'react'; +import PropTypes from 'prop-types'; +import MemberAvatar from '../avatars/MemberAvatar'; +import { _t } from '../../../languageHandler'; +import {MatrixEvent, RoomMember} from "matrix-js-sdk"; +import {useStateToggle} from "../../../hooks/useStateToggle"; + +const EventListSummary = ({events, children, threshold=3, onToggle, startExpanded, summaryMembers=[], summaryText}) => { + const [expanded, toggleExpanded] = useStateToggle(startExpanded); + + // Whenever expanded changes call onToggle + useEffect(() => { + if (onToggle) { + onToggle(); + } + }, [expanded]); + + const eventIds = events.map((e) => e.getId()).join(','); + + // If we are only given few events then just pass them through + if (events.length < threshold) { + return ( +
    + { children } +
    + ); + } + + if (expanded) { + return ( +
    +
    + { _t('collapse') } +
    +
     
    + { children } +
    + ); + } + + const avatars = summaryMembers.map((m) => ); + return ( +
    +
    + { _t('expand') } +
    +
    +
    + + { avatars } + + + { summaryText } + +
    +
    +
    + ); +}; + +EventListSummary.propTypes = { + // An array of member events to summarise + events: PropTypes.arrayOf(PropTypes.instanceOf(MatrixEvent)).isRequired, + // An array of EventTiles to render when expanded + children: PropTypes.arrayOf(PropTypes.element).isRequired, + // The minimum number of events needed to trigger summarisation + threshold: PropTypes.number, + // Called when the event list expansion is toggled + onToggle: PropTypes.func, + // Whether or not to begin with state.expanded=true + startExpanded: PropTypes.bool, + + // The list of room members for which to show avatars next to the summary + summaryMembers: PropTypes.arrayOf(PropTypes.instanceOf(RoomMember)), + // The text to show as the summary of this event list + summaryText: PropTypes.string.isRequired, +}; + +export default EventListSummary; diff --git a/src/components/views/elements/MemberEventListSummary.js b/src/components/views/elements/MemberEventListSummary.js index ba31eb5a38..ef80efaa68 100644 --- a/src/components/views/elements/MemberEventListSummary.js +++ b/src/components/views/elements/MemberEventListSummary.js @@ -19,16 +19,17 @@ limitations under the License. import React from 'react'; import PropTypes from 'prop-types'; import createReactClass from 'create-react-class'; -import MemberAvatar from '../avatars/MemberAvatar'; import { _t } from '../../../languageHandler'; import { formatCommaSeparatedList } from '../../../utils/FormattingUtils'; +import sdk from "../../../index"; +import {MatrixEvent} from "matrix-js-sdk"; module.exports = createReactClass({ displayName: 'MemberEventListSummary', propTypes: { // An array of member events to summarise - events: PropTypes.array.isRequired, + events: PropTypes.arrayOf(PropTypes.instanceOf(MatrixEvent)).isRequired, // An array of EventTiles to render when expanded children: PropTypes.array.isRequired, // The maximum number of names to show in either each summary e.g. 2 would result "A, B and 234 others left" @@ -43,12 +44,6 @@ module.exports = createReactClass({ startExpanded: PropTypes.bool, }, - getInitialState: function() { - return { - expanded: Boolean(this.props.startExpanded), - }; - }, - getDefaultProps: function() { return { summaryLength: 1, @@ -57,37 +52,27 @@ module.exports = createReactClass({ }; }, - shouldComponentUpdate: function(nextProps, nextState) { + shouldComponentUpdate: function(nextProps) { // Update if // - The number of summarised events has changed - // - or if the summary is currently expanded // - or if the summary is about to toggle to become collapsed // - or if there are fewEvents, meaning the child eventTiles are shown as-is return ( nextProps.events.length !== this.props.events.length || - this.state.expanded || nextState.expanded || nextProps.events.length < this.props.threshold ); }, - _toggleSummary: function() { - this.setState({ - expanded: !this.state.expanded, - }); - this.props.onToggle(); - }, - /** - * Render the JSX for users aggregated by their transition sequences (`eventAggregates`) where + * Generate the text for users aggregated by their transition sequences (`eventAggregates`) where * the sequences are ordered by `orderedTransitionSequences`. * @param {object[]} eventAggregates a map of transition sequence to array of user display names * or user IDs. * @param {string[]} orderedTransitionSequences an array which is some ordering of * `Object.keys(eventAggregates)`. - * @returns {ReactElement} a single containing the textual summary of the aggregated - * events that occurred. + * @returns {string} the textual summary of the aggregated events that occurred. */ - _renderSummary: function(eventAggregates, orderedTransitionSequences) { + _generateSummary: function(eventAggregates, orderedTransitionSequences) { const summaries = orderedTransitionSequences.map((transitions) => { const userNames = eventAggregates[transitions]; const nameList = this._renderNameList(userNames); @@ -118,11 +103,7 @@ module.exports = createReactClass({ return null; } - return ( - - { summaries.join(", ") } - - ); + return summaries.join(", "); }, /** @@ -208,7 +189,7 @@ module.exports = createReactClass({ * For a certain transition, t, describe what happened to the users that * underwent the transition. * @param {string} t the transition type. - * @param {integer} userCount number of usernames + * @param {number} userCount number of usernames * @param {number} repeats the number of times the transition was repeated in a row. * @returns {string} the written Human Readable equivalent of the transition. */ @@ -288,19 +269,6 @@ module.exports = createReactClass({ return res; }, - _renderAvatars: function(roomMembers) { - const avatars = roomMembers.slice(0, this.props.avatarsMaxLength).map((m) => { - return ( - - ); - }); - return ( - - { avatars } - - ); - }, - _getTransitionSequence: function(events) { return events.map(this._getTransition); }, @@ -396,22 +364,6 @@ module.exports = createReactClass({ render: function() { const eventsToRender = this.props.events; - const eventIds = eventsToRender.map((e) => e.getId()).join(','); - const fewEvents = eventsToRender.length < this.props.threshold; - const expanded = this.state.expanded || fewEvents; - - let expandedEvents = null; - if (expanded) { - expandedEvents = this.props.children; - } - - if (fewEvents) { - return ( -
    - { expandedEvents } -
    - ); - } // Map user IDs to an array of objects: const userEvents = { @@ -455,30 +407,14 @@ module.exports = createReactClass({ (seq1, seq2) => aggregate.indices[seq1] > aggregate.indices[seq2], ); - let summaryContainer = null; - if (!expanded) { - summaryContainer = ( -
    -
    - { this._renderAvatars(avatarMembers) } - { this._renderSummary(aggregate.names, orderedTransitionSequences) } -
    -
    - ); - } - const toggleButton = ( -
    - { expanded ? _t('collapse') : _t('expand') } -
    - ); - - return ( -
    - { toggleButton } - { summaryContainer } - { expanded ?
     
    : null } - { expandedEvents } -
    - ); + const EventListSummary = sdk.getComponent("views.elements.EventListSummary"); + return ; }, }); diff --git a/src/hooks/useStateToggle.js b/src/hooks/useStateToggle.js new file mode 100644 index 0000000000..58cf123bfb --- /dev/null +++ b/src/hooks/useStateToggle.js @@ -0,0 +1,27 @@ +/* +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. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {useState} from 'react'; + +// Hook to simplify toggling of a boolean state value +// Returns value, method to toggle boolean value and method to set the boolean value +export const useStateToggle = (initialValue) => { + const [value, setValue] = useState(Boolean(initialValue)); + const toggleValue = () => { + setValue(!value); + }; + return [value, toggleValue, setValue]; +}; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index bd9229aaea..f524a22d4b 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1133,6 +1133,8 @@ "Yes": "Yes", "No": "No", "Please create a new issue on GitHub so that we can investigate this bug.": "Please create a new issue on GitHub so that we can investigate this bug.", + "collapse": "collapse", + "expand": "expand", "Communities": "Communities", "You cannot delete this image. (%(code)s)": "You cannot delete this image. (%(code)s)", "Uploaded on %(date)s by %(user)s": "Uploaded on %(date)s by %(user)s", @@ -1195,8 +1197,6 @@ "%(severalUsers)smade no changes %(count)s times|one": "%(severalUsers)smade no changes", "%(oneUser)smade no changes %(count)s times|other": "%(oneUser)smade no changes %(count)s times", "%(oneUser)smade no changes %(count)s times|one": "%(oneUser)smade no changes", - "collapse": "collapse", - "expand": "expand", "Power level": "Power level", "Custom level": "Custom level", "Unable to load event that was replied to, it either does not exist or you do not have permission to view it.": "Unable to load event that was replied to, it either does not exist or you do not have permission to view it.", @@ -1611,6 +1611,7 @@ "Old cryptography data detected": "Old cryptography data detected", "Data from an older version of Riot has been detected. This will have caused end-to-end cryptography to malfunction in the older version. End-to-end encrypted messages exchanged recently whilst using the older version may not be decryptable in this version. This may also cause messages exchanged with this version to fail. If you experience problems, log out and back in again. To retain message history, export and re-import your keys.": "Data from an older version of Riot has been detected. This will have caused end-to-end cryptography to malfunction in the older version. End-to-end encrypted messages exchanged recently whilst using the older version may not be decryptable in this version. This may also cause messages exchanged with this version to fail. If you experience problems, log out and back in again. To retain message history, export and re-import your keys.", "Logout": "Logout", + "%(creator)s created and configured the room.": "%(creator)s created and configured the room.", "Your Communities": "Your Communities", "Did you know: you can use communities to filter your Riot.im experience!": "Did you know: you can use communities to filter your Riot.im experience!", "To set up a filter, drag a community avatar over to the filter panel on the far left hand side of the screen. You can click on an avatar in the filter panel at any time to see only the rooms and people associated with that community.": "To set up a filter, drag a community avatar over to the filter panel on the far left hand side of the screen. You can click on an avatar in the filter panel at any time to see only the rooms and people associated with that community.", diff --git a/test/components/views/elements/MemberEventListSummary-test.js b/test/components/views/elements/MemberEventListSummary-test.js index 09a4739f06..95f7e7999a 100644 --- a/test/components/views/elements/MemberEventListSummary-test.js +++ b/test/components/views/elements/MemberEventListSummary-test.js @@ -1,6 +1,7 @@ import expect from 'expect'; import React from 'react'; import ReactTestUtils from 'react-dom/test-utils'; +import ShallowRenderer from "react-test-renderer/shallow"; import sdk from 'matrix-react-sdk'; import * as languageHandler from '../../../../src/languageHandler'; import * as testUtils from '../../../test-utils'; @@ -112,7 +113,7 @@ describe('MemberEventListSummary', function() { threshold: 3, }; - const renderer = testUtils.getRenderer(); + const renderer = new ShallowRenderer(); renderer.render(); const result = renderer.getRenderOutput(); @@ -134,7 +135,7 @@ describe('MemberEventListSummary', function() { threshold: 3, }; - const renderer = testUtils.getRenderer(); + const renderer = new ShallowRenderer(); renderer.render(); const result = renderer.getRenderOutput(); @@ -162,7 +163,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -198,7 +199,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -246,7 +247,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -299,7 +300,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -358,7 +359,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -396,7 +397,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -447,7 +448,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -521,7 +522,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -568,7 +569,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -604,7 +605,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -632,7 +633,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -659,7 +660,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; @@ -684,7 +685,7 @@ describe('MemberEventListSummary', function() { , ); const summary = ReactTestUtils.findRenderedDOMComponentWithClass( - instance, "mx_MemberEventListSummary_summary", + instance, "mx_EventListSummary_summary", ); const summaryText = summary.innerText; diff --git a/yarn.lock b/yarn.lock index ba7fea21ba..908c653e57 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6582,6 +6582,16 @@ react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.4, react-is resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.9.0.tgz#21ca9561399aad0ff1a7701c01683e8ca981edcb" integrity sha512-tJBzzzIgnnRfEm046qRcURvwQnZVXmuCbscxUO5RWrGTXpon2d4c8mI0D8WE6ydVIm29JiLB6+RslkIvym9Rjw== +react-is@^16.9.0: + version "16.9.0" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.9.0.tgz#21ca9561399aad0ff1a7701c01683e8ca981edcb" + integrity sha512-tJBzzzIgnnRfEm046qRcURvwQnZVXmuCbscxUO5RWrGTXpon2d4c8mI0D8WE6ydVIm29JiLB6+RslkIvym9Rjw== + +react-is@^16.9.0: + version "16.9.0" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.9.0.tgz#21ca9561399aad0ff1a7701c01683e8ca981edcb" + integrity sha512-tJBzzzIgnnRfEm046qRcURvwQnZVXmuCbscxUO5RWrGTXpon2d4c8mI0D8WE6ydVIm29JiLB6+RslkIvym9Rjw== + react-lifecycles-compat@^3.0.0: version "3.0.4" resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"