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

@@ -96,7 +96,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

@@ -116,7 +116,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

@@ -139,7 +139,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

@@ -70,7 +70,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

@@ -134,7 +134,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

@@ -181,7 +181,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

@@ -529,24 +529,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.FocusSendMessageComposer, 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
             }
         }
     };