Merge pull request #6353 from SimonBrandner/feature/improved-composer

Improve handling of pills in the composer

res/css/views/rooms/_BasicMessageComposer.scss

@@ -65,6 +65,14 @@ limitations under the License.
                     font-size: $font-10-4px;
                 }
             }
+
+            span.mx_UserPill {
+                cursor: pointer;
+            }
+
+            span.mx_RoomPill {
+                cursor: default;
+            }
         }
 
         &.mx_BasicMessageComposer_input_disabled {

src/ActiveRoomObserver.ts

@@ -14,8 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import RoomViewStore from './stores/RoomViewStore';
 import { EventSubscription } from 'fbemitter';
+import RoomViewStore from './stores/RoomViewStore';
 
 type Listener = (isActive: boolean) => void;
 

src/components/views/rooms/BasicMessageComposer.tsx

@@ -31,7 +31,7 @@ import {
 } from '../../../editor/operations';
 import { getCaretOffsetAndText, getRangeForSelection } from '../../../editor/dom';
 import Autocomplete, { generateCompletionDomId } from '../rooms/Autocomplete';
-import { getAutoCompleteCreator } from '../../../editor/parts';
+import { getAutoCompleteCreator, Type } from '../../../editor/parts';
 import { parseEvent, parsePlainTextMessage } from '../../../editor/deserialize';
 import { renderModel } from '../../../editor/render';
 import TypingStore from "../../../stores/TypingStore";
@@ -169,7 +169,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
         range.expandBackwardsWhile((index, offset) => {
             const part = model.parts[index];
             n -= 1;
-            return n >= 0 && (part.type === "plain" || part.type === "pill-candidate");
+            return n >= 0 && (part.type === Type.Plain || part.type === Type.PillCandidate);
         });
         const emoticonMatch = REGEX_EMOTICON_WHITESPACE.exec(range.text);
         if (emoticonMatch) {
@@ -541,6 +541,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
             handled = true;
         } else if (event.key === Key.BACKSPACE || event.key === Key.DELETE) {
             this.formatBarRef.current.hide();
+            handled = this.fakeDeletion(event.key === Key.BACKSPACE);
         }
 
         if (handled) {
@@ -549,6 +550,29 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
         }
     };
 
+    /**
+     * Because pills have contentEditable="false" there is no event emitted when
+     * the user tries to delete them. Therefore we need to fake what would
+     * normally happen
+     * @param direction in which to delete
+     * @returns handled
+     */
+    private fakeDeletion(backward: boolean): boolean {
+        const selection = document.getSelection();
+        // Use the default handling for ranges
+        if (selection.type === "Range") return false;
+
+        this.modifiedFlag = true;
+        const { caret, text } = getCaretOffsetAndText(this.editorRef.current, selection);
+
+        // Do the deletion itself
+        if (backward) caret.offset--;
+        const newText = text.slice(0, caret.offset) + text.slice(caret.offset + 1);
+
+        this.props.model.update(newText, backward ? "deleteContentBackward" : "deleteContentForward", caret);
+        return true;
+    }
+
     private async tabCompleteName(): Promise<void> {
         try {
             await new Promise<void>(resolve => this.setState({ showVisualBell: false }, resolve));
@@ -558,9 +582,9 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
             const range = model.startRange(position);
             range.expandBackwardsWhile((index, offset, part) => {
                 return part.text[offset] !== " " && part.text[offset] !== "+" && (
-                    part.type === "plain" ||
+                    part.type === Type.Plain ||
-                    part.type === "pill-candidate" ||
+                    part.type === Type.PillCandidate ||
-                    part.type === "command"
+                    part.type === Type.Command
                 );
             });
             const { partCreator } = model;

src/components/views/rooms/EditMessageComposer.tsx

@@ -25,7 +25,7 @@ import { getCaretOffsetAndText } from '../../../editor/dom';
 import { htmlSerializeIfNeeded, textSerialize, containsEmote, stripEmoteCommand } from '../../../editor/serialize';
 import { findEditableEvent } from '../../../utils/EventUtils';
 import { parseEvent } from '../../../editor/deserialize';
-import { CommandPartCreator, Part, PartCreator } from '../../../editor/parts';
+import { CommandPartCreator, Part, PartCreator, Type } from '../../../editor/parts';
 import EditorStateTransfer from '../../../utils/EditorStateTransfer';
 import BasicMessageComposer from "./BasicMessageComposer";
 import MatrixClientContext from "../../../contexts/MatrixClientContext";
@@ -242,12 +242,12 @@ export default class EditMessageComposer extends React.Component<IProps, IState>
         const parts = this.model.parts;
         const firstPart = parts[0];
         if (firstPart) {
-            if (firstPart.type === "command" && firstPart.text.startsWith("/") && !firstPart.text.startsWith("//")) {
+            if (firstPart.type === Type.Command && firstPart.text.startsWith("/") && !firstPart.text.startsWith("//")) {
                 return true;
             }
 
             if (firstPart.text.startsWith("/") && !firstPart.text.startsWith("//")
-                && (firstPart.type === "plain" || firstPart.type === "pill-candidate")) {
+                && (firstPart.type === Type.Plain || firstPart.type === Type.PillCandidate)) {
                 return true;
             }
         }
@@ -268,7 +268,7 @@ export default class EditMessageComposer extends React.Component<IProps, IState>
     private getSlashCommand(): [Command, string, string] {
         const commandText = this.model.parts.reduce((text, part) => {
             // use mxid to textify user pills in a command
-            if (part.type === "user-pill") {
+            if (part.type === Type.UserPill) {
                 return text + part.resourceId;
             }
             return text + part.text;

src/components/views/rooms/SendMessageComposer.tsx

@@ -31,7 +31,7 @@ import {
     textSerialize,
     unescapeMessage,
 } from '../../../editor/serialize';
-import { CommandPartCreator, Part, PartCreator, SerializedPart } from '../../../editor/parts';
+import { CommandPartCreator, Part, PartCreator, SerializedPart, Type } from '../../../editor/parts';
 import BasicMessageComposer from "./BasicMessageComposer";
 import ReplyThread from "../elements/ReplyThread";
 import { findEditableEvent } from '../../../utils/EventUtils';
@@ -240,14 +240,14 @@ export default class SendMessageComposer extends React.Component<IProps> {
         const parts = this.model.parts;
         const firstPart = parts[0];
         if (firstPart) {
-            if (firstPart.type === "command" && firstPart.text.startsWith("/") && !firstPart.text.startsWith("//")) {
+            if (firstPart.type === Type.Command && firstPart.text.startsWith("/") && !firstPart.text.startsWith("//")) {
                 return true;
             }
             // be extra resilient when somehow the AutocompleteWrapperModel or
             // CommandPartCreator fails to insert a command part, so we don't send
             // a command as a message
             if (firstPart.text.startsWith("/") && !firstPart.text.startsWith("//")
-                && (firstPart.type === "plain" || firstPart.type === "pill-candidate")) {
+                && (firstPart.type === Type.Plain || firstPart.type === Type.PillCandidate)) {
                 return true;
             }
         }

src/editor/autocomplete.ts

@@ -43,7 +43,7 @@ export default class AutocompleteWrapperModel {
     ) {
     }
 
-    public onEscape(e: KeyboardEvent) {
+    public onEscape(e: KeyboardEvent): void {
         this.getAutocompleterComponent().onEscape(e);
         this.updateCallback({
             replaceParts: [this.partCreator.plain(this.queryPart.text)],
@@ -51,27 +51,27 @@ export default class AutocompleteWrapperModel {
         });
     }
 
-    public close() {
+    public close(): void {
         this.updateCallback({ close: true });
     }
 
-    public hasSelection() {
+    public hasSelection(): boolean {
         return this.getAutocompleterComponent().hasSelection();
     }
 
-    public hasCompletions() {
+    public hasCompletions(): boolean {
         const ac = this.getAutocompleterComponent();
         return ac && ac.countCompletions() > 0;
     }
 
-    public onEnter() {
+    public onEnter(): void {
         this.updateCallback({ close: true });
     }
 
     /**
      * If there is no current autocompletion, start one and move to the first selection.
      */
-    public async startSelection() {
+    public async startSelection(): Promise<void> {
         const acComponent = this.getAutocompleterComponent();
         if (acComponent.countCompletions() === 0) {
             // Force completions to show for the text currently entered
@@ -81,15 +81,15 @@ export default class AutocompleteWrapperModel {
         }
     }
 
-    public selectPreviousSelection() {
+    public selectPreviousSelection(): void {
         this.getAutocompleterComponent().moveSelection(-1);
     }
 
-    public selectNextSelection() {
+    public selectNextSelection(): void {
         this.getAutocompleterComponent().moveSelection(+1);
     }
 
-    public onPartUpdate(part: Part, pos: DocumentPosition) {
+    public onPartUpdate(part: Part, pos: DocumentPosition): Promise<void> {
         // cache the typed value and caret here
         // so we can restore it in onComponentSelectionChange when the value is undefined (meaning it should be the typed text)
         this.queryPart = part;
@@ -97,7 +97,7 @@ export default class AutocompleteWrapperModel {
         return this.updateQuery(part.text);
     }
 
-    public onComponentSelectionChange(completion: ICompletion) {
+    public onComponentSelectionChange(completion: ICompletion): void {
         if (!completion) {
             this.updateCallback({
                 replaceParts: [this.queryPart],
@@ -109,14 +109,14 @@ export default class AutocompleteWrapperModel {
         }
     }
 
-    public onComponentConfirm(completion: ICompletion) {
+    public onComponentConfirm(completion: ICompletion): void {
         this.updateCallback({
             replaceParts: this.partForCompletion(completion),
             close: true,
         });
     }
 
-    private partForCompletion(completion: ICompletion) {
+    private partForCompletion(completion: ICompletion): Part[] {
         const { completionId } = completion;
         const text = completion.completion;
         switch (completion.type) {

src/editor/caret.ts

@@ -19,7 +19,7 @@ import { needsCaretNodeBefore, needsCaretNodeAfter } from "./render";
 import Range from "./range";
 import EditorModel from "./model";
 import DocumentPosition, { IPosition } from "./position";
-import { Part } from "./parts";
+import { Part, Type } from "./parts";
 
 export type Caret = Range | DocumentPosition;
 
@@ -113,7 +113,7 @@ function findNodeInLineForPart(parts: Part[], partIndex: number) {
     // to find newline parts
     for (let i = 0; i <= partIndex; ++i) {
         const part = parts[i];
-        if (part.type === "newline") {
+        if (part.type === Type.Newline) {
             lineIndex += 1;
             nodeIndex = -1;
             prevPart = null;
@@ -128,7 +128,7 @@ function findNodeInLineForPart(parts: Part[], partIndex: number) {
             // and not an adjacent caret node
             if (i < partIndex) {
                 const nextPart = parts[i + 1];
-                const isLastOfLine = !nextPart || nextPart.type === "newline";
+                const isLastOfLine = !nextPart || nextPart.type === Type.Newline;
                 if (needsCaretNodeAfter(part, isLastOfLine)) {
                     nodeIndex += 1;
                 }

src/editor/deserialize.ts

@@ -20,7 +20,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event";
 import { walkDOMDepthFirst } from "./dom";
 import { checkBlockNode } from "../HtmlUtils";
 import { getPrimaryPermalinkEntity } from "../utils/permalinks/Permalinks";
-import { PartCreator } from "./parts";
+import { PartCreator, Type } from "./parts";
 import SdkConfig from "../SdkConfig";
 
 function parseAtRoomMentions(text: string, partCreator: PartCreator) {
@@ -206,7 +206,7 @@ function prefixQuoteLines(isFirstNode, parts, partCreator) {
         parts.splice(0, 0, partCreator.plain(QUOTE_LINE_PREFIX));
     }
     for (let i = 0; i < parts.length; i += 1) {
-        if (parts[i].type === "newline") {
+        if (parts[i].type === Type.Newline) {
             parts.splice(i + 1, 0, partCreator.plain(QUOTE_LINE_PREFIX));
             i += 1;
         }

src/editor/diff.ts

@@ -21,7 +21,7 @@ export interface IDiff {
     at?: number;
 }
 
-function firstDiff(a: string, b: string) {
+function firstDiff(a: string, b: string): number {
     const compareLen = Math.min(a.length, b.length);
     for (let i = 0; i < compareLen; ++i) {
         if (a[i] !== b[i]) {

src/editor/history.ts

@@ -36,7 +36,7 @@ export default class HistoryManager {
     private addedSinceLastPush = false;
     private removedSinceLastPush = false;
 
-    clear() {
+    public clear(): void {
         this.stack = [];
         this.newlyTypedCharCount = 0;
         this.currentIndex = -1;
@@ -103,7 +103,7 @@ export default class HistoryManager {
     }
 
     // needs to persist parts and caret position
-    tryPush(model: EditorModel, caret: Caret, inputType: string, diff: IDiff) {
+    public tryPush(model: EditorModel, caret: Caret, inputType: string, diff: IDiff): boolean {
         // ignore state restoration echos.
         // these respect the inputType values of the input event,
         // but are actually passed in from MessageEditor calling model.reset()
@@ -121,22 +121,22 @@ export default class HistoryManager {
         return shouldPush;
     }
 
-    ensureLastChangesPushed(model: EditorModel) {
+    public ensureLastChangesPushed(model: EditorModel): void {
         if (this.changedSinceLastPush) {
             this.pushState(model, this.lastCaret);
         }
     }
 
-    canUndo() {
+    public canUndo(): boolean {
         return this.currentIndex >= 1 || this.changedSinceLastPush;
     }
 
-    canRedo() {
+    public canRedo(): boolean {
         return this.currentIndex < (this.stack.length - 1);
     }
 
     // returns state that should be applied to model
-    undo(model: EditorModel) {
+    public undo(model: EditorModel): IHistory {
         if (this.canUndo()) {
             this.ensureLastChangesPushed(model);
             this.currentIndex -= 1;
@@ -145,7 +145,7 @@ export default class HistoryManager {
     }
 
     // returns state that should be applied to model
-    redo() {
+    public redo(): IHistory {
         if (this.canRedo()) {
             this.changedSinceLastPush = false;
             this.currentIndex += 1;

src/editor/offset.ts

@@ -15,16 +15,17 @@ limitations under the License.
 */
 
 import EditorModel from "./model";
+import DocumentPosition from "./position";
 
 export default class DocumentOffset {
     constructor(public offset: number, public readonly atNodeEnd: boolean) {
     }
 
-    asPosition(model: EditorModel) {
+    public asPosition(model: EditorModel): DocumentPosition {
         return model.positionForOffset(this.offset, this.atNodeEnd);
     }
 
-    add(delta: number, atNodeEnd = false) {
+    public add(delta: number, atNodeEnd = false): DocumentOffset {
         return new DocumentOffset(this.offset + delta, atNodeEnd);
     }
 }

src/editor/operations.ts

@@ -15,13 +15,13 @@ limitations under the License.
 */
 
 import Range from "./range";
-import { Part } from "./parts";
+import { Part, Type } from "./parts";
 
 /**
  * Some common queries and transformations on the editor model
  */
 
-export function replaceRangeAndExpandSelection(range: Range, newParts: Part[]) {
+export function replaceRangeAndExpandSelection(range: Range, newParts: Part[]): void {
     const { model } = range;
     model.transform(() => {
         const oldLen = range.length;
@@ -32,7 +32,7 @@ export function replaceRangeAndExpandSelection(range: Range, newParts: Part[]) {
     });
 }
 
-export function replaceRangeAndMoveCaret(range: Range, newParts: Part[]) {
+export function replaceRangeAndMoveCaret(range: Range, newParts: Part[]): void {
     const { model } = range;
     model.transform(() => {
         const oldLen = range.length;
@@ -43,29 +43,29 @@ export function replaceRangeAndMoveCaret(range: Range, newParts: Part[]) {
     });
 }
 
-export function rangeStartsAtBeginningOfLine(range: Range) {
+export function rangeStartsAtBeginningOfLine(range: Range): boolean {
     const { model } = range;
     const startsWithPartial = range.start.offset !== 0;
     const isFirstPart = range.start.index === 0;
-    const previousIsNewline = !isFirstPart && model.parts[range.start.index - 1].type === "newline";
+    const previousIsNewline = !isFirstPart && model.parts[range.start.index - 1].type === Type.Newline;
     return !startsWithPartial && (isFirstPart || previousIsNewline);
 }
 
-export function rangeEndsAtEndOfLine(range: Range) {
+export function rangeEndsAtEndOfLine(range: Range): boolean {
     const { model } = range;
     const lastPart = model.parts[range.end.index];
     const endsWithPartial = range.end.offset !== lastPart.text.length;
     const isLastPart = range.end.index === model.parts.length - 1;
-    const nextIsNewline = !isLastPart && model.parts[range.end.index + 1].type === "newline";
+    const nextIsNewline = !isLastPart && model.parts[range.end.index + 1].type === Type.Newline;
     return !endsWithPartial && (isLastPart || nextIsNewline);
 }
 
-export function formatRangeAsQuote(range: Range) {
+export function formatRangeAsQuote(range: Range): void {
     const { model, parts } = range;
     const { partCreator } = model;
     for (let i = 0; i < parts.length; ++i) {
         const part = parts[i];
-        if (part.type === "newline") {
+        if (part.type === Type.Newline) {
             parts.splice(i + 1, 0, partCreator.plain("> "));
         }
     }
@@ -81,10 +81,10 @@ export function formatRangeAsQuote(range: Range) {
     replaceRangeAndExpandSelection(range, parts);
 }
 
-export function formatRangeAsCode(range: Range) {
+export function formatRangeAsCode(range: Range): void {
     const { model, parts } = range;
     const { partCreator } = model;
-    const needsBlock = parts.some(p => p.type === "newline");
+    const needsBlock = parts.some(p => p.type === Type.Newline);
     if (needsBlock) {
         parts.unshift(partCreator.plain("```"), partCreator.newline());
         if (!rangeStartsAtBeginningOfLine(range)) {
@@ -105,9 +105,9 @@ export function formatRangeAsCode(range: Range) {
 
 // parts helper methods
 const isBlank = part => !part.text || !/\S/.test(part.text);
-const isNL = part => part.type === "newline";
+const isNL = part => part.type === Type.Newline;
 
-export function toggleInlineFormat(range: Range, prefix: string, suffix = prefix) {
+export function toggleInlineFormat(range: Range, prefix: string, suffix = prefix): void {
     const { model, parts } = range;
     const { partCreator } = model;
 

src/editor/parts.ts

@@ -25,6 +25,8 @@ import AutocompleteWrapperModel, {
     UpdateQuery,
 } from "./autocomplete";
 import * as Avatar from "../Avatar";
+import defaultDispatcher from "../dispatcher/dispatcher";
+import { Action } from "../dispatcher/actions";
 
 interface ISerializedPart {
     type: Type.Plain | Type.Newline | Type.Command | Type.PillCandidate;
@@ -39,7 +41,7 @@ interface ISerializedPillPart {
 
 export type SerializedPart = ISerializedPart | ISerializedPillPart;
 
-enum Type {
+export enum Type {
     Plain = "plain",
     Newline = "newline",
     Command = "command",
@@ -57,12 +59,12 @@ interface IBasePart {
     createAutoComplete(updateCallback: UpdateCallback): void;
 
     serialize(): SerializedPart;
-    remove(offset: number, len: number): string;
+    remove(offset: number, len: number): string | undefined;
     split(offset: number): IBasePart;
     validateAndInsert(offset: number, str: string, inputType: string): boolean;
-    appendUntilRejected(str: string, inputType: string): string;
+    appendUntilRejected(str: string, inputType: string): string | undefined;
-    updateDOMNode(node: Node);
+    updateDOMNode(node: Node): void;
-    canUpdateDOMNode(node: Node);
+    canUpdateDOMNode(node: Node): boolean;
     toDOMNode(): Node;
 }
 
@@ -85,19 +87,19 @@ abstract class BasePart {
         this._text = text;
     }
 
-    acceptsInsertion(chr: string, offset: number, inputType: string) {
+    protected acceptsInsertion(chr: string, offset: number, inputType: string): boolean {
         return true;
     }
 
-    acceptsRemoval(position: number, chr: string) {
+    protected acceptsRemoval(position: number, chr: string): boolean {
         return true;
     }
 
-    merge(part: Part) {
+    public merge(part: Part): boolean {
         return false;
     }
 
-    split(offset: number) {
+    public split(offset: number): IBasePart {
         const splitText = this.text.substr(offset);
         this._text = this.text.substr(0, offset);
         return new PlainPart(splitText);
@@ -105,7 +107,7 @@ abstract class BasePart {
 
     // removes len chars, or returns the plain text this part should be replaced with
     // if the part would become invalid if it removed everything.
-    remove(offset: number, len: number) {
+    public remove(offset: number, len: number): string | undefined {
         // validate
         const strWithRemoval = this.text.substr(0, offset) + this.text.substr(offset + len);
         for (let i = offset; i < (len + offset); ++i) {
@@ -118,7 +120,7 @@ abstract class BasePart {
     }
 
     // append str, returns the remaining string if a character was rejected.
-    appendUntilRejected(str: string, inputType: string) {
+    public appendUntilRejected(str: string, inputType: string): string | undefined {
         const offset = this.text.length;
         for (let i = 0; i < str.length; ++i) {
             const chr = str.charAt(i);
@@ -132,7 +134,7 @@ abstract class BasePart {
 
     // inserts str at offset if all the characters in str were accepted, otherwise don't do anything
     // return whether the str was accepted or not.
-    validateAndInsert(offset: number, str: string, inputType: string) {
+    public validateAndInsert(offset: number, str: string, inputType: string): boolean {
         for (let i = 0; i < str.length; ++i) {
             const chr = str.charAt(i);
             if (!this.acceptsInsertion(chr, offset + i, inputType)) {
@@ -145,42 +147,42 @@ abstract class BasePart {
         return true;
     }
 
-    createAutoComplete(updateCallback: UpdateCallback): void {}
+    public createAutoComplete(updateCallback: UpdateCallback): void {}
 
-    trim(len: number) {
+    protected trim(len: number): string {
         const remaining = this._text.substr(len);
         this._text = this._text.substr(0, len);
         return remaining;
     }
 
-    get text() {
+    public get text(): string {
         return this._text;
     }
 
-    abstract get type(): Type;
+    public abstract get type(): Type;
 
-    get canEdit() {
+    public get canEdit(): boolean {
         return true;
     }
 
-    toString() {
+    public toString(): string {
         return `${this.type}(${this.text})`;
     }
 
-    serialize(): SerializedPart {
+    public serialize(): SerializedPart {
         return {
             type: this.type as ISerializedPart["type"],
             text: this.text,
         };
     }
 
-    abstract updateDOMNode(node: Node);
+    public abstract updateDOMNode(node: Node): void;
-    abstract canUpdateDOMNode(node: Node);
+    public abstract canUpdateDOMNode(node: Node): boolean;
-    abstract toDOMNode(): Node;
+    public abstract toDOMNode(): Node;
 }
 
 abstract class PlainBasePart extends BasePart {
-    acceptsInsertion(chr: string, offset: number, inputType: string) {
+    protected acceptsInsertion(chr: string, offset: number, inputType: string): boolean {
         if (chr === "\n") {
             return false;
         }
@@ -203,11 +205,11 @@ abstract class PlainBasePart extends BasePart {
         return true;
     }
 
-    toDOMNode() {
+    public toDOMNode(): Node {
         return document.createTextNode(this.text);
     }
 
-    merge(part) {
+    public merge(part): boolean {
         if (part.type === this.type) {
             this._text = this.text + part.text;
             return true;
@@ -215,47 +217,49 @@ abstract class PlainBasePart extends BasePart {
         return false;
     }
 
-    updateDOMNode(node: Node) {
+    public updateDOMNode(node: Node): void {
         if (node.textContent !== this.text) {
             node.textContent = this.text;
         }
     }
 
-    canUpdateDOMNode(node: Node) {
+    public canUpdateDOMNode(node: Node): boolean {
         return node.nodeType === Node.TEXT_NODE;
     }
 }
 
 // exported for unit tests, should otherwise only be used through PartCreator
 export class PlainPart extends PlainBasePart implements IBasePart {
-    get type(): IBasePart["type"] {
+    public get type(): IBasePart["type"] {
         return Type.Plain;
     }
 }
 
-abstract class PillPart extends BasePart implements IPillPart {
+export abstract class PillPart extends BasePart implements IPillPart {
     constructor(public resourceId: string, label) {
         super(label);
     }
 
-    acceptsInsertion(chr: string) {
+    protected acceptsInsertion(chr: string): boolean {
         return chr !== " ";
     }
 
-    acceptsRemoval(position: number, chr: string) {
+    protected acceptsRemoval(position: number, chr: string): boolean {
         return position !== 0;  //if you remove initial # or @, pill should become plain
     }
 
-    toDOMNode() {
+    public toDOMNode(): Node {
         const container = document.createElement("span");
         container.setAttribute("spellcheck", "false");
+        container.setAttribute("contentEditable", "false");
+        container.onclick = this.onClick;
         container.className = this.className;
         container.appendChild(document.createTextNode(this.text));
         this.setAvatar(container);
         return container;
     }
 
-    updateDOMNode(node: HTMLElement) {
+    public updateDOMNode(node: HTMLElement): void {
         const textNode = node.childNodes[0];
         if (textNode.textContent !== this.text) {
             textNode.textContent = this.text;
@@ -263,10 +267,13 @@ abstract class PillPart extends BasePart implements IPillPart {
         if (node.className !== this.className) {
             node.className = this.className;
         }
+        if (node.onclick !== this.onClick) {
+            node.onclick = this.onClick;
+        }
         this.setAvatar(node);
     }
 
-    canUpdateDOMNode(node: HTMLElement) {
+    public canUpdateDOMNode(node: HTMLElement): boolean {
         return node.nodeType === Node.ELEMENT_NODE &&
                node.nodeName === "SPAN" &&
                node.childNodes.length === 1 &&
@@ -274,7 +281,7 @@ abstract class PillPart extends BasePart implements IPillPart {
     }
 
     // helper method for subclasses
-    protected setAvatarVars(node: HTMLElement, avatarUrl: string, initialLetter: string) {
+    protected setAvatarVars(node: HTMLElement, avatarUrl: string, initialLetter: string): void {
         const avatarBackground = `url('${avatarUrl}')`;
         const avatarLetter = `'${initialLetter}'`;
         // check if the value is changing,
@@ -287,7 +294,7 @@ abstract class PillPart extends BasePart implements IPillPart {
         }
     }
 
-    serialize(): ISerializedPillPart {
+    public serialize(): ISerializedPillPart {
         return {
             type: this.type,
             text: this.text,
@@ -295,41 +302,43 @@ abstract class PillPart extends BasePart implements IPillPart {
         };
     }
 
-    get canEdit() {
+    public get canEdit(): boolean {
         return false;
     }
 
-    abstract get type(): IPillPart["type"];
+    public abstract get type(): IPillPart["type"];
 
-    abstract get className(): string;
+    protected abstract get className(): string;
 
-    abstract setAvatar(node: HTMLElement): void;
+    protected onClick?: () => void;
+
+    protected abstract setAvatar(node: HTMLElement): void;
 }
 
 class NewlinePart extends BasePart implements IBasePart {
-    acceptsInsertion(chr: string, offset: number) {
+    protected acceptsInsertion(chr: string, offset: number): boolean {
         return offset === 0 && chr === "\n";
     }
 
-    acceptsRemoval(position: number, chr: string) {
+    protected acceptsRemoval(position: number, chr: string): boolean {
         return true;
     }
 
-    toDOMNode() {
+    public toDOMNode(): Node {
         return document.createElement("br");
     }
 
-    merge() {
+    public merge(): boolean {
         return false;
     }
 
-    updateDOMNode() {}
+    public updateDOMNode(): void {}
 
-    canUpdateDOMNode(node: HTMLElement) {
+    public canUpdateDOMNode(node: HTMLElement): boolean {
         return node.tagName === "BR";
     }
 
-    get type(): IBasePart["type"] {
+    public get type(): IBasePart["type"] {
         return Type.Newline;
     }
 
@@ -337,7 +346,7 @@ class NewlinePart extends BasePart implements IBasePart {
     // rather than trying to append to it, which is what we want.
     // As a newline can also be only one character, it makes sense
     // as it can only be one character long. This caused #9741.
-    get canEdit() {
+    public get canEdit(): boolean {
         return false;
     }
 }
@@ -347,7 +356,7 @@ class RoomPillPart extends PillPart {
         super(resourceId, label);
     }
 
-    setAvatar(node: HTMLElement) {
+    protected setAvatar(node: HTMLElement): void {
         let initialLetter = "";
         let avatarUrl = Avatar.avatarUrlForRoom(this.room, 16, 16, "crop");
         if (!avatarUrl) {
@@ -357,11 +366,11 @@ class RoomPillPart extends PillPart {
         this.setAvatarVars(node, avatarUrl, initialLetter);
     }
 
-    get type(): IPillPart["type"] {
+    public get type(): IPillPart["type"] {
         return Type.RoomPill;
     }
 
-    get className() {
+    protected get className() {
         return "mx_RoomPill mx_Pill";
     }
 }
@@ -371,11 +380,11 @@ class AtRoomPillPart extends RoomPillPart {
         super(text, text, room);
     }
 
-    get type(): IPillPart["type"] {
+    public get type(): IPillPart["type"] {
         return Type.AtRoomPill;
     }
 
-    serialize(): ISerializedPillPart {
+    public serialize(): ISerializedPillPart {
         return {
             type: this.type,
             text: this.text,
@@ -388,7 +397,15 @@ class UserPillPart extends PillPart {
         super(userId, displayName);
     }
 
-    setAvatar(node: HTMLElement) {
+    public get type(): IPillPart["type"] {
+        return Type.UserPill;
+    }
+
+    protected get className() {
+        return "mx_UserPill mx_Pill";
+    }
+
+    protected setAvatar(node: HTMLElement): void {
         if (!this.member) {
             return;
         }
@@ -402,13 +419,12 @@ class UserPillPart extends PillPart {
         this.setAvatarVars(node, avatarUrl, initialLetter);
     }
 
-    get type(): IPillPart["type"] {
+    protected onClick = (): void => {
-        return Type.UserPill;
+        defaultDispatcher.dispatch({
-    }
+            action: Action.ViewUser,
-
+            member: this.member,
-    get className() {
+        });
-        return "mx_UserPill mx_Pill";
+    };
-    }
 }
 
 class PillCandidatePart extends PlainBasePart implements IPillCandidatePart {
@@ -416,11 +432,11 @@ class PillCandidatePart extends PlainBasePart implements IPillCandidatePart {
         super(text);
     }
 
-    createAutoComplete(updateCallback: UpdateCallback): AutocompleteWrapperModel {
+    public createAutoComplete(updateCallback: UpdateCallback): AutocompleteWrapperModel {
         return this.autoCompleteCreator.create(updateCallback);
     }
 
-    acceptsInsertion(chr: string, offset: number, inputType: string) {
+    protected acceptsInsertion(chr: string, offset: number, inputType: string): boolean {
         if (offset === 0) {
             return true;
         } else {
@@ -428,11 +444,11 @@ class PillCandidatePart extends PlainBasePart implements IPillCandidatePart {
         }
     }
 
-    merge() {
+    public merge(): boolean {
         return false;
     }
 
-    acceptsRemoval(position: number, chr: string) {
+    protected acceptsRemoval(position: number, chr: string): boolean {
         return true;
     }
 
@@ -463,17 +479,21 @@ interface IAutocompleteCreator {
 export class PartCreator {
     protected readonly autoCompleteCreator: IAutocompleteCreator;
 
-    constructor(private room: Room, private client: MatrixClient, autoCompleteCreator: AutoCompleteCreator = null) {
+    constructor(
+        private readonly room: Room,
+        private readonly client: MatrixClient,
+        autoCompleteCreator: AutoCompleteCreator = null,
+    ) {
         // pre-create the creator as an object even without callback so it can already be passed
         // to PillCandidatePart (e.g. while deserializing) and set later on
-        this.autoCompleteCreator = { create: autoCompleteCreator && autoCompleteCreator(this) };
+        this.autoCompleteCreator = { create: autoCompleteCreator?.(this) };
     }
 
-    setAutoCompleteCreator(autoCompleteCreator: AutoCompleteCreator) {
+    public setAutoCompleteCreator(autoCompleteCreator: AutoCompleteCreator): void {
         this.autoCompleteCreator.create = autoCompleteCreator(this);
     }
 
-    createPartForInput(input: string, partIndex: number, inputType?: string): Part {
+    public createPartForInput(input: string, partIndex: number, inputType?: string): Part {
         switch (input[0]) {
             case "#":
             case "@":
@@ -487,11 +507,11 @@ export class PartCreator {
         }
     }
 
-    createDefaultPart(text: string) {
+    public createDefaultPart(text: string): Part {
         return this.plain(text);
     }
 
-    deserializePart(part: SerializedPart): Part {
+    public deserializePart(part: SerializedPart): Part {
         switch (part.type) {
             case Type.Plain:
                 return this.plain(part.text);
@@ -508,19 +528,19 @@ export class PartCreator {
         }
     }
 
-    plain(text: string) {
+    public plain(text: string): PlainPart {
         return new PlainPart(text);
     }
 
-    newline() {
+    public newline(): NewlinePart {
         return new NewlinePart("\n");
     }
 
-    pillCandidate(text: string) {
+    public pillCandidate(text: string): PillCandidatePart {
         return new PillCandidatePart(text, this.autoCompleteCreator);
     }
 
-    roomPill(alias: string, roomId?: string) {
+    public roomPill(alias: string, roomId?: string): RoomPillPart {
         let room;
         if (roomId || alias[0] !== "#") {
             room = this.client.getRoom(roomId || alias);
@@ -533,16 +553,20 @@ export class PartCreator {
         return new RoomPillPart(alias, room ? room.name : alias, room);
     }
 
-    atRoomPill(text: string) {
+    public atRoomPill(text: string): AtRoomPillPart {
         return new AtRoomPillPart(text, this.room);
     }
 
-    userPill(displayName: string, userId: string) {
+    public userPill(displayName: string, userId: string): UserPillPart {
         const member = this.room.getMember(userId);
         return new UserPillPart(userId, displayName, member);
     }
 
-    createMentionParts(insertTrailingCharacter: boolean, displayName: string, userId: string) {
+    public createMentionParts(
+        insertTrailingCharacter: boolean,
+        displayName: string,
+        userId: string,
+    ): [UserPillPart, PlainPart] {
         const pill = this.userPill(displayName, userId);
         const postfix = this.plain(insertTrailingCharacter ? ": " : " ");
         return [pill, postfix];
@@ -567,7 +591,7 @@ export class CommandPartCreator extends PartCreator {
     }
 
     public deserializePart(part: SerializedPart): Part {
-        if (part.type === "command") {
+        if (part.type === Type.Command) {
             return this.command(part.text);
         } else {
             return super.deserializePart(part);

src/editor/position.ts

@@ -30,7 +30,7 @@ export default class DocumentPosition implements IPosition {
     constructor(public readonly index: number, public readonly offset: number) {
     }
 
-    compare(otherPos: DocumentPosition) {
+    public compare(otherPos: DocumentPosition): number {
         if (this.index === otherPos.index) {
             return this.offset - otherPos.offset;
         } else {
@@ -38,7 +38,7 @@ export default class DocumentPosition implements IPosition {
         }
     }
 
-    iteratePartsBetween(other: DocumentPosition, model: EditorModel, callback: Callback) {
+    public iteratePartsBetween(other: DocumentPosition, model: EditorModel, callback: Callback): void {
         if (this.index === -1 || other.index === -1) {
             return;
         }
@@ -57,7 +57,7 @@ export default class DocumentPosition implements IPosition {
         }
     }
 
-    forwardsWhile(model: EditorModel, predicate: Predicate) {
+    public forwardsWhile(model: EditorModel, predicate: Predicate): DocumentPosition {
         if (this.index === -1) {
             return this;
         }
@@ -82,7 +82,7 @@ export default class DocumentPosition implements IPosition {
         }
     }
 
-    backwardsWhile(model: EditorModel, predicate: Predicate) {
+    public backwardsWhile(model: EditorModel, predicate: Predicate): DocumentPosition {
         if (this.index === -1) {
             return this;
         }
@@ -107,7 +107,7 @@ export default class DocumentPosition implements IPosition {
         }
     }
 
-    asOffset(model: EditorModel) {
+    public asOffset(model: EditorModel): DocumentOffset {
         if (this.index === -1) {
             return new DocumentOffset(0, true);
         }
@@ -121,7 +121,7 @@ export default class DocumentPosition implements IPosition {
         return new DocumentOffset(offset, atEnd);
     }
 
-    isAtEnd(model: EditorModel) {
+    public isAtEnd(model: EditorModel): boolean {
         if (model.parts.length === 0) {
             return true;
         }
@@ -130,7 +130,7 @@ export default class DocumentPosition implements IPosition {
         return this.index === lastPartIdx && this.offset === lastPart.text.length;
     }
 
-    isAtStart() {
+    public isAtStart(): boolean {
         return this.index === 0 && this.offset === 0;
     }
 }

src/editor/range.ts

@@ -32,23 +32,23 @@ export default class Range {
         this._end = bIsLarger ? positionB : positionA;
     }
 
-    moveStart(delta: number) {
+    public moveStart(delta: number): void {
         this._start = this._start.forwardsWhile(this.model, () => {
             delta -= 1;
             return delta >= 0;
         });
     }
 
-    trim() {
+    public trim(): void {
         this._start = this._start.forwardsWhile(this.model, whitespacePredicate);
         this._end = this._end.backwardsWhile(this.model, whitespacePredicate);
     }
 
-    expandBackwardsWhile(predicate: Predicate) {
+    public expandBackwardsWhile(predicate: Predicate): void {
         this._start = this._start.backwardsWhile(this.model, predicate);
     }
 
-    get text() {
+    public get text(): string {
         let text = "";
         this._start.iteratePartsBetween(this._end, this.model, (part, startIdx, endIdx) => {
             const t = part.text.substring(startIdx, endIdx);
@@ -63,7 +63,7 @@ export default class Range {
      * @param {Part[]} parts the parts to replace the range with
      * @return {Number} the net amount of characters added, can be negative.
      */
-    replace(parts: Part[]) {
+    public replace(parts: Part[]): number {
         const newLength = parts.reduce((sum, part) => sum + part.text.length, 0);
         let oldLength = 0;
         this._start.iteratePartsBetween(this._end, this.model, (part, startIdx, endIdx) => {
@@ -77,8 +77,8 @@ export default class Range {
      * Returns a copy of the (partial) parts within the range.
      * For partial parts, only the text is adjusted to the part that intersects with the range.
      */
-    get parts() {
+    public get parts(): Part[] {
-        const parts = [];
+        const parts: Part[] = [];
         this._start.iteratePartsBetween(this._end, this.model, (part, startIdx, endIdx) => {
             const serializedPart = part.serialize();
             serializedPart.text = part.text.substring(startIdx, endIdx);
@@ -88,7 +88,7 @@ export default class Range {
         return parts;
     }
 
-    get length() {
+    public get length(): number {
         let len = 0;
         this._start.iteratePartsBetween(this._end, this.model, (part, startIdx, endIdx) => {
             len += endIdx - startIdx;
@@ -96,11 +96,11 @@ export default class Range {
         return len;
     }
 
-    get start() {
+    public get start(): DocumentPosition {
         return this._start;
     }
 
-    get end() {
+    public get end(): DocumentPosition {
         return this._end;
     }
 }

src/editor/render.ts

@@ -15,19 +15,19 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { Part } from "./parts";
+import { Part, Type } from "./parts";
 import EditorModel from "./model";
 
-export function needsCaretNodeBefore(part: Part, prevPart: Part) {
+export function needsCaretNodeBefore(part: Part, prevPart: Part): boolean {
-    const isFirst = !prevPart || prevPart.type === "newline";
+    const isFirst = !prevPart || prevPart.type === Type.Newline;
     return !part.canEdit && (isFirst || !prevPart.canEdit);
 }
 
-export function needsCaretNodeAfter(part: Part, isLastOfLine: boolean) {
+export function needsCaretNodeAfter(part: Part, isLastOfLine: boolean): boolean {
     return !part.canEdit && isLastOfLine;
 }
 
-function insertAfter(node: HTMLElement, nodeToInsert: HTMLElement) {
+function insertAfter(node: HTMLElement, nodeToInsert: HTMLElement): void {
     const next = node.nextSibling;
     if (next) {
         node.parentElement.insertBefore(nodeToInsert, next);
@@ -44,25 +44,25 @@ export const CARET_NODE_CHAR = "\ufeff";
 // a caret node is a node that allows the caret to be placed
 // where otherwise it wouldn't be possible
 // (e.g. next to a pill span without adjacent text node)
-function createCaretNode() {
+function createCaretNode(): HTMLElement {
     const span = document.createElement("span");
     span.className = "caretNode";
     span.appendChild(document.createTextNode(CARET_NODE_CHAR));
     return span;
 }
 
-function updateCaretNode(node: HTMLElement) {
+function updateCaretNode(node: HTMLElement): void {
     // ensure the caret node contains only a zero-width space
     if (node.textContent !== CARET_NODE_CHAR) {
         node.textContent = CARET_NODE_CHAR;
     }
 }
 
-export function isCaretNode(node: HTMLElement) {
+export function isCaretNode(node: HTMLElement): boolean {
     return node && node.tagName === "SPAN" && node.className === "caretNode";
 }
 
-function removeNextSiblings(node: ChildNode) {
+function removeNextSiblings(node: ChildNode): void {
     if (!node) {
         return;
     }
@@ -74,7 +74,7 @@ function removeNextSiblings(node: ChildNode) {
     }
 }
 
-function removeChildren(parent: HTMLElement) {
+function removeChildren(parent: HTMLElement): void {
     const firstChild = parent.firstChild;
     if (firstChild) {
         removeNextSiblings(firstChild);
@@ -82,7 +82,7 @@ function removeChildren(parent: HTMLElement) {
     }
 }
 
-function reconcileLine(lineContainer: ChildNode, parts: Part[]) {
+function reconcileLine(lineContainer: ChildNode, parts: Part[]): void {
     let currentNode;
     let prevPart;
     const lastPart = parts[parts.length - 1];
@@ -131,13 +131,13 @@ function reconcileLine(lineContainer: ChildNode, parts: Part[]) {
     removeNextSiblings(currentNode);
 }
 
-function reconcileEmptyLine(lineContainer) {
+function reconcileEmptyLine(lineContainer: HTMLElement): void {
     // empty div needs to have a BR in it to give it height
     let foundBR = false;
     let partNode = lineContainer.firstChild;
     while (partNode) {
         const nextNode = partNode.nextSibling;
-        if (!foundBR && partNode.tagName === "BR") {
+        if (!foundBR && (partNode as HTMLElement).tagName === "BR") {
             foundBR = true;
         } else {
             partNode.remove();
@@ -149,9 +149,9 @@ function reconcileEmptyLine(lineContainer) {
     }
 }
 
-export function renderModel(editor: HTMLDivElement, model: EditorModel) {
+export function renderModel(editor: HTMLDivElement, model: EditorModel): void {
     const lines = model.parts.reduce((linesArr, part) => {
-        if (part.type === "newline") {
+        if (part.type === Type.Newline) {
             linesArr.push([]);
         } else {
             const lastLine = linesArr[linesArr.length - 1];
@@ -175,7 +175,7 @@ export function renderModel(editor: HTMLDivElement, model: EditorModel) {
         if (parts.length) {
             reconcileLine(lineContainer, parts);
         } else {
-            reconcileEmptyLine(lineContainer);
+            reconcileEmptyLine(lineContainer as HTMLElement);
         }
     });
     if (lines.length) {

src/editor/serialize.ts

@@ -22,30 +22,31 @@ import { AllHtmlEntities } from 'html-entities';
 import SettingsStore from '../settings/SettingsStore';
 import SdkConfig from '../SdkConfig';
 import cheerio from 'cheerio';
+import { Type } from './parts';
 
-export function mdSerialize(model: EditorModel) {
+export function mdSerialize(model: EditorModel): string {
     return model.parts.reduce((html, part) => {
         switch (part.type) {
-            case "newline":
+            case Type.Newline:
                 return html + "\n";
-            case "plain":
+            case Type.Plain:
-            case "command":
+            case Type.Command:
-            case "pill-candidate":
+            case Type.PillCandidate:
-            case "at-room-pill":
+            case Type.AtRoomPill:
                 return html + part.text;
-            case "room-pill":
+            case Type.RoomPill:
                 // Here we use the resourceId for compatibility with non-rich text clients
                 // See https://github.com/vector-im/element-web/issues/16660
                 return html +
                     `[${part.resourceId.replace(/[[\\\]]/g, c => "\\" + c)}](${makeGenericPermalink(part.resourceId)})`;
-            case "user-pill":
+            case Type.UserPill:
                 return html +
                     `[${part.text.replace(/[[\\\]]/g, c => "\\" + c)}](${makeGenericPermalink(part.resourceId)})`;
         }
     }, "");
 }
 
-export function htmlSerializeIfNeeded(model: EditorModel, { forceHTML = false } = {}) {
+export function htmlSerializeIfNeeded(model: EditorModel, { forceHTML = false } = {}): string {
     let md = mdSerialize(model);
     // copy of raw input to remove unwanted math later
     const orig = md;
@@ -156,31 +157,31 @@ export function htmlSerializeIfNeeded(model: EditorModel, { forceHTML = false }
     }
 }
 
-export function textSerialize(model: EditorModel) {
+export function textSerialize(model: EditorModel): string {
     return model.parts.reduce((text, part) => {
         switch (part.type) {
-            case "newline":
+            case Type.Newline:
                 return text + "\n";
-            case "plain":
+            case Type.Plain:
-            case "command":
+            case Type.Command:
-            case "pill-candidate":
+            case Type.PillCandidate:
-            case "at-room-pill":
+            case Type.AtRoomPill:
                 return text + part.text;
-            case "room-pill":
+            case Type.RoomPill:
                 // Here we use the resourceId for compatibility with non-rich text clients
                 // See https://github.com/vector-im/element-web/issues/16660
                 return text + `${part.resourceId}`;
-            case "user-pill":
+            case Type.UserPill:
                 return text + `${part.text}`;
         }
     }, "");
 }
 
-export function containsEmote(model: EditorModel) {
+export function containsEmote(model: EditorModel): boolean {
     return startsWith(model, "/me ", false);
 }
 
-export function startsWith(model: EditorModel, prefix: string, caseSensitive = true) {
+export function startsWith(model: EditorModel, prefix: string, caseSensitive = true): boolean {
     const firstPart = model.parts[0];
     // part type will be "plain" while editing,
     // and "command" while composing a message.
@@ -190,26 +191,26 @@ export function startsWith(model: EditorModel, prefix: string, caseSensitive = t
         text = text.toLowerCase();
     }
 
-    return firstPart && (firstPart.type === "plain" || firstPart.type === "command") && text.startsWith(prefix);
+    return firstPart && (firstPart.type === Type.Plain || firstPart.type === Type.Command) && text.startsWith(prefix);
 }
 
-export function stripEmoteCommand(model: EditorModel) {
+export function stripEmoteCommand(model: EditorModel): EditorModel {
     // trim "/me "
     return stripPrefix(model, "/me ");
 }
 
-export function stripPrefix(model: EditorModel, prefix: string) {
+export function stripPrefix(model: EditorModel, prefix: string): EditorModel {
     model = model.clone();
     model.removeText({ index: 0, offset: 0 }, prefix.length);
     return model;
 }
 
-export function unescapeMessage(model: EditorModel) {
+export function unescapeMessage(model: EditorModel): EditorModel {
     const { parts } = model;
     if (parts.length) {
         const firstPart = parts[0];
         // only unescape \/ to / at start of editor
-        if (firstPart.type === "plain" && firstPart.text.startsWith("\\/")) {
+        if (firstPart.type === Type.Plain && firstPart.text.startsWith("\\/")) {
             model = model.clone();
             model.removeText({ index: 0, offset: 0 }, 1);
         }