From 9cec3828650490018e674bb4d79ebe97768ce3d4 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 27 Feb 2021 22:46:38 -0700 Subject: [PATCH 1/6] Change sending->sent state to match new designs For https://github.com/vector-im/element-web/issues/16424 --- res/css/views/rooms/_EventTile.scss | 39 ++++++++++++------- res/img/element-icons/circle-sending.svg | 3 ++ res/img/element-icons/circle-sent.svg | 4 ++ res/themes/dark/css/_dark.scss | 3 -- res/themes/legacy-dark/css/_legacy-dark.scss | 3 -- .../legacy-light/css/_legacy-light.scss | 2 - res/themes/light/css/_light.scss | 2 - .../views/messages/EditHistoryMessage.js | 1 + src/components/views/rooms/EventTile.js | 21 +++++++++- 9 files changed, 54 insertions(+), 24 deletions(-) create mode 100644 res/img/element-icons/circle-sending.svg create mode 100644 res/img/element-icons/circle-sent.svg diff --git a/res/css/views/rooms/_EventTile.scss b/res/css/views/rooms/_EventTile.scss index 5841cf2853..028d9a7556 100644 --- a/res/css/views/rooms/_EventTile.scss +++ b/res/css/views/rooms/_EventTile.scss @@ -213,23 +213,36 @@ $left-gutter: 64px; color: $accent-fg-color; } -.mx_EventTile_encrypting { - color: $event-encrypting-color !important; -} - -.mx_EventTile_sending { - color: $event-sending-color; -} - -.mx_EventTile_sending .mx_UserPill, -.mx_EventTile_sending .mx_RoomPill { - opacity: 0.5; -} - .mx_EventTile_notSent { color: $event-notsent-color; } +.mx_EventTile_receiptSent, +.mx_EventTile_receiptSending { + // We don't use `position: relative` on the element because then it won't line + // up with the other read receipts + + &::before { + background-color: $tertiary-fg-color; + mask-repeat: no-repeat; + mask-position: center; + mask-size: 14px; + width: 14px; + height: 14px; + content: ''; + position: absolute; + top: 0; + left: 0; + right: 0; + } +} +.mx_EventTile_receiptSent::before { + mask-image: url('$(res)/img/element-icons/circle-sent.svg'); +} +.mx_EventTile_receiptSending::before { + mask-image: url('$(res)/img/element-icons/circle-sending.svg'); +} + .mx_EventTile_contextual { opacity: 0.4; } diff --git a/res/img/element-icons/circle-sending.svg b/res/img/element-icons/circle-sending.svg new file mode 100644 index 0000000000..2d15a0f716 --- /dev/null +++ b/res/img/element-icons/circle-sending.svg @@ -0,0 +1,3 @@ + + + diff --git a/res/img/element-icons/circle-sent.svg b/res/img/element-icons/circle-sent.svg new file mode 100644 index 0000000000..04a00ceff7 --- /dev/null +++ b/res/img/element-icons/circle-sent.svg @@ -0,0 +1,4 @@ + + + + diff --git a/res/themes/dark/css/_dark.scss b/res/themes/dark/css/_dark.scss index a878aa3cdd..344f012d45 100644 --- a/res/themes/dark/css/_dark.scss +++ b/res/themes/dark/css/_dark.scss @@ -137,9 +137,6 @@ $panel-divider-color: transparent; $widget-menu-bar-bg-color: $header-panel-bg-color; $widget-body-bg-color: rgba(141, 151, 165, 0.2); -// event tile lifecycle -$event-sending-color: $text-secondary-color; - // event redaction $event-redacted-fg-color: #606060; $event-redacted-border-color: #000000; diff --git a/res/themes/legacy-dark/css/_legacy-dark.scss b/res/themes/legacy-dark/css/_legacy-dark.scss index 3e3c299af9..ca3ead9ea8 100644 --- a/res/themes/legacy-dark/css/_legacy-dark.scss +++ b/res/themes/legacy-dark/css/_legacy-dark.scss @@ -132,9 +132,6 @@ $panel-divider-color: $header-panel-border-color; $widget-menu-bar-bg-color: $header-panel-bg-color; $widget-body-bg-color: #1A1D23; -// event tile lifecycle -$event-sending-color: $text-secondary-color; - // event redaction $event-redacted-fg-color: #606060; $event-redacted-border-color: #000000; diff --git a/res/themes/legacy-light/css/_legacy-light.scss b/res/themes/legacy-light/css/_legacy-light.scss index a740ba155c..fa44c128d0 100644 --- a/res/themes/legacy-light/css/_legacy-light.scss +++ b/res/themes/legacy-light/css/_legacy-light.scss @@ -222,8 +222,6 @@ $widget-body-bg-color: #fff; $yellow-background: #fff8e3; // event tile lifecycle -$event-encrypting-color: #abddbc; -$event-sending-color: #ddd; $event-notsent-color: #f44; $event-highlight-fg-color: $warning-color; diff --git a/res/themes/light/css/_light.scss b/res/themes/light/css/_light.scss index 1c89d83c01..ca52d0dcfa 100644 --- a/res/themes/light/css/_light.scss +++ b/res/themes/light/css/_light.scss @@ -222,8 +222,6 @@ $widget-body-bg-color: #FFF; $yellow-background: #fff8e3; // event tile lifecycle -$event-encrypting-color: #abddbc; -$event-sending-color: #ddd; $event-notsent-color: #f44; $event-highlight-fg-color: $warning-color; diff --git a/src/components/views/messages/EditHistoryMessage.js b/src/components/views/messages/EditHistoryMessage.js index df27773a40..6c420a16fc 100644 --- a/src/components/views/messages/EditHistoryMessage.js +++ b/src/components/views/messages/EditHistoryMessage.js @@ -158,6 +158,7 @@ export default class EditHistoryMessage extends React.PureComponent { const isSending = (['sending', 'queued', 'encrypting'].indexOf(this.state.sendStatus) !== -1); const classes = classNames({ "mx_EventTile": true, + // Note: we keep these sending state classes for tests, not for our styles "mx_EventTile_sending": isSending, "mx_EventTile_notSent": this.state.sendStatus === 'not_sent', }); diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index 87fb190678..9110316850 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -454,8 +454,26 @@ export default class EventTile extends React.Component { }; getReadAvatars() { - // return early if there are no read receipts + // return early if there are no read receipts, with our message state if applicable if (!this.props.readReceipts || this.props.readReceipts.length === 0) { + const room = this.context.getRoom(this.props.mxEvent.getRoomId()); + const myUserId = MatrixClientPeg.get().getUserId(); + if (this.props.mxEvent.getSender() === myUserId && room) { + // We only search for the most recent 50 events because surely someone will have + // sent *something* in that time, even if it is a membership event or something. + const readUsers = room.getUsersWhoHaveRead(this.props.mxEvent, 50); + const hasBeenRead = readUsers.length === 0 || readUsers.some(u => u !== myUserId); + console.log(room.getUsersReadUpTo(this.props.mxEvent)); + let receipt = null; + if (!this.props.eventSendStatus || this.props.eventSendStatus === 'sent') { + if (!hasBeenRead) { + receipt = ; + } + } else { + receipt = ; + } + return {receipt}; + } return (); } @@ -692,6 +710,7 @@ export default class EventTile extends React.Component { mx_EventTile_isEditing: isEditing, mx_EventTile_info: isInfoMessage, mx_EventTile_12hr: this.props.isTwelveHour, + // Note: we keep these sending state classes for tests, not for our styles mx_EventTile_encrypting: this.props.eventSendStatus === 'encrypting', mx_EventTile_sending: !isEditing && isSending, mx_EventTile_notSent: this.props.eventSendStatus === 'not_sent', From db8978580c80f86a91dd2783a2415065dc49e57b Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 1 Mar 2021 16:21:04 -0700 Subject: [PATCH 2/6] Improve special read receipt checking See comments in code --- src/components/views/rooms/EventTile.js | 133 ++++++++++++++++++++---- 1 file changed, 114 insertions(+), 19 deletions(-) diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index 9110316850..01e932dd3a 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -264,6 +264,79 @@ export default class EventTile extends React.Component { this._tile = createRef(); this._replyThread = createRef(); + + // Throughout the component we manage a read receipt listener to see if our tile still + // qualifies for a "sent" or "sending" state (based on their relevant conditions). We + // don't want to over-subscribe to the read receipt events being fired, so we use a flag + // to determine if we've already subscribed and use a combination of other flags to find + // out if we should even be subscribed at all. + this._isListeningForReceipts = false; + } + + /** + * When true, the tile qualifies for some sort of special read receipt. This could be a 'sending' + * or 'sent' receipt, for example. + * @returns {boolean} + * @private + */ + get _isEligibleForSpecialReceipt() { + // First, if there are other read receipts then just short-circuit this. + if (this.props.readReceipts && this.props.readReceipts.length > 0) return false; + if (!this.props.mxEvent) return false; + + // Sanity check (should never happen, but we shouldn't explode if it does) + const room = this.context.getRoom(this.props.mxEvent.getRoomId()); + if (!room) return false; + + // Quickly check to see if the event was sent by us. If it wasn't, it won't qualify for + // special read receipts. + const myUserId = MatrixClientPeg.get().getUserId(); + if (this.props.mxEvent.getSender() !== myUserId) return false; + + // Finally, determine if the type is relevant to the user. This notably excludes state + // events and pretty much anything that can't be sent by the composer as a message. For + // those we rely on local echo giving the impression of things changing, and expect them + // to be quick. + const simpleSendableEvents = [EventType.Sticker, EventType.RoomMessage, EventType.RoomMessageEncrypted]; + if (!simpleSendableEvents.includes(this.props.mxEvent.getType())) return false; + + // Default case + return true; + } + + get _shouldShowSentReceipt() { + // If we're not even eligible, don't show the receipt. + if (!this._isEligibleForSpecialReceipt) return false; + + // Check to make sure the sending state is appropriate. A null/undefined send status means + // that the message is 'sent', so we're just double checking that it's explicitly not sent. + if (this.props.eventSendStatus && this.props.eventSendStatus !== 'sent') return false; + + // No point in doing the complex math if we're not going to even show this special receipt. + if (this._shouldShowSendingReceipt) return false; + + // Next we check to see if any newer events have read receipts. If they do then we don't + // show our special state - the user already has feedback about their message. We only + // search for the most recent 50 events because surely someone will have sent *something* + // in that time, even if it is a membership event or something. + const room = this.context.getRoom(this.props.mxEvent.getRoomId()); + const myUserId = MatrixClientPeg.get().getUserId(); + const readUsers = room.getUsersWhoHaveRead(this.props.mxEvent, 50); + const hasBeenRead = readUsers.length === 0 || readUsers.some(u => u !== myUserId); + return !hasBeenRead; + } + + get _shouldShowSendingReceipt() { + // If we're not even eligible, don't show the receipt. + if (!this._isEligibleForSpecialReceipt) return false; + + // Check the event send status to see if we are pending. Null/undefined status means the + // message was sent, so check for that and 'sent' explicitly. + if (!this.props.eventSendStatus || this.props.eventSendStatus === 'sent') return false; + + // Default to showing - there's no other event properties/behaviours we care about at + // this point. + return true; } // TODO: [REACT-WARNING] Move into constructor @@ -281,6 +354,11 @@ export default class EventTile extends React.Component { if (this.props.showReactions) { this.props.mxEvent.on("Event.relationsCreated", this._onReactionsCreated); } + + if (this._shouldShowSentReceipt || this._shouldShowSendingReceipt) { + client.on("Room.receipt", this._onRoomReceipt); + this._isListeningForReceipts = true; + } } // TODO: [REACT-WARNING] Replace with appropriate lifecycle event @@ -305,12 +383,40 @@ export default class EventTile extends React.Component { const client = this.context; client.removeListener("deviceVerificationChanged", this.onDeviceVerificationChanged); client.removeListener("userTrustStatusChanged", this.onUserVerificationChanged); + client.removeListener("Room.receipt", this._onRoomReceipt); + this._isListeningForReceipts = false; this.props.mxEvent.removeListener("Event.decrypted", this._onDecrypted); if (this.props.showReactions) { this.props.mxEvent.removeListener("Event.relationsCreated", this._onReactionsCreated); } } + componentDidUpdate(prevProps, prevState, snapshot) { + // If we're not listening for receipts and expect to be, register a listener. + if (!this._isListeningForReceipts && (this._shouldShowSentReceipt || this._shouldShowSendingReceipt)) { + this.context.on("Room.receipt", this._onRoomReceipt); + this._isListeningForReceipts = true; + } + } + + _onRoomReceipt = (ev, room) => { + // ignore events for other rooms + const tileRoom = MatrixClientPeg.get().getRoom(this.props.mxEvent.getRoomId()); + if (room !== tileRoom) return; + + if (!this._shouldShowSentReceipt && !this._shouldShowSendingReceipt && !this._isListeningForReceipts) { + return; + } + + this.forceUpdate(() => { + // Per elsewhere in this file, we can remove the listener once we will have no further purpose for it. + if (!this._shouldShowSentReceipt && !this._shouldShowSendingReceipt) { + this.context.removeListener("Room.receipt", this._onRoomReceipt); + this._isListeningForReceipts = false; + } + }); + }; + /** called when the event is decrypted after we show it. */ _onDecrypted = () => { @@ -454,26 +560,15 @@ export default class EventTile extends React.Component { }; getReadAvatars() { - // return early if there are no read receipts, with our message state if applicable + if (this._shouldShowSentReceipt) { + return ; + } + if (this._shouldShowSendingReceipt) { + return ; + } + + // return early if there are no read receipts if (!this.props.readReceipts || this.props.readReceipts.length === 0) { - const room = this.context.getRoom(this.props.mxEvent.getRoomId()); - const myUserId = MatrixClientPeg.get().getUserId(); - if (this.props.mxEvent.getSender() === myUserId && room) { - // We only search for the most recent 50 events because surely someone will have - // sent *something* in that time, even if it is a membership event or something. - const readUsers = room.getUsersWhoHaveRead(this.props.mxEvent, 50); - const hasBeenRead = readUsers.length === 0 || readUsers.some(u => u !== myUserId); - console.log(room.getUsersReadUpTo(this.props.mxEvent)); - let receipt = null; - if (!this.props.eventSendStatus || this.props.eventSendStatus === 'sent') { - if (!hasBeenRead) { - receipt = ; - } - } else { - receipt = ; - } - return {receipt}; - } return (); } From 08d35073de93367b059b23c1581b8eb765822754 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 2 Mar 2021 11:04:12 -0700 Subject: [PATCH 3/6] Improve commentary --- src/components/views/messages/EditHistoryMessage.js | 2 +- src/components/views/rooms/EventTile.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/views/messages/EditHistoryMessage.js b/src/components/views/messages/EditHistoryMessage.js index 6c420a16fc..0967be937a 100644 --- a/src/components/views/messages/EditHistoryMessage.js +++ b/src/components/views/messages/EditHistoryMessage.js @@ -158,7 +158,7 @@ export default class EditHistoryMessage extends React.PureComponent { const isSending = (['sending', 'queued', 'encrypting'].indexOf(this.state.sendStatus) !== -1); const classes = classNames({ "mx_EventTile": true, - // Note: we keep these sending state classes for tests, not for our styles + // Note: we keep the `sending` state class for tests, not for our styles "mx_EventTile_sending": isSending, "mx_EventTile_notSent": this.state.sendStatus === 'not_sent', }); diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index 01e932dd3a..b4192fc8d3 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -408,6 +408,8 @@ export default class EventTile extends React.Component { return; } + // We force update because we have no state or prop changes to queue up, instead relying on + // the getters we use here to determine what needs rendering. this.forceUpdate(() => { // Per elsewhere in this file, we can remove the listener once we will have no further purpose for it. if (!this._shouldShowSentReceipt && !this._shouldShowSendingReceipt) { @@ -805,8 +807,7 @@ export default class EventTile extends React.Component { mx_EventTile_isEditing: isEditing, mx_EventTile_info: isInfoMessage, mx_EventTile_12hr: this.props.isTwelveHour, - // Note: we keep these sending state classes for tests, not for our styles - mx_EventTile_encrypting: this.props.eventSendStatus === 'encrypting', + // Note: we keep the `sending` state class for tests, not for our styles mx_EventTile_sending: !isEditing && isSending, mx_EventTile_notSent: this.props.eventSendStatus === 'not_sent', mx_EventTile_highlight: this.props.tileShape === 'notif' ? false : this.shouldHighlight(), From ca63e937d396387254c91d225735b288cc2c6701 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 5 Mar 2021 10:56:13 -0700 Subject: [PATCH 4/6] Fix up logic to show sent receipts correctly --- src/components/structures/MessagePanel.js | 10 ++++++++ src/components/views/rooms/EventTile.js | 29 +++++++++++++---------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 161227a139..dab1c953a9 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -595,6 +595,15 @@ export default class MessagePanel extends React.Component { const readReceipts = this._readReceiptsByEvent[eventId]; + let isLastSuccessful = false; + const isSentState = s => !s || s === 'sent'; + const isSent = isSentState(mxEv.getAssociatedStatus()) + if (!nextEvent && isSent) { + isLastSuccessful = true; + } else if (nextEvent && isSent && !isSentState(nextEvent.getAssociatedStatus())) { + isLastSuccessful = true; + } + // use txnId as key if available so that we don't remount during sending ret.push(
  • u !== myUserId); - return !hasBeenRead; + if (receipts.some(r => r.userId !== myUserId)) return false; + + // Finally, we should show a receipt. + return true; } get _shouldShowSendingReceipt() { From f87f2b11ef945d7d78e8912e6df9b75514f10764 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 5 Mar 2021 11:05:56 -0700 Subject: [PATCH 5/6] Appease the linter --- src/components/structures/MessagePanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index dab1c953a9..c1ccf61470 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -597,7 +597,7 @@ export default class MessagePanel extends React.Component { let isLastSuccessful = false; const isSentState = s => !s || s === 'sent'; - const isSent = isSentState(mxEv.getAssociatedStatus()) + const isSent = isSentState(mxEv.getAssociatedStatus()); if (!nextEvent && isSent) { isLastSuccessful = true; } else if (nextEvent && isSent && !isSentState(nextEvent.getAssociatedStatus())) { From fdea45ad74905346744e5a4c5c77bd3ddf64d3f1 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 8 Mar 2021 08:51:57 -0700 Subject: [PATCH 6/6] Add a quick sender check to isLastSuccessful --- src/components/structures/MessagePanel.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index c1ccf61470..9deda54bee 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -604,6 +604,10 @@ export default class MessagePanel extends React.Component { isLastSuccessful = true; } + // We only want to consider "last successful" if the event is sent by us, otherwise of course + // it's successful: we received it. + isLastSuccessful = isLastSuccessful && mxEv.getSender() === MatrixClientPeg.get().getUserId(); + // use txnId as key if available so that we don't remount during sending ret.push(