Browse Source

ref(smartsearch): improve keyboard navigation (#35966)

* ref(smartsearch): improve keyboard navigation

* ref(smartsearch): preserve open dropdown if there is no query

* fix: specifically blur on esc key + dropdown showing only

* fix remaining tests

Co-authored-by: jennmueng <ldrwarlock@gmail.com>
Jonas 2 years ago
parent
commit
75835f56c3

+ 17 - 4
static/app/components/smartSearchBar/index.tsx

@@ -72,6 +72,8 @@ const ACTION_OVERFLOW_STEPS = 75;
 
 const makeQueryState = (query: string) => ({
   query,
+  // Anytime the query changes and it is not "" the dropdown should show
+  showDropdown: true,
   parsedQuery: parseSearch(query),
 });
 
@@ -273,11 +275,16 @@ type State = {
    */
   query: string;
   searchGroups: SearchGroup[];
+
   /**
    * The current search term (or 'key') that that we will be showing
    * autocompletion for.
    */
   searchTerm: string;
+  /**
+   * Boolean indicating if dropdown should be shown
+   */
+  showDropdown: boolean;
   tags: Record<string, string>;
   /**
    * Indicates that we have a query that we've already determined not to have
@@ -306,6 +313,7 @@ class SmartSearchBar extends Component<Props, State> {
 
   state: State = {
     query: this.initialQuery,
+    showDropdown: false,
     parsedQuery: parseSearch(this.initialQuery),
     searchTerm: '',
     searchGroups: [],
@@ -567,14 +575,14 @@ class SmartSearchBar extends Component<Props, State> {
       callIfFunction(this.props.onSearch, this.state.query)
     );
 
-  onQueryFocus = () => this.setState({inputHasFocus: true});
+  onQueryFocus = () => this.setState({inputHasFocus: true, showDropdown: true});
 
   onQueryBlur = (e: React.FocusEvent<HTMLTextAreaElement>) => {
     // wait before closing dropdown in case blur was a result of clicking a
     // menu option
     const blurHandler = () => {
       this.blurTimeout = undefined;
-      this.setState({inputHasFocus: false});
+      this.setState({inputHasFocus: false, showDropdown: false});
       callIfFunction(this.props.onBlur, e.target.value);
     };
 
@@ -760,7 +768,7 @@ class SmartSearchBar extends Component<Props, State> {
     evt.preventDefault();
     const isSelectingDropdownItems = this.state.activeSearchItem > -1;
 
-    if (!isSelectingDropdownItems) {
+    if (!this.state.showDropdown) {
       this.blur();
       return;
     }
@@ -776,6 +784,7 @@ class SmartSearchBar extends Component<Props, State> {
 
     this.setState({
       activeSearchItem: -1,
+      showDropdown: false,
       searchGroups: [...this.state.searchGroups],
     });
   };
@@ -1450,6 +1459,10 @@ class SmartSearchBar extends Component<Props, State> {
     this.onAutoCompleteFromAst(replaceText, item);
   };
 
+  get showSearchDropdown(): boolean {
+    return this.state.loading || this.state.searchGroups.length > 0;
+  }
+
   render() {
     const {
       api,
@@ -1579,7 +1592,7 @@ class SmartSearchBar extends Component<Props, State> {
           )}
         </ActionsBar>
 
-        {(loading || searchGroups.length > 0) && (
+        {this.state.showDropdown && (
           <SearchDropdown
             css={{display: inputHasFocus ? 'block' : 'none'}}
             className={dropdownClassName}

+ 2 - 0
tests/js/spec/components/smartSearchBar/index.spec.jsx

@@ -1005,6 +1005,8 @@ describe('SmartSearchBar', function () {
 
       mockCursorPosition(searchBarInst, 8);
 
+      searchBar.find('textarea').simulate('focus');
+
       searchBarInst.updateAutoCompleteItems();
 
       await tick();

+ 27 - 23
tests/js/spec/views/issueList/searchBar.spec.jsx

@@ -13,9 +13,6 @@ describe('IssueListSearchBar', function () {
     organization: {access: [], features: []},
   });
 
-  const clickInput = searchBar =>
-    searchBar.find('textarea[name="query"]').simulate('click');
-
   const mockCursorPosition = (wrapper, pos) => {
     const component = wrapper.find('SmartSearchBar').instance();
     delete component.cursorPosition;
@@ -51,15 +48,7 @@ describe('IssueListSearchBar', function () {
   });
 
   describe('updateAutoCompleteItems()', function () {
-    beforeAll(function () {
-      jest.useFakeTimers();
-    });
-
-    afterAll(function () {
-      jest.useRealTimers();
-    });
-
-    it('sets state with complete tag', function () {
+    it('sets state with complete tag', async () => {
       const loader = (key, value) => {
         expect(key).toEqual('url');
         expect(value).toEqual('fu');
@@ -74,13 +63,17 @@ describe('IssueListSearchBar', function () {
       };
       const searchBar = mountWithTheme(<IssueListSearchBar {...props} />, routerContext);
       mockCursorPosition(searchBar, 5);
-      clickInput(searchBar);
-      jest.advanceTimersByTime(301);
+
+      searchBar.find('textarea').simulate('click');
+      searchBar.find('textarea').simulate('focus');
+
+      await tick();
+
       expect(searchBar.find('SearchDropdown').prop('searchSubstring')).toEqual('"fu"');
       expect(searchBar.find('SearchDropdown').prop('items')).toEqual([]);
     });
 
-    it('sets state when value has colon', function () {
+    it('sets state when value has colon', async () => {
       const loader = (key, value) => {
         expect(key).toEqual('url');
         expect(value).toEqual('http://example.com');
@@ -97,17 +90,22 @@ describe('IssueListSearchBar', function () {
       };
 
       const searchBar = mountWithTheme(<IssueListSearchBar {...props} />, routerContext);
+
       mockCursorPosition(searchBar, 5);
-      clickInput(searchBar);
+
+      searchBar.find('textarea').simulate('click');
+      searchBar.find('textarea').simulate('focus');
+
+      await tick();
+
       expect(searchBar.state.searchTerm).toEqual();
       expect(searchBar.find('SearchDropdown').prop('searchSubstring')).toEqual(
         '"http://example.com"'
       );
       expect(searchBar.find('SearchDropdown').prop('items')).toEqual([]);
-      jest.advanceTimersByTime(301);
     });
 
-    it('does not request values when tag is `timesSeen`', function () {
+    it('does not request values when tag is `timesSeen`', async () => {
       // This should never get called
       const loader = jest.fn(x => x);
 
@@ -120,15 +118,18 @@ describe('IssueListSearchBar', function () {
         onSearch: jest.fn(),
       };
       const searchBar = mountWithTheme(<IssueListSearchBar {...props} />, routerContext);
-      clickInput(searchBar);
-      jest.advanceTimersByTime(301);
+
+      searchBar.find('textarea').simulate('click');
+      searchBar.find('textarea').simulate('focus');
+
+      await tick();
+
       expect(loader).not.toHaveBeenCalled();
     });
   });
 
   describe('Recent Searches', function () {
     it('saves search query as a recent search', async function () {
-      jest.useFakeTimers();
       const saveRecentSearch = MockApiClient.addMockResponse({
         url: '/organizations/org-slug/recent-searches/',
         method: 'POST',
@@ -149,8 +150,11 @@ describe('IssueListSearchBar', function () {
       };
       const searchBar = mountWithTheme(<IssueListSearchBar {...props} />, routerContext);
       mockCursorPosition(searchBar, 5);
-      clickInput(searchBar);
-      jest.advanceTimersByTime(301);
+      searchBar.find('textarea').simulate('focus');
+      searchBar.find('textarea').simulate('click');
+
+      await tick();
+
       expect(searchBar.find('SearchDropdown').prop('searchSubstring')).toEqual('"fu"');
       expect(searchBar.find('SearchDropdown').prop('items')).toEqual([]);