From eb4ff50c3cfa39463e3318167e12ac308d3956a6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 12:16:34 +0200 Subject: [PATCH 01/10] do parts creation only in PartCreator to not scatter dependencies --- .../views/elements/MessageEditor.js | 2 +- src/editor/autocomplete.js | 26 +++---- src/editor/deserialize.js | 71 +++++++++--------- src/editor/parts.js | 74 +++++++++++-------- 4 files changed, 89 insertions(+), 84 deletions(-) diff --git a/src/components/views/elements/MessageEditor.js b/src/components/views/elements/MessageEditor.js index 9faae4588b..9f5265cfd3 100644 --- a/src/components/views/elements/MessageEditor.js +++ b/src/components/views/elements/MessageEditor.js @@ -233,7 +233,7 @@ export default class MessageEditor extends React.Component { parts = editState.getSerializedParts().map(p => partCreator.deserializePart(p)); } else { // otherwise, parse the body of the event - parts = parseEvent(editState.getEvent(), room, this.context.matrixClient); + parts = parseEvent(editState.getEvent(), partCreator); } return new EditorModel( diff --git a/src/editor/autocomplete.js b/src/editor/autocomplete.js index 6cb5974729..4d35904976 100644 --- a/src/editor/autocomplete.js +++ b/src/editor/autocomplete.js @@ -15,22 +15,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {UserPillPart, RoomPillPart, PlainPart} from "./parts"; - export default class AutocompleteWrapperModel { - constructor(updateCallback, getAutocompleterComponent, updateQuery, room, client) { + constructor(updateCallback, getAutocompleterComponent, updateQuery, partCreator) { this._updateCallback = updateCallback; this._getAutocompleterComponent = getAutocompleterComponent; this._updateQuery = updateQuery; + this._partCreator = partCreator; this._query = null; - this._room = room; - this._client = client; } onEscape(e) { this._getAutocompleterComponent().onEscape(e); this._updateCallback({ - replacePart: new PlainPart(this._queryPart.text), + replacePart: this._partCreator.plain(this._queryPart.text), caretOffset: this._queryOffset, close: true, }); @@ -93,21 +90,18 @@ export default class AutocompleteWrapperModel { } _partForCompletion(completion) { - const firstChr = completion.completionId && completion.completionId[0]; + const {completionId} = completion; + const text = completion.completion; + const firstChr = completionId && completionId[0]; switch (firstChr) { case "@": { - const displayName = completion.completion; - const userId = completion.completionId; - const member = this._room.getMember(userId); - return new UserPillPart(userId, displayName, member); - } - case "#": { - const displayAlias = completion.completionId; - return new RoomPillPart(displayAlias, this._client); + return this._partCreator.userPill(text, completionId); } + case "#": + return this._partCreator.roomPill(completionId); // also used for emoji completion default: - return new PlainPart(completion.completion); + return this._partCreator.plain(text); } } } diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index 2d98bbc41a..e08c1d59d9 100644 --- a/src/editor/deserialize.js +++ b/src/editor/deserialize.js @@ -16,73 +16,68 @@ limitations under the License. */ import { MATRIXTO_URL_PATTERN } from '../linkify-matrix'; -import { PlainPart, UserPillPart, RoomPillPart, NewlinePart } from "./parts"; import { walkDOMDepthFirst } from "./dom"; const REGEX_MATRIXTO = new RegExp(MATRIXTO_URL_PATTERN); -function parseLink(a, room, client) { +function parseLink(a, partCreator) { const {href} = a; const pillMatch = REGEX_MATRIXTO.exec(href) || []; const resourceId = pillMatch[1]; // The room/user ID const prefix = pillMatch[2]; // The first character of prefix switch (prefix) { case "@": - return new UserPillPart( - resourceId, - a.textContent, - room.getMember(resourceId), - ); + return partCreator.userPill(a.textContent, resourceId); case "#": - return new RoomPillPart(resourceId, client); + return partCreator.roomPill(resourceId); default: { if (href === a.textContent) { - return new PlainPart(a.textContent); + return partCreator.plain(a.textContent); } else { - return new PlainPart(`[${a.textContent}](${href})`); + return partCreator.plain(`[${a.textContent}](${href})`); } } } } -function parseCodeBlock(n) { +function parseCodeBlock(n, partCreator) { const parts = []; const preLines = ("```\n" + n.textContent + "```").split("\n"); preLines.forEach((l, i) => { - parts.push(new PlainPart(l)); + parts.push(partCreator.plain(l)); if (i < preLines.length - 1) { - parts.push(new NewlinePart("\n")); + parts.push(partCreator.newline()); } }); return parts; } -function parseElement(n, room, client) { +function parseElement(n, partCreator) { switch (n.nodeName) { case "A": - return parseLink(n, room, client); + return parseLink(n, partCreator); case "BR": - return new NewlinePart("\n"); + return partCreator.newline(); case "EM": - return new PlainPart(`*${n.textContent}*`); + return partCreator.plain(`*${n.textContent}*`); case "STRONG": - return new PlainPart(`**${n.textContent}**`); + return partCreator.plain(`**${n.textContent}**`); case "PRE": - return parseCodeBlock(n); + return parseCodeBlock(n, partCreator); case "CODE": - return new PlainPart(`\`${n.textContent}\``); + return partCreator.plain(`\`${n.textContent}\``); case "DEL": - return new PlainPart(`${n.textContent}`); + return partCreator.plain(`${n.textContent}`); case "LI": if (n.parentElement.nodeName === "OL") { - return new PlainPart(` 1. `); + return partCreator.plain(` 1. `); } else { - return new PlainPart(` - `); + return partCreator.plain(` - `); } default: // don't textify block nodes we'll decend into if (!checkDecendInto(n)) { - return new PlainPart(n.textContent); + return partCreator.plain(n.textContent); } } } @@ -125,22 +120,22 @@ function checkIgnored(n) { return true; } -function prefixQuoteLines(isFirstNode, parts) { +function prefixQuoteLines(isFirstNode, parts, partCreator) { const PREFIX = "> "; // a newline (to append a > to) wouldn't be added to parts for the first line // if there was no content before the BLOCKQUOTE, so handle that if (isFirstNode) { - parts.splice(0, 0, new PlainPart(PREFIX)); + parts.splice(0, 0, partCreator.plain(PREFIX)); } for (let i = 0; i < parts.length; i += 1) { if (parts[i].type === "newline") { - parts.splice(i + 1, 0, new PlainPart(PREFIX)); + parts.splice(i + 1, 0, partCreator.plain(PREFIX)); i += 1; } } } -function parseHtmlMessage(html, room, client) { +function parseHtmlMessage(html, partCreator) { // no nodes from parsing here should be inserted in the document, // as scripts in event handlers, etc would be executed then. // we're only taking text, so that is fine @@ -159,13 +154,13 @@ function parseHtmlMessage(html, room, client) { const newParts = []; if (lastNode && (checkBlockNode(lastNode) || checkBlockNode(n))) { - newParts.push(new NewlinePart("\n")); + newParts.push(partCreator.newline()); } if (n.nodeType === Node.TEXT_NODE) { - newParts.push(new PlainPart(n.nodeValue)); + newParts.push(partCreator.plain(n.nodeValue)); } else if (n.nodeType === Node.ELEMENT_NODE) { - const parseResult = parseElement(n, room, client); + const parseResult = parseElement(n, partCreator); if (parseResult) { if (Array.isArray(parseResult)) { newParts.push(...parseResult); @@ -177,14 +172,14 @@ function parseHtmlMessage(html, room, client) { if (newParts.length && inQuote) { const isFirstPart = parts.length === 0; - prefixQuoteLines(isFirstPart, newParts); + prefixQuoteLines(isFirstPart, newParts, partCreator); } parts.push(...newParts); // extra newline after quote, only if there something behind it... if (lastNode && lastNode.nodeName === "BLOCKQUOTE") { - parts.push(new NewlinePart("\n")); + parts.push(partCreator.newline()); } lastNode = null; return checkDecendInto(n); @@ -205,18 +200,18 @@ function parseHtmlMessage(html, room, client) { return parts; } -export function parseEvent(event, room, client) { +export function parseEvent(event, partCreator) { const content = event.getContent(); let parts; if (content.format === "org.matrix.custom.html") { - parts = parseHtmlMessage(content.formatted_body || "", room, client); + parts = parseHtmlMessage(content.formatted_body || "", partCreator); } else { const body = content.body || ""; const lines = body.split("\n"); parts = lines.reduce((parts, line, i) => { const isLast = i === lines.length - 1; - const text = new PlainPart(line); - const newLine = !isLast && new NewlinePart("\n"); + const text = partCreator.plain(line); + const newLine = !isLast && partCreator.newline(); if (newLine) { return parts.concat(text, newLine); } else { @@ -225,7 +220,7 @@ export function parseEvent(event, room, client) { }, []); } if (content.msgtype === "m.emote") { - parts.unshift(new PlainPart("/me ")); + parts.unshift(partCreator.plain("/me ")); } return parts; } diff --git a/src/editor/parts.js b/src/editor/parts.js index a122c7ab7a..7305fb1232 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -107,7 +107,7 @@ class BasePart { } } -export class PlainPart extends BasePart { +class PlainPart extends BasePart { acceptsInsertion(chr) { return chr !== "@" && chr !== "#" && chr !== ":" && chr !== "\n"; } @@ -199,7 +199,7 @@ class PillPart extends BasePart { } } -export class NewlinePart extends BasePart { +class NewlinePart extends BasePart { acceptsInsertion(chr, i) { return (this.text.length + i) === 0 && chr === "\n"; } @@ -235,20 +235,10 @@ export class NewlinePart extends BasePart { } } -export class RoomPillPart extends PillPart { - constructor(displayAlias, client) { +class RoomPillPart extends PillPart { + constructor(displayAlias, room) { super(displayAlias, displayAlias); - this._room = this._findRoomByAlias(displayAlias, client); - } - - _findRoomByAlias(alias, client) { - if (alias[0] === '#') { - return client.getRooms().find((r) => { - return r.getAliases().includes(alias); - }); - } else { - return client.getRoom(alias); - } + this._room = room; } setAvatar(node) { @@ -270,7 +260,7 @@ export class RoomPillPart extends PillPart { } } -export class UserPillPart extends PillPart { +class UserPillPart extends PillPart { constructor(userId, displayName, member) { super(userId, displayName); this._member = member; @@ -311,7 +301,7 @@ export class UserPillPart extends PillPart { } -export class PillCandidatePart extends PlainPart { +class PillCandidatePart extends PlainPart { constructor(text, autoCompleteCreator) { super(text); this._autoCompleteCreator = autoCompleteCreator; @@ -351,8 +341,7 @@ export class PartCreator { updateCallback, getAutocompleterComponent, updateQuery, - room, - client, + this, ); }; } @@ -362,7 +351,7 @@ export class PartCreator { case "#": case "@": case ":": - return new PillCandidatePart("", this._autoCompleteCreator); + return this.pillCandidate(""); case "\n": return new NewlinePart(); default: @@ -371,24 +360,51 @@ export class PartCreator { } createDefaultPart(text) { - return new PlainPart(text); + return this.plain(text); } deserializePart(part) { switch (part.type) { case "plain": - return new PlainPart(part.text); + return this.plain(part.text); case "newline": - return new NewlinePart(part.text); + return this.newline(); case "pill-candidate": - return new PillCandidatePart(part.text, this._autoCompleteCreator); + return this.pillCandidate(part.text); case "room-pill": - return new RoomPillPart(part.text, this._client); - case "user-pill": { - const member = this._room.getMember(part.userId); - return new UserPillPart(part.userId, part.text, member); - } + return this.roomPill(part.text); + case "user-pill": + return this.userPill(part.text, part.userId); } } + + plain(text) { + return new PlainPart(text); + } + + newline() { + return new NewlinePart("\n"); + } + + pillCandidate(text) { + return new PillCandidatePart(text, this._autoCompleteCreator); + } + + roomPill(alias) { + let room; + if (alias[0] === '#') { + room = this._client.getRooms().find((r) => { + return r.getAliases().includes(alias); + }); + } else { + room = this._client.getRoom(alias); + } + return new RoomPillPart(alias, room); + } + + userPill(displayName, userId) { + const member = this._room.getMember(userId); + return new UserPillPart(userId, displayName, member); + } } From dfec5058c58ff7c4cfcd959eab9946b29f6cdef8 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 18:24:18 +0200 Subject: [PATCH 02/10] support creating @room pills in partcreator --- src/editor/parts.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/editor/parts.js b/src/editor/parts.js index 7305fb1232..d13a94253f 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -260,6 +260,13 @@ class RoomPillPart extends PillPart { } } +class AtRoomPillPart extends RoomPillPart { + get type() { + return "at-room-pill"; + } +} + + class UserPillPart extends PillPart { constructor(userId, displayName, member) { super(userId, displayName); @@ -402,6 +409,10 @@ export class PartCreator { return new RoomPillPart(alias, room); } + atRoomPill(text) { + return new AtRoomPillPart(text, this._room); + } + userPill(displayName, userId) { const member = this._room.getMember(userId); return new UserPillPart(userId, displayName, member); From 63b11f50015cc327dd735230e3a269dd2e363d67 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 18:24:36 +0200 Subject: [PATCH 03/10] (de)serialize at-room-pills just like pill-candidate (no html needed) --- src/editor/serialize.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/editor/serialize.js b/src/editor/serialize.js index 73fbbe5d01..876130074c 100644 --- a/src/editor/serialize.js +++ b/src/editor/serialize.js @@ -24,6 +24,7 @@ export function mdSerialize(model) { return html + "\n"; case "plain": case "pill-candidate": + case "at-room-pill": return html + part.text; case "room-pill": case "user-pill": @@ -47,6 +48,7 @@ export function textSerialize(model) { return text + "\n"; case "plain": case "pill-candidate": + case "at-room-pill": return text + part.text; case "room-pill": case "user-pill": @@ -58,13 +60,11 @@ export function textSerialize(model) { export function requiresHtml(model) { return model.parts.some(part => { switch (part.type) { - case "newline": - case "plain": - case "pill-candidate": - return false; case "room-pill": case "user-pill": return true; + default: + return false; } }); } From 65d56d1490e3adc0f93d1fb0eb0d74f84a6e20b9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 18:25:02 +0200 Subject: [PATCH 04/10] transform @room to AtRoomPill while deserializing html to md --- src/editor/deserialize.js | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index e08c1d59d9..200f3eb885 100644 --- a/src/editor/deserialize.js +++ b/src/editor/deserialize.js @@ -20,6 +20,21 @@ import { walkDOMDepthFirst } from "./dom"; const REGEX_MATRIXTO = new RegExp(MATRIXTO_URL_PATTERN); +function parseAtRoomMentions(text, partCreator) { + const ATROOM = "@room"; + const parts = []; + text.split(ATROOM).forEach((textPart, i, arr) => { + if (textPart.length) { + parts.push(partCreator.plain(textPart)); + } + const isLast = i === arr.length - 1; + if (!isLast) { + parts.push(partCreator.atRoomPill(ATROOM)); + } + }); + return parts; +} + function parseLink(a, partCreator) { const {href} = a; const pillMatch = REGEX_MATRIXTO.exec(href) || []; @@ -158,7 +173,7 @@ function parseHtmlMessage(html, partCreator) { } if (n.nodeType === Node.TEXT_NODE) { - newParts.push(partCreator.plain(n.nodeValue)); + newParts.push(...parseAtRoomMentions(n.nodeValue, partCreator)); } else if (n.nodeType === Node.ELEMENT_NODE) { const parseResult = parseElement(n, partCreator); if (parseResult) { @@ -210,13 +225,11 @@ export function parseEvent(event, partCreator) { const lines = body.split("\n"); parts = lines.reduce((parts, line, i) => { const isLast = i === lines.length - 1; - const text = partCreator.plain(line); - const newLine = !isLast && partCreator.newline(); - if (newLine) { - return parts.concat(text, newLine); - } else { - return parts.concat(text); + const newParts = parseAtRoomMentions(line, partCreator); + if (!isLast) { + newParts.push(partCreator.newline()); } + return parts.concat(newParts); }, []); } if (content.msgtype === "m.emote") { From 78971f168f6212853956b0e8f0db3135ca4bef90 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 18:25:43 +0200 Subject: [PATCH 05/10] create AtRoomPill when autocomplete returns @room --- src/editor/autocomplete.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/editor/autocomplete.js b/src/editor/autocomplete.js index 4d35904976..2aedf8d7f5 100644 --- a/src/editor/autocomplete.js +++ b/src/editor/autocomplete.js @@ -95,7 +95,11 @@ export default class AutocompleteWrapperModel { const firstChr = completionId && completionId[0]; switch (firstChr) { case "@": { - return this._partCreator.userPill(text, completionId); + if (completionId === "@room") { + return this._partCreator.atRoomPill(completionId); + } else { + return this._partCreator.userPill(text, completionId); + } } case "#": return this._partCreator.roomPill(completionId); From 497ba1ecd443619246c2764ba9bd828a4e62ac8d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 18:26:01 +0200 Subject: [PATCH 06/10] prevent @room pills being applied multiple times when rerendering --- src/components/views/messages/TextualBody.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/views/messages/TextualBody.js b/src/components/views/messages/TextualBody.js index 6f480b8d3c..d76956d193 100644 --- a/src/components/views/messages/TextualBody.js +++ b/src/components/views/messages/TextualBody.js @@ -214,7 +214,13 @@ module.exports = React.createClass({ // update the current node with one that's now taken its place node = pillContainer; } - } else if (node.nodeType === Node.TEXT_NODE) { + } else if ( + node.nodeType === Node.TEXT_NODE && + // as applying pills happens outside of react, make sure we're not doubly + // applying @room pills here, as a rerender with the same content won't touch the DOM + // to clear the pills from the last run of pillifyLinks + !node.parentElement.classList.contains("mx_AtRoomPill") + ) { const Pill = sdk.getComponent('elements.Pill'); let currentTextNode = node; From d9e62b54fce0c7f2ef8c1ca264dc1c0ff236451d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 14 Jun 2019 18:42:30 +0200 Subject: [PATCH 07/10] also support deserializing at-room-pill when transferring editor state --- src/editor/parts.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/editor/parts.js b/src/editor/parts.js index d13a94253f..67d36e6660 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -376,6 +376,8 @@ export class PartCreator { return this.plain(part.text); case "newline": return this.newline(); + case "at-room-pill": + return this.atRoomPill(part.text); case "pill-candidate": return this.pillCandidate(part.text); case "room-pill": From 47579f37e8986ed4594c52642b68a8d5e2368ddb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Jun 2019 08:40:58 +0200 Subject: [PATCH 08/10] clarify why its always safe to not append @room at end while parsing --- src/editor/deserialize.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index 200f3eb885..9307812bad 100644 --- a/src/editor/deserialize.js +++ b/src/editor/deserialize.js @@ -27,6 +27,9 @@ function parseAtRoomMentions(text, partCreator) { if (textPart.length) { parts.push(partCreator.plain(textPart)); } + // it's safe to never append @room after the last text + // as split will report an empty string at the end if + // `text` ended in @room. const isLast = i === arr.length - 1; if (!isLast) { parts.push(partCreator.atRoomPill(ATROOM)); From 3119feaa17036acaf5c788c0bc6635aeb6d43f1a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Jun 2019 09:48:25 +0200 Subject: [PATCH 09/10] whitespace --- src/editor/parts.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/editor/parts.js b/src/editor/parts.js index 67d36e6660..dc2c1e69a2 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -266,7 +266,6 @@ class AtRoomPillPart extends RoomPillPart { } } - class UserPillPart extends PillPart { constructor(userId, displayName, member) { super(userId, displayName); From 1db505c6674b51e040023b925aa468c332437f75 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 18 Jun 2019 09:50:31 +0200 Subject: [PATCH 10/10] comment typo --- src/editor/deserialize.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/deserialize.js b/src/editor/deserialize.js index 9307812bad..7e4e82affe 100644 --- a/src/editor/deserialize.js +++ b/src/editor/deserialize.js @@ -27,7 +27,7 @@ function parseAtRoomMentions(text, partCreator) { if (textPart.length) { parts.push(partCreator.plain(textPart)); } - // it's safe to never append @room after the last text + // it's safe to never append @room after the last textPart // as split will report an empty string at the end if // `text` ended in @room. const isLast = i === arr.length - 1;