Custom expression editor: fix the behavior of Tab (#19594)

frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx

@@ -132,6 +132,12 @@ export default class ExpressionEditorTextfield extends React.Component {
       fontSize: "12px",
     });
 
+    const passKeysToBrowser = editor.commands.byName.passKeysToBrowser;
+    editor.commands.bindKey("Tab", passKeysToBrowser);
+    editor.commands.bindKey("Shift-Tab", passKeysToBrowser);
+    editor.commands.removeCommand(editor.commands.byName.indent);
+    editor.commands.removeCommand(editor.commands.byName.outdent);
+
     this.setCaretPosition(
       this.state.source.length,
       this.state.source.length === 0,
@@ -218,14 +224,11 @@ export default class ExpressionEditorTextfield extends React.Component {
     }
   };
 
-  handleTab = () => {
+  chooseSuggestion = () => {
     const { highlightedSuggestionIndex, suggestions } = this.state;
-    const { editor } = this.input.current;
 
     if (suggestions.length) {
       this.onSuggestionSelected(highlightedSuggestionIndex);
-    } else {
-      editor.commands.byName.tab();
     }
   };
 
@@ -274,10 +277,31 @@ export default class ExpressionEditorTextfield extends React.Component {
 
   clearSuggestions() {
     this.setState({
-      suggestions: [],
       highlightedSuggestionIndex: 0,
       helpText: null,
     });
+    this.updateSuggestions([]);
+  }
+
+  updateSuggestions(suggestions = []) {
+    this.setState({ suggestions });
+
+    // Correctly bind Tab depending on whether suggestions are available or not
+    if (this.input.current) {
+      const { editor } = this.input.current;
+      const { suggestions } = this.state;
+      const tabBinding = editor.commands.commandKeyBinding.tab;
+      if (suggestions.length > 0) {
+        // Something to suggest? Tab is for choosing one of them
+        editor.commands.bindKey("Tab", editor.commands.byName.chooseSuggestion);
+      } else {
+        if (Array.isArray(tabBinding) && tabBinding.length > 1) {
+          // No more suggestions? Keep a single binding and remove the
+          // second one (added to choose a suggestion)
+          editor.commands.commandKeyBinding.tab = tabBinding.shift();
+        }
+      }
+    }
   }
 
   compileExpression() {
@@ -338,10 +362,8 @@ export default class ExpressionEditorTextfield extends React.Component {
       targetOffset: cursor.column,
     });
 
-    this.setState({
-      suggestions: suggestions || [],
-      helpText,
-    });
+    this.setState({ helpText });
+    this.updateSuggestions(suggestions);
   }
 
   errorAsMarkers(errorMessage = null) {
@@ -387,10 +409,10 @@ export default class ExpressionEditorTextfield extends React.Component {
       },
     },
     {
-      name: "tab",
-      bindKey: { win: "Tab", mac: "Tab" },
+      name: "chooseSuggestion",
+      bindKey: null,
       exec: () => {
-        this.handleTab();
+        this.chooseSuggestion();
       },
     },
   ];

frontend/test/metabase/scenarios/custom-column/custom-column.cy.spec.js

@@ -462,4 +462,68 @@ describe("scenarios > question > custom column", () => {
     cy.findByText("MiscDate Previous 30 Years"); // Filter name
     cy.findByText("MiscDate"); // Column name
   });
+
+  it("should allow switching focus with Tab", () => {
+    openOrdersTable({ mode: "notebook" });
+    cy.icon("add_data").click();
+
+    enterCustomColumnDetails({ formula: "1 + 2" });
+
+    // next focus: a link
+    cy.realPress("Tab");
+    cy.focused()
+      .should("have.attr", "class")
+      .and("eq", "link");
+    cy.focused()
+      .should("have.attr", "target")
+      .and("eq", "_blank");
+
+    // next focus: the textbox for the name
+    cy.realPress("Tab");
+    cy.focused()
+      .should("have.attr", "value")
+      .and("eq", "");
+    cy.focused()
+      .should("have.attr", "placeholder")
+      .and("eq", "Something nice and descriptive");
+
+    // Shift+Tab twice and we're back at the editor
+    cy.realPress(["Shift", "Tab"]);
+    cy.realPress(["Shift", "Tab"]);
+    cy.focused()
+      .should("have.attr", "class")
+      .and("eq", "ace_text-input");
+  });
+
+  it("should allow choosing a suggestion with Tab", () => {
+    openOrdersTable({ mode: "notebook" });
+    cy.icon("add_data").click();
+
+    enterCustomColumnDetails({ formula: "[" });
+
+    // Suggestion popover shows up and this select the first one ([Created At])
+    cy.realPress("Tab");
+
+    // Focus remains on the expression editor
+    cy.focused()
+      .should("have.attr", "class")
+      .and("eq", "ace_text-input");
+
+    // Tab twice to focus on the name box
+    cy.realPress("Tab");
+    cy.realPress("Tab");
+    cy.focused()
+      .should("have.attr", "value")
+      .and("eq", "");
+    cy.focused()
+      .should("have.attr", "placeholder")
+      .and("eq", "Something nice and descriptive");
+
+    // Shift+Tab twice and we're back at the editor
+    cy.realPress(["Shift", "Tab"]);
+    cy.realPress(["Shift", "Tab"]);
+    cy.focused()
+      .should("have.attr", "class")
+      .and("eq", "ace_text-input");
+  });
 });

frontend/test/metabase/scenarios/question/filter.cy.spec.js

@@ -818,6 +818,45 @@ describe("scenarios > question > filter", () => {
     cy.findByText("Expecting field but found 0");
   });
 
+  it("should allow switching focus with Tab", () => {
+    openOrdersTable({ mode: "notebook" });
+    cy.findByText("Filter").click();
+    cy.findByText("Custom Expression").click();
+    cy.get(".ace_text-input").type("[Tax] > 0");
+
+    // Tab switches the focus to the "Done" button
+    cy.realPress("Tab");
+    cy.focused()
+      .should("have.attr", "class")
+      .and("contains", "Button");
+  });
+
+  it("should allow choosing a suggestion with Tab", () => {
+    openOrdersTable({ mode: "notebook" });
+    cy.findByText("Filter").click();
+    cy.findByText("Custom Expression").click();
+
+    // Try to auto-complete Tax
+    cy.get(".ace_text-input").type("Ta");
+
+    // Suggestion popover shows up and this select the first one ([Created At])
+    cy.realPress("Tab");
+
+    // Focus remains on the expression editor
+    cy.focused()
+      .should("have.attr", "class")
+      .and("eq", "ace_text-input");
+
+    // Finish to complete a valid expression, i.e. [Tax] > 42
+    cy.get(".ace_text-input").type("> 42");
+
+    // Tab switches the focus to the "Done" button
+    cy.realPress("Tab");
+    cy.focused()
+      .should("have.attr", "class")
+      .and("contains", "Button");
+  });
+
   it.skip("should work on twice summarized questions (metabase#15620)", () => {
     visitQuestionAdhoc({
       dataset_query: {