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,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { EventSubscription } from 'fbemitter';
 import RoomViewStore from './stores/RoomViewStore';
 
 type Listener = (isActive: boolean) => void;
@@ -30,7 +31,7 @@ type Listener = (isActive: boolean) => void;
 export class ActiveRoomObserver {
     private listeners: {[key: string]: Listener[]} = {};
     private _activeRoomId = RoomViewStore.getRoomId();
-    private readonly roomStoreToken: string;
+    private readonly roomStoreToken: EventSubscription;
 
     constructor() {
         // TODO: We could self-destruct when the last listener goes away, or at least stop listening.

src/components/views/rooms/BasicMessageComposer.tsx

@@ -32,7 +32,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";
@@ -157,7 +157,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) {
@@ -507,6 +507,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) {
@@ -515,6 +516,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));
@@ -524,9 +548,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 === "pill-candidate" ||
-                    part.type === "command"
+                    part.type === Type.Plain ||
+                    part.type === Type.PillCandidate ||
+                    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/components/views/voip/CallPreview.tsx

@@ -240,7 +240,7 @@ export default class CallPreview extends React.Component<IProps, IState> {
         this.scheduledUpdate.mark();
     };
 
-    private onRoomViewStoreUpdate = (payload) => {
+    private onRoomViewStoreUpdate = () => {
         if (RoomViewStore.getRoomId() === this.state.roomId) return;
 
         const roomId = RoomViewStore.getRoomId();

src/editor/autocomplete.ts

@@ -43,35 +43,35 @@ 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)],
             close: true,
         });
     }
 
-    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,23 +81,23 @@ 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;
         this.partIndex = pos.index;
         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) {
@@ -200,7 +200,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);
     }
 }