Merge pull request #612 from vector-im/threading-fallback-reply

Threading fallback - PR 2 - Support rich reply
This commit is contained in:
Bruno Windels 2022-01-14 19:10:39 +01:00 committed by GitHub
commit 46c61953f6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 253 additions and 87 deletions

View file

@ -225,7 +225,7 @@ export function tests() {
return new BaseMessageTile({entry, roomVM: {room}, timeline, platform: {logger}});
}
return null;
}, (tile, params, entry) => tile?.updateEntry(entry, params));
}, (tile, params, entry) => tile?.updateEntry(entry, params, function () {}));
return tiles;
}

View file

@ -150,7 +150,7 @@ export class TilesCollection extends BaseObservableList {
const tileIdx = this._findTileIdx(entry);
const tile = this._findTileAtIdx(entry, tileIdx);
if (tile) {
const action = tile.updateEntry(entry, params);
const action = tile.updateEntry(entry, params, this._tileCreator);
if (action.shouldReplace) {
const newTile = this._tileCreator(entry);
if (newTile) {

View file

@ -34,8 +34,7 @@ const baseUrl = 'https://matrix.to';
const linkPrefix = `${baseUrl}/#/`;
class Deserializer {
constructor(result, mediaRepository, allowReplies) {
this.allowReplies = allowReplies;
constructor(result, mediaRepository) {
this.result = result;
this.mediaRepository = mediaRepository;
}
@ -289,7 +288,7 @@ class Deserializer {
}
_isAllowedNode(node) {
return this.allowReplies || !this._ensureElement(node, "MX-REPLY");
return !this._ensureElement(node, "MX-REPLY");
}
_parseInlineNodes(nodes, into) {
@ -345,9 +344,9 @@ class Deserializer {
}
}
export function parseHTMLBody(platform, mediaRepository, allowReplies, html) {
export function parseHTMLBody(platform, mediaRepository, html) {
const parseResult = platform.parseHTML(html);
const deserializer = new Deserializer(parseResult, mediaRepository, allowReplies);
const deserializer = new Deserializer(parseResult, mediaRepository);
const parts = deserializer.parseAnyNodes(parseResult.rootNodes);
return new MessageBody(html, parts);
}
@ -401,8 +400,8 @@ export async function tests() {
parseHTML: (html) => new HTMLParseResult(parse(html))
};
function test(assert, input, output, replies=true) {
assert.deepEqual(parseHTMLBody(platform, null, replies, input), new MessageBody(input, output));
function test(assert, input, output) {
assert.deepEqual(parseHTMLBody(platform, null, input), new MessageBody(input, output));
}
return {
@ -500,23 +499,14 @@ export async function tests() {
];
test(assert, input, output);
},
"Replies are inserted when allowed": assert => {
const input = 'Hello, <em><mx-reply>World</mx-reply></em>!';
const output = [
new TextPart('Hello, '),
new FormatPart("em", [new TextPart('World')]),
new TextPart('!'),
];
test(assert, input, output);
},
"Replies are stripped when not allowed": assert => {
"Reply fallback is always stripped": assert => {
const input = 'Hello, <em><mx-reply>World</mx-reply></em>!';
const output = [
new TextPart('Hello, '),
new FormatPart("em", []),
new TextPart('!'),
];
test(assert, input, output, false);
assert.deepEqual(parseHTMLBody(platform, null, input), new MessageBody(input, output));
}
/* Doesnt work: HTML library doesn't handle <pre><code> properly.
"Text with code block": assert => {

View file

@ -24,15 +24,25 @@ export class BaseMessageTile extends SimpleTile {
this._date = this._entry.timestamp ? new Date(this._entry.timestamp) : null;
this._isContinuation = false;
this._reactions = null;
this._replyTile = null;
if (this._entry.annotations || this._entry.pendingAnnotations) {
this._updateReactions();
}
this._updateReplyTileIfNeeded(options.tilesCreator, undefined);
}
get _mediaRepository() {
return this._room.mediaRepository;
}
get permaLink() {
return `https://matrix.to/#/${encodeURIComponent(this._room.id)}/${encodeURIComponent(this._entry.id)}`;
}
get senderProfileLink() {
return `https://matrix.to/#/${encodeURIComponent(this.sender)}`;
}
get displayName() {
return this._entry.displayName || this.sender;
}
@ -82,6 +92,10 @@ export class BaseMessageTile extends SimpleTile {
return this._entry.isUnverified;
}
get isReply() {
return this._entry.isReply;
}
_getContent() {
return this._entry.content;
}
@ -102,14 +116,30 @@ export class BaseMessageTile extends SimpleTile {
}
}
updateEntry(entry, param) {
const action = super.updateEntry(entry, param);
updateEntry(entry, param, tilesCreator) {
const action = super.updateEntry(entry, param, tilesCreator);
if (action.shouldUpdate) {
this._updateReactions();
}
this._updateReplyTileIfNeeded(tilesCreator, param);
return action;
}
_updateReplyTileIfNeeded(tilesCreator, param) {
const replyEntry = this._entry.contextEntry;
if (replyEntry) {
// this is an update to contextEntry used for replyPreview
const action = this._replyTile?.updateEntry(replyEntry, param, tilesCreator);
if (action?.shouldReplace || !this._replyTile) {
this.disposeTracked(this._replyTile);
this._replyTile = tilesCreator(replyEntry);
}
if(action?.shouldUpdate) {
this._replyTile?.emitChange();
}
}
}
startReply() {
this._roomVM.startReply(this._entry);
}
@ -202,4 +232,11 @@ export class BaseMessageTile extends SimpleTile {
this._reactions.update(annotations, pendingAnnotations);
}
}
get replyTile() {
if (!this._entry.contextEventId) {
return null;
}
return this._replyTile;
}
}

View file

@ -56,4 +56,5 @@ export class BaseTextTile extends BaseMessageTile {
}
return this._messageBody;
}
}

View file

@ -18,8 +18,8 @@ import {BaseTextTile} from "./BaseTextTile.js";
import {UpdateAction} from "../UpdateAction.js";
export class EncryptedEventTile extends BaseTextTile {
updateEntry(entry, params) {
const parentResult = super.updateEntry(entry, params);
updateEntry(entry, params, tilesCreator) {
const parentResult = super.updateEntry(entry, params, tilesCreator);
// event got decrypted, recreate the tile and replace this one with it
if (entry.eventType !== "m.room.encrypted") {
// the "shape" parameter trigger tile recreation in TimelineView

View file

@ -81,8 +81,8 @@ export class GapTile extends SimpleTile {
this._siblingChanged = true;
}
updateEntry(entry, params) {
super.updateEntry(entry, params);
updateEntry(entry, params, tilesCreator) {
super.updateEntry(entry, params, tilesCreator);
if (!entry.isGap) {
return UpdateAction.Remove();
} else {

View file

@ -50,7 +50,7 @@ export class TextTile extends BaseTextTile {
_parseBody(body, format) {
let messageBody;
if (format === BodyFormat.Html) {
messageBody = parseHTMLBody(this.platform, this._mediaRepository, this._entry.isReply, body);
messageBody = parseHTMLBody(this.platform, this._mediaRepository, body);
} else {
messageBody = parsePlainBody(body);
}

View file

@ -28,8 +28,8 @@ import {EncryptionEnabledTile} from "./tiles/EncryptionEnabledTile.js";
import {MissingAttachmentTile} from "./tiles/MissingAttachmentTile.js";
export function tilesCreator(baseOptions) {
return function tilesCreator(entry, emitUpdate) {
const options = Object.assign({entry, emitUpdate}, baseOptions);
const tilesCreator = function tilesCreator(entry, emitUpdate) {
const options = Object.assign({entry, emitUpdate, tilesCreator}, baseOptions);
if (entry.isGap) {
return new GapTile(options);
} else if (entry.isPending && entry.pendingEvent.isMissingAttachments) {
@ -76,5 +76,6 @@ export function tilesCreator(baseOptions) {
return null;
}
}
}
};
return tilesCreator;
}

View file

@ -258,8 +258,8 @@ export class Timeline {
this._addLocalRelationsToNewRemoteEntries(newEntries);
this._updateEntriesFetchedFromHomeserver(newEntries);
this._moveEntryToRemoteEntries(newEntries);
this._remoteEntries.setManySorted(newEntries);
this._loadContextEntriesWhereNeeded(newEntries);
this._remoteEntries.setManySorted(newEntries);
}
/**
@ -320,22 +320,43 @@ export class Timeline {
continue;
}
const id = entry.contextEventId;
let contextEvent = this._findLoadedEventById(id);
// before looking into remoteEntries, check the entries
// that about to be added first
let contextEvent = entries.find(e => e.id === id);
if (!contextEvent) {
contextEvent = await this._getEventFromStorage(id) ?? await this._getEventFromHomeserver(id);
if (contextEvent) {
// this entry was created from storage/hs, so it's not tracked by remoteEntries
// we track them here so that we can update reply previews later
this._contextEntriesNotInTimeline.set(id, contextEvent);
}
contextEvent = this._findLoadedEventById(id);
}
if (contextEvent) {
entry.setContextEntry(contextEvent);
this._emitUpdateForEntry(entry, "contextEntry");
// we don't emit an update here, as the add or update
// that the callee will emit hasn't been emitted yet.
} else {
// we don't await here, which is not ideal,
// but one of our callers, addEntries, is not async
// so there is not much point.
// Also, we want to run the entry fetching in parallel.
this._loadContextEntryNotInTimeline(entry);
}
}
}
async _loadContextEntryNotInTimeline(entry) {
const id = entry.contextEventId;
let contextEvent = await this._getEventFromStorage(id);
if (!contextEvent) {
contextEvent = await this._getEventFromHomeserver(id);
}
if (contextEvent) {
// this entry was created from storage/hs, so it's not tracked by remoteEntries
// we track them here so that we can update reply previews later
this._contextEntriesNotInTimeline.set(id, contextEvent);
entry.setContextEntry(contextEvent);
// here, we awaited something, so from now on we do have to emit
// an update if we set the context entry.
this._emitUpdateForEntry(entry, "contextEntry");
}
}
/**
* Fetches an entry with the given event-id from localEntries, remoteEntries or contextEntriesNotInTimeline.
* @param {string} eventId event-id of the entry
@ -366,7 +387,7 @@ export class Timeline {
}
return eventEntry;
}
// tries to prepend `amount` entries to the `entries` list.
/**
* [loadAtTop description]
@ -469,6 +490,7 @@ import {EventEntry} from "./entries/EventEntry.js";
import {User} from "../../User.js";
import {PendingEvent} from "../sending/PendingEvent.js";
import {createAnnotation} from "./relations.js";
import {redactEvent} from "./common.js";
export function tests() {
const fragmentIdComparer = new FragmentIdComparer([]);
@ -742,7 +764,9 @@ export function tests() {
const entryA = new EventEntry({ event: withTextBody("foo", createEvent("m.room.message", "event_id_1", alice)) });
const entryB = new EventEntry({ event: withReply("event_id_1", createEvent("m.room.message", "event_id_2", bob)), eventIndex: 2 });
await timeline.load(new User(alice), "join", new NullLogItem());
timeline.entries.subscribe({ onAdd: () => null, });
timeline.entries.subscribe({
onAdd() {},
});
timeline.addEntries([entryA, entryB]);
assert.deepEqual(entryB.contextEntry, entryA);
},
@ -802,21 +826,22 @@ export function tests() {
"redaction of context entry triggers updates in other entries": async assert => {
const timeline = new Timeline({roomId, storage: await createMockStorage(), closeCallback: () => {},
fragmentIdComparer, pendingEvents: new ObservableArray(), clock: new MockClock(), hsApi});
const entryB = new EventEntry({ event: withReply("event_id_1", createEvent("m.room.message", "event_id_2", bob)), eventIndex: 2 });
const entryC = new EventEntry({ event: withReply("event_id_1", createEvent("m.room.message", "event_id_3", bob)), eventIndex: 3 });
const entryA = new EventEntry({ event: withTextBody("foo", createEvent("m.room.message", "event_id_1", alice)), eventIndex: 1, fragmentId: 1 });
const entryB = new EventEntry({ event: withReply("event_id_1", createEvent("m.room.message", "event_id_2", bob)), eventIndex: 2, fragmentId: 1 });
const entryC = new EventEntry({ event: withReply("event_id_1", createEvent("m.room.message", "event_id_3", bob)), eventIndex: 3, fragmentId: 1 });
await timeline.load(new User(alice), "join", new NullLogItem());
const bin = [];
timeline.entries.subscribe({
onUpdate: (index) => {
const e = timeline.remoteEntries[index];
onUpdate: (index, e) => {
bin.push(e.id);
},
onAdd: () => null,
});
timeline.addEntries([entryB, entryC]);
await poll(() => timeline._remoteEntries.array.length === 2 && timeline._contextEntriesNotInTimeline.get("event_id_1"));
const redactingEntry = new EventEntry({ event: withRedacts("event_id_1", "foo", createEvent("m.room.redaction", "event_id_3", alice)) });
timeline.addEntries([redactingEntry]);
timeline.addEntries([entryA, entryB, entryC]);
const eventAClone = JSON.parse(JSON.stringify(entryA.event));
redactEvent(withRedacts("event_id_1", "foo", createEvent("m.room.redaction", "event_id_4", alice)), eventAClone);
const redactedEntry = new EventEntry({ event: eventAClone, eventIndex: 1, fragmentId: 1 });
timeline.replaceEntries([redactedEntry]);
assert.strictEqual(bin.includes("event_id_2"), true);
assert.strictEqual(bin.includes("event_id_3"), true);
},

View file

@ -16,7 +16,6 @@ limitations under the License.
*/
// import {RecordRequester, ReplayRequester} from "./matrix/net/request/replay";
import {Client} from "../../matrix/Client.js";
import {RootViewModel} from "../../domain/RootViewModel.js";
import {createNavigation, createRouter} from "../../domain/navigation/index.js";
// Don't use a default export here, as we use multiple entries during legacy build,

View file

@ -56,7 +56,8 @@ class HTMLParseResult {
const sanitizeConfig = {
ALLOWED_URI_REGEXP: /^(?:(?:(?:f|ht)tps?|mailto|tel|callto|cid|xmpp|xxx|mxc):|[^a-z]|[a-z+.-]+(?:[^a-z+.-:]|$))/i,
ADD_TAGS: ['mx-reply']
FORBID_TAGS: ['mx-reply'],
KEEP_CONTENT: false,
}
export function parseHTML(html) {

View file

@ -49,6 +49,17 @@ limitations under the License.
margin-top: 4px;
}
.ReplyPreviewView .Timeline_message {
display: grid;
grid-template: "body" auto;
margin-left: 0;
padding: 0;
}
.ReplyPreviewView .Timeline_message:not(.continuation) {
margin-top: 0;
}
@media screen and (max-width: 800px) {
.Timeline_message {
grid-template:

View file

@ -17,7 +17,7 @@ limitations under the License.
import {TemplateView} from "../../general/TemplateView";
import {Popup} from "../../general/Popup.js";
import {Menu} from "../../general/Menu.js";
import {viewClassForEntry} from "./TimelineView"
import {viewClassForEntry} from "./common"
export class MessageComposer extends TemplateView {
constructor(viewModel) {
@ -56,7 +56,7 @@ export class MessageComposer extends TemplateView {
className: "cancel",
onClick: () => this._clearReplyingTo()
}, "Close"),
t.view(new View(rvm, false, "div"))
t.view(new View(rvm, { interactive: false }, "div"))
])
});
const input = t.div({className: "MessageComposer_input"}, [

View file

@ -14,15 +14,11 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import type {TileView} from "./common";
import {viewClassForEntry} from "./common";
import {ListView} from "../../general/ListView";
import {TemplateView, Builder} from "../../general/TemplateView";
import {IObservableValue} from "../../general/BaseUpdateView";
import {GapView} from "./timeline/GapView.js";
import {TextMessageView} from "./timeline/TextMessageView.js";
import {ImageView} from "./timeline/ImageView.js";
import {VideoView} from "./timeline/VideoView.js";
import {FileView} from "./timeline/FileView.js";
import {LocationView} from "./timeline/LocationView.js";
import {MissingAttachmentView} from "./timeline/MissingAttachmentView.js";
import {AnnouncementView} from "./timeline/AnnouncementView.js";
import {RedactedView} from "./timeline/RedactedView.js";
@ -36,27 +32,6 @@ export interface TimelineViewModel extends IObservableValue {
setVisibleTileRange(start?: SimpleTile, end?: SimpleTile);
}
type TileView = GapView | AnnouncementView | TextMessageView |
ImageView | VideoView | FileView | MissingAttachmentView | RedactedView;
type TileViewConstructor = (this: TileView, SimpleTile) => void;
export function viewClassForEntry(entry: SimpleTile): TileViewConstructor | undefined {
switch (entry.shape) {
case "gap": return GapView;
case "announcement": return AnnouncementView;
case "message":
case "message-status":
return TextMessageView;
case "image": return ImageView;
case "video": return VideoView;
case "file": return FileView;
case "location": return LocationView;
case "missing-attachment": return MissingAttachmentView;
case "redacted":
return RedactedView;
}
}
function bottom(node: HTMLElement): number {
return node.offsetTop + node.clientHeight;
}

View file

@ -0,0 +1,55 @@
/*
Copyright 2021 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 {TextMessageView} from "./timeline/TextMessageView.js";
import {ImageView} from "./timeline/ImageView.js";
import {VideoView} from "./timeline/VideoView.js";
import {FileView} from "./timeline/FileView.js";
import {LocationView} from "./timeline/LocationView.js";
import {MissingAttachmentView} from "./timeline/MissingAttachmentView.js";
import {AnnouncementView} from "./timeline/AnnouncementView.js";
import {RedactedView} from "./timeline/RedactedView.js";
import {SimpleTile} from "../../../../../domain/session/room/timeline/tiles/SimpleTile.js";
import {GapView} from "./timeline/GapView.js";
export type TileView = GapView | AnnouncementView | TextMessageView |
ImageView | VideoView | FileView | LocationView | MissingAttachmentView | RedactedView;
// TODO: this is what works for a ctor but doesn't actually check we constrain the returned ctors to the types above
type TileViewConstructor = (this: TileView, SimpleTile) => void;
export function viewClassForEntry(entry: SimpleTile): TileViewConstructor | undefined {
switch (entry.shape) {
case "gap":
return GapView;
case "announcement":
return AnnouncementView;
case "message":
case "message-status":
return TextMessageView;
case "image":
return ImageView;
case "video":
return VideoView;
case "file":
return FileView;
case "location":
return LocationView;
case "missing-attachment":
return MissingAttachmentView;
case "redacted":
return RedactedView;
}
}

View file

@ -24,14 +24,17 @@ import {Menu} from "../../../general/Menu.js";
import {ReactionsView} from "./ReactionsView.js";
export class BaseMessageView extends TemplateView {
constructor(value, interactive = true, tagName = "li") {
constructor(value, renderFlags, tagName = "li") {
super(value);
this._menuPopup = null;
this._tagName = tagName;
// TODO An enum could be nice to make code easier to read at call sites.
this._interactive = interactive;
this._renderFlags = renderFlags;
}
get _interactive() { return this._renderFlags?.interactive ?? true; }
get _isReplyPreview() { return this._renderFlags?.reply; }
render(t, vm) {
const children = [this.renderMessageBody(t, vm)];
if (this._interactive) {
@ -54,7 +57,7 @@ export class BaseMessageView extends TemplateView {
if (isContinuation && wasContinuation === false) {
li.removeChild(li.querySelector(".Timeline_messageAvatar"));
li.removeChild(li.querySelector(".Timeline_messageSender"));
} else if (!isContinuation) {
} else if (!isContinuation && !this._isReplyPreview) {
const avatar = tag.a({href: vm.memberPanelLink, className: "Timeline_messageAvatar"}, [renderStaticAvatar(vm, 30)]);
const sender = tag.div({className: `Timeline_messageSender usercolor${vm.avatarColorNumber}`}, vm.displayName);
li.insertBefore(avatar, li.firstChild);
@ -104,7 +107,7 @@ export class BaseMessageView extends TemplateView {
createMenuOptions(vm) {
const options = [];
if (vm.canReact && vm.shape !== "redacted") {
if (vm.canReact && vm.shape !== "redacted" && !vm.isPending) {
options.push(new QuickReactionsMenuOption(vm));
options.push(Menu.option(vm.i18n`Reply`, () => vm.startReply()));
}

View file

@ -0,0 +1,49 @@
/*
Copyright 2021 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 {renderStaticAvatar} from "../../../avatar";
import {TemplateView} from "../../../general/TemplateView";
import {viewClassForEntry} from "../common";
export class ReplyPreviewView extends TemplateView {
render(t, vm) {
const viewClass = viewClassForEntry(vm);
if (!viewClass) {
throw new Error(`Shape ${vm.shape} is unrecognized.`)
}
const view = new viewClass(vm, { reply: true, interactive: false });
return t.div(
{ className: "ReplyPreviewView" },
t.blockquote([
t.a({ className: "link", href: vm.permaLink }, "In reply to"),
t.a({ className: "pill", href: vm.senderProfileLink }, [
renderStaticAvatar(vm, 12, undefined, true),
vm.displayName,
]),
t.br(),
t.view(view),
])
);
}
}
export class ReplyPreviewError extends TemplateView {
render(t) {
return t.blockquote({ className: "ReplyPreviewView" }, [
t.div({ className: "Timeline_messageBody statusMessage" }, "This reply could not be found.")
]);
}
}

View file

@ -16,6 +16,7 @@ limitations under the License.
import {tag, text} from "../../../general/html";
import {BaseMessageView} from "./BaseMessageView.js";
import {ReplyPreviewError, ReplyPreviewView} from "./ReplyPreviewView.js";
export class TextMessageView extends BaseMessageView {
renderMessageBody(t, vm) {
@ -24,10 +25,27 @@ export class TextMessageView extends BaseMessageView {
className: {
"Timeline_messageBody": true,
statusMessage: vm => vm.shape === "message-status",
},
});
}
}, t.mapView(vm => vm.replyTile, replyTile => {
if (this._isReplyPreview) {
// if this._isReplyPreview = true, this is already a reply preview, don't nest replies for now.
return null;
}
else if (vm.isReply && !replyTile) {
return new ReplyPreviewError();
}
else if (replyTile) {
return new ReplyPreviewView(replyTile);
}
else {
return null;
}
}));
const shouldRemove = (element) => element?.nodeType === Node.ELEMENT_NODE && element.className !== "ReplyPreviewView";
t.mapSideEffect(vm => vm.body, body => {
while (container.lastChild) {
while (shouldRemove(container.lastChild)) {
container.removeChild(container.lastChild);
}
for (const part of body.parts) {
@ -35,6 +53,7 @@ export class TextMessageView extends BaseMessageView {
}
container.appendChild(time);
});
return container;
}
}