Fix AutoComplete double enter (#384)

* Fix double-enter on autocompleting with enter (fixes #383)
* Simplify TagsControl with form onSubmit

---------

Signed-off-by: Carl Gieringer <78054+carlgieringer@users.noreply.github.com>

package.json

@@ -84,6 +84,7 @@
   },
   "packageManager": "yarn@3.3.1",
   "resolutions": {
-    "react-native@^0.66": "patch:react-native@npm%3A0.66.5#./.yarn/patches/react-native-npm-0.66.5-22e5dd8ec5.patch"
+    "react-native@^0.66": "patch:react-native@npm%3A0.66.5#./.yarn/patches/react-native-npm-0.66.5-22e5dd8ec5.patch",
+    "@react-md/autocomplete@^2": "patch:@react-md/autocomplete@npm%3A2.9.1#./.yarn/patches/@react-md-autocomplete-npm-2.9.1-0914f23b5c.patch"
   }
 }

premiser-ui/README.md

@@ -1,11 +1,6 @@
-# Debugging Tests
+# Howdju web app (`premiser-ui`)
 
-```
-yarn test:debug
-```
-
-Then open Chrome at `chrome://inspect` and select the node instance.
-[Details](https://jestjs.io/docs/en/troubleshooting#tests-are-failing-and-you-don-t-know-why)
+This is the yarn workspace for Howdju's web app.
 
 ## Hot module reload
 
@@ -31,3 +26,11 @@ I did:
 
 But then I discovered that iframe.onLoad wasn't working in local development (used by extension), and I suspected the
 hot loaders interaction with react-dom, so I just switched to `yarn add 'react-dom@^16.2.0'`
+
+## react-md
+
+This workspace currently depends on a patch of @react-md/autocomplete@2.9.1 that adds
+`event.preventDefault()` when the user presses enter to autocomplete. This change prevents
+submitting a form when the user is autocompleting.
+
+PR for that change is here: https://github.com/mlaursen/react-md/pull/1439

premiser-ui/src/ApiAutoComplete.tsx

@@ -39,7 +39,8 @@ export interface Props<T>
   > {
   id: ComponentId;
   name: string;
-  autocompleteThrottleMs?: number;
+  /** The number of milliseconds after typing to wait before fetching suggestions. */
+  autocompleteDebounceMs?: number;
   fetchSuggestions: FetchSuggestionsActionCreator;
   cancelSuggestions: CancelSuggestionsActionCreator;
   /** Whether the input should be constrained to a single line. */
@@ -61,7 +62,7 @@ export interface Props<T>
 export default function ApiAutoComplete({
   id,
   name,
-  autocompleteThrottleMs = 250,
+  autocompleteDebounceMs = 250,
   fetchSuggestions,
   cancelSuggestions,
   singleLine = true,
@@ -79,7 +80,7 @@ export default function ApiAutoComplete({
   const dispatch = useAppDispatch();
   const debouncedFetchSuggestions = useDebouncedCallback((value: string) => {
     dispatch(fetchSuggestions(value, suggestionsKey));
-  }, autocompleteThrottleMs);
+  }, autocompleteDebounceMs);
 
   function onChange(event: ChangeEvent<HTMLInputElement>) {
     const name = event.target.name;

premiser-ui/src/CreatePropositionPage.tsx

@@ -491,7 +491,6 @@ class CreatePropositionPage extends Component<Props> {
                     suggestionsKey={combineSuggestionsKeys(id, tagsName)}
                     onTag={this.onTagProposition}
                     onUnTag={this.onUnTagProposition}
-                    onSubmit={onSubmit}
                   />
                 </CardText>
 

premiser-ui/src/TagsControl.test.tsx

@@ -1,17 +1,26 @@
 import React from "react";
-import { screen } from "@testing-library/react";
+import { fireEvent, screen } from "@testing-library/react";
 import TagsControl from "./TagsControl";
 import {
   renderWithProviders,
   setupUserEvent,
   withFakeTimers,
+  withMockServer,
 } from "./testUtils";
-import { CreatePropositionInput, Tag, TagVote } from "howdju-common";
+import {
+  CreatePropositionInput,
+  httpStatusCodes,
+  Tag,
+  TagVote,
+} from "howdju-common";
+import { InferResponseBody, serviceRoutes } from "howdju-service-routes";
+import { rest } from "msw";
 
 withFakeTimers();
+const server = withMockServer();
 
 describe("TagsControl", () => {
-  test("pressing enter with non-empty tag name adds tag", async () => {
+  test("pressing enter with non-empty tag name adds tag and doesn't submit enclosing form", async () => {
     const onSubmit = jest.fn();
     const onTag = jest.fn();
     const onUnTag = jest.fn();
@@ -42,33 +51,32 @@ describe("TagsControl", () => {
     expect(onUnTag).not.toHaveBeenCalled();
   });
 
-  test("pressing enter with empty tag name submits", async () => {
-    const onSubmit = jest.fn();
+  test("pressing enter with empty tag name collapses input", async () => {
     const onTag = jest.fn();
     const onUnTag = jest.fn();
 
     renderWithProviders(
-      <form onSubmit={onSubmit}>
-        <TagsControl
-          id="test-tags-control"
-          tags={[]}
-          votes={[]}
-          suggestionsKey="test-suggestions-key"
-          onTag={onTag}
-          onUnTag={onUnTag}
-          onSubmit={onSubmit}
-        />
-      </form>
+      <TagsControl
+        id="test-tags-control"
+        tags={[]}
+        votes={[]}
+        suggestionsKey="test-suggestions-key"
+        onTag={onTag}
+        onUnTag={onUnTag}
+        inputCollapsable={true}
+      />
     );
 
     const user = setupUserEvent();
 
     // Act
+    await user.click(screen.getByRole("button", { description: /add tag/i }));
     await user.type(screen.getByLabelText(/tag/i), "{Enter}");
 
     // Assert
-    // Ensure we didn't submit for both form and TagsControl.
-    expect(onSubmit).toHaveBeenCalledOnce();
+    expect(
+      screen.queryByRole("input", { name: "tagName" })
+    ).not.toBeInTheDocument();
   });
 
   test("downvoting tag calls untag", async () => {
@@ -101,5 +109,53 @@ describe("TagsControl", () => {
     // Assert
     expect(onUnTag).toHaveBeenCalled();
   });
-  // TODO(222): add more coverage
+
+  test("autocompleting with partial input tags only the autocompleted tag.", async () => {
+    const user = setupUserEvent();
+    const onTag = jest.fn();
+    const onUnTag = jest.fn();
+
+    server.use(
+      rest.get(`http://localhost/search-tags`, (_req, res, ctx) => {
+        const response: InferResponseBody<typeof serviceRoutes.searchTags> = {
+          tags: [
+            { id: "1", name: "Politics" },
+            { id: "2", name: "Political" },
+          ],
+        };
+        return res(ctx.status(httpStatusCodes.OK), ctx.json(response));
+      })
+    );
+
+    renderWithProviders(
+      <TagsControl
+        id="test-tags-control"
+        tags={[]}
+        votes={[]}
+        suggestionsKey="test-suggestions-key"
+        onTag={onTag}
+        onUnTag={onUnTag}
+        autocompleteDebounceMs={0}
+      />
+    );
+
+    // Act
+
+    const input = screen.getByLabelText(/tag/i);
+
+    await user.type(input, "Poli");
+    fireEvent.keyDown(input, { key: "ArrowDown" });
+    await user.type(input, "{Enter}");
+
+    // Assert
+
+    // Ensure we didn't tag both "Poli" (the text entry) and "Politics" (the first tag, selected
+    // using down-arrow.)
+    expect(onTag).toHaveBeenCalledOnceWith({ id: "1", name: "Politics" });
+  });
+  test.todo("initially shows tags with votes");
+  test.todo("initially hides tags lacking votes");
+  test.todo("shows tags lacking votes after clicking show all");
+  test.todo("shows recommended tags initially");
+  test.todo("hides recommended tags after downvoting them");
 });

premiser-ui/src/TagsControl.tsx

@@ -1,4 +1,4 @@
-import React, { useState } from "react";
+import React, { FormEvent, useState } from "react";
 import find from "lodash/find";
 import get from "lodash/get";
 import includes from "lodash/includes";
@@ -19,7 +19,6 @@ import TagsViewer from "./TagsViewer";
 import { Keys } from "./keyCodes";
 import {
   ComponentId,
-  OnEventCallback,
   OnKeyDownCallback,
   OnPropertyChangeCallback,
   SuggestionsKey,
@@ -61,17 +60,16 @@ export interface Props {
   /** Enable collapsing the tag name input */
   inputCollapsable?: boolean;
   addTitle?: string;
-  onSubmit?: OnEventCallback;
+  /** The autocomplete suggestion fetch debounce milliseconds. */
+  autocompleteDebounceMs?: number;
 }
 
-const upDownArrowKeys = [Keys.ARROW_UP, Keys.ARROW_DOWN];
-
 export default function TagsControl(props: Props) {
   const {
     tags,
     votes,
     recommendedTags,
-    commitChipKeys = [Keys.ENTER, Keys.COMMA],
+    commitChipKeys = [Keys.COMMA],
     id,
     suggestionsKey,
     votePolarity = {
@@ -84,45 +82,31 @@ export default function TagsControl(props: Props) {
     onUnTag,
     onAntiTag,
     addTitle = "Add tag",
-    onSubmit,
+    autocompleteDebounceMs,
     ...rest
   } = props;
 
   const [tagName, setTagName] = useState("");
   const [isInputCollapsed, setIsInputCollapsed] = useState(true);
-  // Whether the user has highlighted the autocomplete dropdown.
-  //
-  // react-md's AutoComplete leaves the focus in the text box, so that pressing enter
-  // both triggers an autocomplete and triggers an Enter keydown in the text input
-  // (which we would also like to use to create a tag.) To avoid both an autocomplete
-  // and submitting a partial tag name, we try to keep track of whether the dropdown is
-  // highlighted and skip the text input Enter behavior if so.
-  const [isDropdownHighlighted, setIsDropdownHighlighted] = useState(false);
 
-  const onTagNameKeyDown: OnKeyDownCallback = (event) => {
-    if (includes(upDownArrowKeys, event.key)) {
-      setIsDropdownHighlighted(true);
+  function submitTagName(event: FormEvent<HTMLFormElement>) {
+    // Stop it from submitting an enclosing form
+    event.preventDefault();
+    // Stop it from propagating to an enclosing form
+    event.stopPropagation();
+
+    addTag(tagName);
+    setTagName("");
+    if (inputCollapsable) {
+      setIsInputCollapsed(true);
     }
+  }
 
-    if (
-      !isDropdownHighlighted &&
-      includes(commitChipKeys, event.key) &&
-      tagName
-    ) {
-      // Stops ApiAutocomplete from proceeding to call onSubmit callbacks
+  const onTagNameKeyDown: OnKeyDownCallback = (event) => {
+    if (includes(commitChipKeys, event.key) && tagName) {
       event.preventDefault();
-      if (event.key === Keys.ENTER) {
-        // Stops the form from submitting via native user agent behavior
-        event.stopPropagation();
-      }
       addTag(tagName);
       setTagName("");
-      setIsDropdownHighlighted(false);
-    }
-
-    if (inputCollapsable && event.key === Keys.ENTER) {
-      setIsInputCollapsed(true);
-      setIsDropdownHighlighted(false);
     }
   };
 
@@ -133,7 +117,6 @@ export default function TagsControl(props: Props) {
   const onTagNameAutocomplete = (tag: Tag) => {
     onTag(tag);
     setTagName("");
-    setIsDropdownHighlighted(false);
   };
 
   const onClickTag = (tagName: string) => {
@@ -195,33 +178,40 @@ export default function TagsControl(props: Props) {
       addTag(tagName);
     }
     setTagName("");
-    setIsDropdownHighlighted(false);
     setIsInputCollapsed(true);
   };
 
   const tagNameAutocompleteId = combineIds(id, "tag-name");
   const extraChildren = [];
   if (!inputCollapsable || !isInputCollapsed) {
     extraChildren.push(
-      <TagNameAutocomplete
-        id={tagNameAutocompleteId}
-        key={tagNameAutocompleteId}
-        name="tagName"
-        value={tagName}
-        className="tag-name-autocomplete"
-        suggestionsKey={suggestionsKey}
-        onAutoComplete={onTagNameAutocomplete}
-        onPropertyChange={onTagNamePropertyChange}
-        onKeyDown={onTagNameKeyDown}
-        rightControls={
-          inputCollapsable ? (
-            <Button icon onClick={() => closeInput()}>
-              done
-            </Button>
-          ) : undefined
-        }
-        onSubmit={onSubmit}
-      />
+      // This form makes it easier to receive a submit only when the autocomplete listbox is closed.
+      // But it also makes it possible for us to nest forms, which is tecnically invalid HTML.
+      // Other solutions include updating AutoComplete to call onKeyDown after it has
+      // preventedDefault so that our onKeyDown could inspect that and do nothing. Or to define a
+      // custom onKeyDown that passes information about the current AutoComplete state, like whether
+      // the listbox is visible.
+      <form onSubmit={submitTagName}>
+        <TagNameAutocomplete
+          id={tagNameAutocompleteId}
+          key={tagNameAutocompleteId}
+          name="tagName"
+          value={tagName}
+          className="tag-name-autocomplete"
+          suggestionsKey={suggestionsKey}
+          onAutoComplete={onTagNameAutocomplete}
+          onPropertyChange={onTagNamePropertyChange}
+          onKeyDown={onTagNameKeyDown}
+          rightControls={
+            inputCollapsable ? (
+              <Button icon onClick={() => closeInput()}>
+                done
+              </Button>
+            ) : undefined
+          }
+          autocompleteDebounceMs={autocompleteDebounceMs}
+        />
+      </form>
     );
   }
   if (inputCollapsable && isInputCollapsed) {