Rework composer autocomplete to be smarter and not trap tab

src/components/views/rooms/Autocomplete.tsx

@@ -24,8 +24,6 @@ import {Room} from 'matrix-js-sdk/src/models/room';
 import SettingsStore from "../../../settings/SettingsStore";
 import Autocompleter from '../../../autocomplete/Autocompleter';
 
-const COMPOSER_SELECTED = 0;
-
 export const generateCompletionDomId = (number) => `mx_Autocomplete_Completion_${number}`;
 
 interface IProps {
@@ -68,7 +66,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,
@@ -112,7 +110,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,
             });
@@ -148,26 +146,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(this.state.completionList[selectionOffset - 1], selectionOffset - 1);
+            }
+        }
 
         this.setState({
             completions,
@@ -193,8 +196,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);
+        const index = (this.state.selectionOffset + delta + completionCount - 1) % completionCount;
-        this.setSelection(index);
+        this.setSelection(1 + index);
     }
 
     onEscape(e: KeyboardEvent): boolean {
@@ -213,7 +216,7 @@ export default class Autocomplete extends React.PureComponent<IProps, IState> {
     hide = () => {
         this.setState({
             hide: true,
-            selectionOffset: 0,
+            selectionOffset: 1,
             completions: [],
             completionList: [],
         });
@@ -232,8 +235,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;
         }
 

src/components/views/rooms/BasicMessageComposer.tsx

@@ -126,6 +126,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
         super(props);
         this.state = {
             showPillAvatar: SettingsStore.getValue("Pill.shouldShowPillAvatar"),
+            showVisualBell: false,
         };
 
         this.emoticonSettingHandle = SettingsStore.watchSetting('MessageComposerInput.autoReplaceEmoji', null,
@@ -201,7 +202,11 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
         if (isEmpty) {
             this.formatBarRef.current.hide();
         }
-        this.setState({autoComplete: this.props.model.autoComplete});
+        this.setState({
+            autoComplete: this.props.model.autoComplete,
+            // if a change is happening then clear the showVisualBell
+            showVisualBell: diff ? false : this.state.showVisualBell,
+        });
         this.historyManager.tryPush(this.props.model, selection, inputType, diff);
 
         let isTyping = !this.props.model.isEmpty;
@@ -490,6 +495,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
                         }
                         break;
                     case Key.TAB:
+                    case Key.ENTER:
                         if (!metaOrAltPressed) {
                             autoComplete.onTab(event);
                             handled = true;
@@ -504,7 +510,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
                     default:
                         return; // don't preventDefault on anything else
                 }
-            } else if (event.key === Key.TAB) {
+            } else if (!this.props.model.isEmpty && !this.state.showVisualBell && event.key === Key.TAB) {
                 this.tabCompleteName(event);
                 handled = true;
             } else if (event.key === Key.BACKSPACE || event.key === Key.DELETE) {
@@ -545,6 +551,8 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
                     this.setState({showVisualBell: true});
                     model.autoComplete.close();
                 }
+            } else {
+                this.setState({showVisualBell: true});
             }
         } catch (err) {
             console.error(err);
@@ -562,7 +570,7 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
 
     private onAutoCompleteSelectionChange = (completion: ICompletion, completionIndex: number) => {
         this.modifiedFlag = true;
-        this.props.model.autoComplete.onComponentSelectionChange(completion);
+        // this.props.model.autoComplete.onComponentSelectionChange(completion);
         this.setState({completionIndex});
     };
 
@@ -679,6 +687,11 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
         };
 
         const {completionIndex} = this.state;
+        const hasAutocomplete = Boolean(this.state.autoComplete);
+        let activeDescendant;
+        if (hasAutocomplete && completionIndex >= 0) {
+            activeDescendant = generateCompletionDomId(completionIndex);
+        }
 
         return (<div className={wrapperClasses}>
             { autoComplete }
@@ -697,10 +710,11 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
                 aria-label={this.props.label}
                 role="textbox"
                 aria-multiline="true"
-                aria-autocomplete="both"
+                aria-autocomplete="list"
                 aria-haspopup="listbox"
-                aria-expanded={Boolean(this.state.autoComplete)}
+                aria-expanded={hasAutocomplete}
-                aria-activedescendant={completionIndex >= 0 ? generateCompletionDomId(completionIndex) : undefined}
+                aria-owns="mx_Autocomplete"
+                aria-activedescendant={activeDescendant}
                 dir="auto"
             />
         </div>);

src/editor/autocomplete.ts

@@ -74,10 +74,9 @@ export default class AutocompleteWrapperModel {
         if (acComponent.countCompletions() === 0) {
             // Force completions to show for the text currently entered
             await acComponent.forceComplete();
-            // Select the first item by moving "down"
-            await acComponent.moveSelection(+1);
         } else {
-            await acComponent.moveSelection(e.shiftKey ? -1 : +1);
+            await acComponent.onConfirmCompletion();
+            this.updateCallback({close: true});
         }
     }