Rework composer autocomplete to be smarter and not trap tab

src/KeyBindingsDefaults.ts

@@ -161,31 +161,29 @@ const messageComposerBindings = (): KeyBinding<MessageComposerAction>[] => {
 const autocompleteBindings = (): KeyBinding<AutocompleteAction>[] => {
     return [
         {
-            action: AutocompleteAction.CompleteOrNextSelection,
+            action: AutocompleteAction.ForceComplete,
             keyCombo: {
                 key: Key.TAB,
             },
         },
         {
-            action: AutocompleteAction.CompleteOrNextSelection,
+            action: AutocompleteAction.ForceComplete,
             keyCombo: {
                 key: Key.TAB,
                 ctrlKey: true,
             },
         },
         {
-            action: AutocompleteAction.CompleteOrPrevSelection,
+            action: AutocompleteAction.Complete,
             keyCombo: {
-                key: Key.TAB,
-                shiftKey: true,
+                key: Key.ENTER,
             },
         },
         {
-            action: AutocompleteAction.CompleteOrPrevSelection,
+            action: AutocompleteAction.Complete,
             keyCombo: {
-                key: Key.TAB,
+                key: Key.ENTER,
                 ctrlKey: true,
-                shiftKey: true,
             },
         },
         {

src/KeyBindingsManager.ts

@@ -52,13 +52,11 @@ export enum MessageComposerAction {
 
 /** Actions for text editing autocompletion */
 export enum AutocompleteAction {
-    /**
-     * Select previous selection or, if the autocompletion window is not shown, open the window and select the first
-     * selection.
-     */
-    CompleteOrPrevSelection = 'ApplySelection',
-    /** Select next selection or, if the autocompletion window is not shown, open it and select the first selection */
-    CompleteOrNextSelection = 'CompleteOrNextSelection',
+    /** Accepts chosen autocomplete selection */
+    Complete = 'Complete',
+    /** Accepts chosen autocomplete selection or,
+     * if the autocompletion window is not shown, open the window and select the first selection */
+    ForceComplete = 'ForceComplete',
     /** Move to the previous autocomplete selection */
     PrevSelection = 'PrevSelection',
     /** Move to the next autocomplete selection */

src/autocomplete/AutocompleteProvider.tsx

@@ -27,11 +27,11 @@ export interface ICommand {
     };
 }
 
-export default class AutocompleteProvider {
+export default abstract class AutocompleteProvider {
     commandRegex: RegExp;
     forcedCommandRegex: RegExp;
 
-    constructor(commandRegex?: RegExp, forcedCommandRegex?: RegExp) {
+    protected constructor(commandRegex?: RegExp, forcedCommandRegex?: RegExp) {
         if (commandRegex) {
             if (!commandRegex.global) {
                 throw new Error('commandRegex must have global flag set');
@@ -93,23 +93,16 @@ export default class AutocompleteProvider {
         };
     }
 
-    async getCompletions(
+    abstract getCompletions(
         query: string,
         selection: ISelectionRange,
-        force = false,
-        limit = -1,
-    ): Promise<ICompletion[]> {
-        return [];
-    }
+        force: boolean,
+        limit: number,
+    ): Promise<ICompletion[]>;
 
-    getName(): string {
-        return 'Default Provider';
-    }
+    abstract getName(): string;
 
-    renderCompletions(completions: React.ReactNode[]): React.ReactNode | null {
-        console.error('stub; should be implemented in subclasses');
-        return null;
-    }
+    abstract renderCompletions(completions: React.ReactNode[]): React.ReactNode | null;
 
     // Whether we should provide completions even if triggered forcefully, without a sigil.
     shouldForceComplete(): boolean {

src/autocomplete/CommandProvider.tsx

@@ -97,7 +97,7 @@ export default class CommandProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_block"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("Command Autocomplete")}
             >
                 { completions }

src/autocomplete/CommunityProvider.tsx

@@ -118,7 +118,7 @@ export default class CommunityProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_pill mx_Autocomplete_Completion_container_truncate"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("Community Autocomplete")}
             >
                 { completions }

src/autocomplete/DuckDuckGoProvider.tsx

@@ -105,7 +105,7 @@ export default class DuckDuckGoProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_block"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("DuckDuckGo Results")}
             >
                 { completions }

src/autocomplete/EmojiProvider.tsx

@@ -145,7 +145,7 @@ export default class EmojiProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_pill"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("Emoji Autocomplete")}
             >
                 { completions }

src/autocomplete/NotifProvider.tsx

@@ -71,7 +71,7 @@ export default class NotifProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_pill mx_Autocomplete_Completion_container_truncate"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("Notification Autocomplete")}
             >
                 { completions }

src/autocomplete/RoomProvider.tsx

@@ -137,7 +137,7 @@ export default class RoomProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_pill mx_Autocomplete_Completion_container_truncate"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("Room Autocomplete")}
             >
                 { completions }

src/autocomplete/UserProvider.tsx

@@ -183,7 +183,7 @@ export default class UserProvider extends AutocompleteProvider {
         return (
             <div
                 className="mx_Autocomplete_Completion_container_pill"
-                role="listbox"
+                role="presentation"
                 aria-label={_t("User Autocomplete")}
             >
                 { completions }

src/components/structures/LoggedInView.tsx

@@ -537,24 +537,24 @@ class LoggedInView extends React.Component<IProps, IState> {
         }
 
         const isModifier = ev.key === Key.ALT || ev.key === Key.CONTROL || ev.key === Key.META || ev.key === Key.SHIFT;
-        if (!isModifier && !ev.altKey && !ev.ctrlKey && !ev.metaKey) {
+        if (!isModifier && !ev.ctrlKey && !ev.metaKey) {
             // The above condition is crafted to _allow_ characters with Shift
             // already pressed (but not the Shift key down itself).
-
             const isClickShortcut = ev.target !== document.body &&
                 (ev.key === Key.SPACE || ev.key === Key.ENTER);
 
-            // Do not capture the context menu key to improve keyboard accessibility
-            if (ev.key === Key.CONTEXT_MENU) {
-                return;
-            }
+            // We explicitly allow alt to be held due to it being a common accent modifier.
+            // XXX: Forwarding Dead keys in this way does not work as intended but better to at least
+            // move focus to the composer so the user can re-type the dead key correctly.
+            const isPrintable = ev.key.length === 1 || ev.key === "Dead";
 
-            if (!isClickShortcut && ev.key !== Key.TAB && !canElementReceiveInput(ev.target)) {
+            // If the user is entering a printable character outside of an input field
+            // redirect it to the composer for them.
+            if (!isClickShortcut && isPrintable && !canElementReceiveInput(ev.target)) {
                 // synchronous dispatch so we focus before key generates input
                 dis.fire(Action.FocusComposer, true);
                 ev.stopPropagation();
-                // we should *not* preventDefault() here as
-                // that would prevent typing in the now-focussed composer
+                // we should *not* preventDefault() here as that would prevent typing in the now-focused composer
             }
         }
     };

src/components/views/rooms/Autocomplete.tsx

@@ -25,7 +25,6 @@ import SettingsStore from "../../../settings/SettingsStore";
 import Autocompleter from '../../../autocomplete/Autocompleter';
 import {replaceableComponent} from "../../../utils/replaceableComponent";
 
-const COMPOSER_SELECTED = 0;
 const MAX_PROVIDER_MATCHES = 20;
 
 export const generateCompletionDomId = (number) => `mx_Autocomplete_Completion_${number}`;
@@ -34,9 +33,9 @@ interface IProps {
     // the query string for which to show autocomplete suggestions
     query: string;
     // method invoked with range and text content when completion is confirmed
-    onConfirm: (ICompletion) => void;
+    onConfirm: (completion: ICompletion) => void;
     // method invoked when selected (if any) completion changes
-    onSelectionChange?: (ICompletion, number) => void;
+    onSelectionChange?: (partIndex: number) => void;
     selection: ISelectionRange;
     // The room in which we're autocompleting
     room: Room;
@@ -71,7 +70,7 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
             completionList: [],
 
             // how far down the completion list we are (THIS IS 1-INDEXED!)
-            selectionOffset: COMPOSER_SELECTED,
+            selectionOffset: 1,
 
             // whether we should show completions if they're available
             shouldShowCompletions: true,
@@ -115,7 +114,7 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
                 completions: [],
                 completionList: [],
                 // Reset selected completion
-                selectionOffset: COMPOSER_SELECTED,
+                selectionOffset: 1,
                 // Hide the autocomplete box
                 hide: true,
             });
@@ -151,26 +150,31 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
         const completionList = flatMap(completions, (provider) => provider.completions);
 
         // Reset selection when completion list becomes empty.
-        let selectionOffset = COMPOSER_SELECTED;
+        let selectionOffset = 1;
         if (completionList.length > 0) {
             /* If the currently selected completion is still in the completion list,
              try to find it and jump to it. If not, select composer.
              */
-            const currentSelection = this.state.selectionOffset === 0 ? null :
+            const currentSelection = this.state.selectionOffset <= 1 ? null :
                 this.state.completionList[this.state.selectionOffset - 1].completion;
             selectionOffset = completionList.findIndex(
                 (completion) => completion.completion === currentSelection);
             if (selectionOffset === -1) {
-                selectionOffset = COMPOSER_SELECTED;
+                selectionOffset = 1;
             } else {
                 selectionOffset++; // selectionOffset is 1-indexed!
             }
         }
 
-        let hide = this.state.hide;
+        let hide = true;
         // If `completion.command.command` is truthy, then a provider has matched with the query
         const anyMatches = completions.some((completion) => !!completion.command.command);
-        hide = !anyMatches;
+        if (anyMatches) {
+            hide = false;
+            if (this.props.onSelectionChange) {
+                this.props.onSelectionChange(selectionOffset - 1);
+            }
+        }
 
         this.setState({
             completions,
@@ -196,8 +200,8 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
         if (completionCount === 0) return; // there are no items to move the selection through
 
         // Note: selectionOffset 0 represents the unsubstituted text, while 1 means first pill selected
-        const index = (this.state.selectionOffset + delta + completionCount + 1) % (completionCount + 1);
-        this.setSelection(index);
+        const index = (this.state.selectionOffset + delta + completionCount - 1) % completionCount;
+        this.setSelection(1 + index);
     }
 
     onEscape(e: KeyboardEvent): boolean {
@@ -216,7 +220,7 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
     hide = () => {
         this.setState({
             hide: true,
-            selectionOffset: 0,
+            selectionOffset: 1,
             completions: [],
             completionList: [],
         });
@@ -235,8 +239,13 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
         });
     }
 
+    onConfirmCompletion = () => {
+        this.onCompletionClicked(this.state.selectionOffset);
+    }
+
     onCompletionClicked = (selectionOffset: number): boolean => {
-        if (this.countCompletions() === 0 || selectionOffset === COMPOSER_SELECTED) {
+        const count = this.countCompletions();
+        if (count === 0 || selectionOffset < 1 || selectionOffset > count) {
             return false;
         }
 
@@ -249,7 +258,7 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
     setSelection(selectionOffset: number) {
         this.setState({selectionOffset, hide: false});
         if (this.props.onSelectionChange) {
-            this.props.onSelectionChange(this.state.completionList[selectionOffset - 1], selectionOffset - 1);
+            this.props.onSelectionChange(selectionOffset - 1);
         }
     }
 
@@ -293,15 +302,15 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
 
 
             return completions.length > 0 ? (
-                <div key={i} className="mx_Autocomplete_ProviderSection">
+                <div key={i} className="mx_Autocomplete_ProviderSection" role="presentation">
                     <div className="mx_Autocomplete_provider_name">{ completionResult.provider.getName() }</div>
                     { completionResult.provider.renderCompletions(completions) }
                 </div>
             ) : null;
         }).filter((completion) => !!completion);
 
         return !this.state.hide && renderedCompletions.length > 0 ? (
-            <div className="mx_Autocomplete" ref={this.containerRef}>
+            <div id="mx_Autocomplete" className="mx_Autocomplete" ref={this.containerRef} role="listbox">
                 { renderedCompletions }
             </div>
         ) : null;