Browse Source

feat(search): Add visual invalid state w/ tooltip to search (#27181)

Evan Purkhiser 3 years ago
parent
commit
5a4de29c9c

+ 89 - 44
static/app/components/searchSyntax/renderer.tsx

@@ -1,22 +1,51 @@
-import {Fragment} from 'react';
+import {Fragment, useEffect, useState} from 'react';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
+import Tooltip from 'app/components/tooltip';
 import space from 'app/styles/space';
 
 import {ParseResult, Token, TokenResult} from './parser';
+import {isWithinToken} from './utils';
 
-function renderToken(token: TokenResult<Token>) {
+type Props = {
+  /**
+   * The result from parsing the search query string
+   */
+  parsedQuery: ParseResult;
+  /**
+   * The current location of the cursror within the query. This is used to
+   * highlight active tokens and trigger error tooltips.
+   */
+  cursorPosition?: number;
+};
+
+/**
+ * Renders the parsed query with syntax highlighting.
+ */
+export default function HighlightQuery({parsedQuery, cursorPosition}: Props) {
+  const result = renderResult(parsedQuery, cursorPosition ?? -1);
+
+  return <Fragment>{result}</Fragment>;
+}
+
+function renderResult(result: ParseResult, cursor: number) {
+  return result
+    .map(t => renderToken(t, cursor))
+    .map((renderedToken, i) => <Fragment key={i}>{renderedToken}</Fragment>);
+}
+
+function renderToken(token: TokenResult<Token>, cursor: number) {
   switch (token.type) {
     case Token.Spaces:
       return token.value;
 
     case Token.Filter:
-      return <FilterToken filter={token} />;
+      return <FilterToken filter={token} cursor={cursor} />;
 
     case Token.ValueTextList:
     case Token.ValueNumberList:
-      return <ListToken token={token} />;
+      return <ListToken token={token} cursor={cursor} />;
 
     case Token.ValueNumber:
       return <NumberToken token={token} />;
@@ -28,7 +57,7 @@ function renderToken(token: TokenResult<Token>) {
       return <DateTime>{token.text}</DateTime>;
 
     case Token.LogicGroup:
-      return <LogicGroup>{renderResult(token.inner)}</LogicGroup>;
+      return <LogicGroup>{renderResult(token.inner, cursor)}</LogicGroup>;
 
     case Token.LogicBoolean:
       return <LogicBoolean>{token.value}</LogicBoolean>;
@@ -38,42 +67,48 @@ function renderToken(token: TokenResult<Token>) {
   }
 }
 
-function renderResult(result: ParseResult) {
-  return result
-    .map(renderToken)
-    .map((renderedToken, i) => <Fragment key={i}>{renderedToken}</Fragment>);
-}
-
-type Props = {
-  /**
-   * The result from parsing the search query string
-   */
-  parsedQuery: ParseResult;
-  /**
-   * The current location of the cursror within the query. This is used to
-   * highligh active tokens and trigger error tooltips.
-   */
-  cursorPosition?: number;
+const FilterToken = ({
+  filter,
+  cursor,
+}: {
+  filter: TokenResult<Token.Filter>;
+  cursor: number;
+}) => {
+  const isActive = isWithinToken(filter, cursor);
+
+  // This state tracks if the cursor has left the filter token. We initialize it
+  // to !isActive in the case where the filter token is rendered without the
+  // cursor initally being in it.
+  const [hasLeft, setHasLeft] = useState(!isActive);
+
+  // Trigger the effect when isActive changes to updated whether the cursor has
+  // left the token.
+  useEffect(() => {
+    if (!isActive && !hasLeft) {
+      setHasLeft(true);
+    }
+  }, [isActive]);
+
+  const showInvalid = hasLeft && !!filter.invalid;
+  const showTooltip = showInvalid && isActive;
+
+  return (
+    <Tooltip
+      disabled={!showTooltip}
+      title={filter.invalid?.reason}
+      popperStyle={{maxWidth: '350px'}}
+      forceShow
+    >
+      <Filter active={isActive} invalid={showInvalid}>
+        {filter.negated && <Negation>!</Negation>}
+        <KeyToken token={filter.key} negated={filter.negated} />
+        {filter.operator && <Operator>{filter.operator}</Operator>}
+        <Value>{renderToken(filter.value, cursor)}</Value>
+      </Filter>
+    </Tooltip>
+  );
 };
 
-/**
- * Renders the parsed query with syntax highlighting.
- */
-export default function HighlightQuery({parsedQuery}: Props) {
-  const rendered = renderResult(parsedQuery);
-
-  return <Fragment>{rendered}</Fragment>;
-}
-
-const FilterToken = ({filter}: {filter: TokenResult<Token.Filter>}) => (
-  <Filter>
-    {filter.negated && <Negation>!</Negation>}
-    <KeyToken token={filter.key} negated={filter.negated} />
-    {filter.operator && <Operator>{filter.operator}</Operator>}
-    <Value>{renderToken(filter.value)}</Value>
-  </Filter>
-);
-
 const KeyToken = ({
   token,
   negated,
@@ -96,13 +131,15 @@ const KeyToken = ({
 
 const ListToken = ({
   token,
+  cursor,
 }: {
   token: TokenResult<Token.ValueNumberList | Token.ValueTextList>;
+  cursor: number;
 }) => (
   <InList>
     {token.items.map(({value, separator}) => [
       <ListComma key="comma">{separator}</ListComma>,
-      renderToken(value),
+      renderToken(value, cursor),
     ])}
   </InList>
 );
@@ -114,10 +151,18 @@ const NumberToken = ({token}: {token: TokenResult<Token.ValueNumber>}) => (
   </Fragment>
 );
 
-const Filter = styled('span')`
-  --token-bg: ${p => p.theme.searchTokenBackground};
-  --token-border: ${p => p.theme.searchTokenBorder};
-  --token-value-color: ${p => p.theme.blue300};
+type FilterProps = {
+  active: boolean;
+  invalid: boolean;
+};
+
+const colorType = (p: FilterProps) =>
+  `${p.invalid ? 'invalid' : 'valid'}${p.active ? 'Active' : ''}` as const;
+
+const Filter = styled('span')<FilterProps>`
+  --token-bg: ${p => p.theme.searchTokenBackground[colorType(p)]};
+  --token-border: ${p => p.theme.searchTokenBorder[colorType(p)]};
+  --token-value-color: ${p => (p.invalid ? p.theme.red300 : p.theme.blue300)};
 `;
 
 const filterCss = css`

+ 14 - 7
static/app/components/smartSearchBar/index.tsx

@@ -258,7 +258,7 @@ type State = {
    */
   activeSearchItem: number;
   tags: Record<string, string>;
-  dropdownVisible: boolean;
+  inputHasFocus: boolean;
   loading: boolean;
   /**
    * The number of actions that are not in the overflow menu.
@@ -287,7 +287,7 @@ class SmartSearchBar extends React.Component<Props, State> {
     flatSearchItems: [],
     activeSearchItem: -1,
     tags: {},
-    dropdownVisible: false,
+    inputHasFocus: false,
     loading: false,
     numActionsVisible: this.props.actionBarItems?.length ?? 0,
   };
@@ -425,7 +425,7 @@ class SmartSearchBar extends React.Component<Props, State> {
       callIfFunction(this.props.onSearch, this.state.query)
     );
 
-  onQueryFocus = () => this.setState({dropdownVisible: true});
+  onQueryFocus = () => this.setState({inputHasFocus: true});
 
   onQueryBlur = (e: React.FocusEvent<HTMLTextAreaElement>) => {
     // wait before closing dropdown in case blur was a result of clicking a
@@ -433,7 +433,7 @@ class SmartSearchBar extends React.Component<Props, State> {
     const value = e.target.value;
     const blurHandler = () => {
       this.blurTimeout = undefined;
-      this.setState({dropdownVisible: false});
+      this.setState({inputHasFocus: false});
       callIfFunction(this.props.onBlur, value);
     };
 
@@ -578,6 +578,13 @@ class SmartSearchBar extends React.Component<Props, State> {
     if (!this.searchInput.current) {
       return -1;
     }
+
+    // No cursor position when the input loses focus. This is important for
+    // updating the search highlighters active state
+    if (!this.state.inputHasFocus) {
+      return -1;
+    }
+
     return this.searchInput.current.selectionStart ?? -1;
   }
 
@@ -1275,7 +1282,7 @@ class SmartSearchBar extends React.Component<Props, State> {
       parsedQuery,
       searchGroups,
       searchTerm,
-      dropdownVisible,
+      inputHasFocus,
       numActionsVisible,
       loading,
     } = this.state;
@@ -1323,7 +1330,7 @@ class SmartSearchBar extends React.Component<Props, State> {
     const cursor = this.getCursorPosition();
 
     return (
-      <Container ref={this.containerRef} className={className} isOpen={dropdownVisible}>
+      <Container ref={this.containerRef} className={className} isOpen={inputHasFocus}>
         <SearchLabel htmlFor="smart-search-input" aria-label={t('Search events')}>
           <IconSearch />
           {inlineLabel}
@@ -1371,7 +1378,7 @@ class SmartSearchBar extends React.Component<Props, State> {
 
         {(loading || searchGroups.length > 0) && (
           <SearchDropdown
-            css={{display: dropdownVisible ? 'block' : 'none'}}
+            css={{display: inputHasFocus ? 'block' : 'none'}}
             className={dropdownClassName}
             items={searchGroups}
             onClick={this.onAutoComplete}

+ 13 - 3
static/app/components/tooltip.tsx

@@ -72,6 +72,12 @@ type Props = DefaultProps & {
    * Should be set to true if tooltip contains unisolated data (eg. dates)
    */
   disableForVisualTest?: boolean;
+
+  /**
+   * Force the tooltip to be visible without hovering
+   */
+  forceShow?: boolean;
+
   className?: string;
 };
 
@@ -225,8 +231,10 @@ class Tooltip extends React.Component<Props, State> {
   }
 
   render() {
-    const {disabled, children, title, position, popperStyle, isHoverable} = this.props;
+    const {disabled, forceShow, children, title, position, popperStyle, isHoverable} =
+      this.props;
     const {isOpen, usesGlobalPortal} = this.state;
+
     if (disabled) {
       return children;
     }
@@ -243,7 +251,9 @@ class Tooltip extends React.Component<Props, State> {
       },
     };
 
-    const tip = isOpen ? (
+    const visible = forceShow || isOpen;
+
+    const tip = visible ? (
       <Popper placement={position} modifiers={modifiers}>
         {({ref, style, placement, arrowProps}) => (
           <PositionWrapper style={style}>
@@ -267,7 +277,7 @@ class Tooltip extends React.Component<Props, State> {
               style={computeOriginFromArrow(position, arrowProps)}
               transition={{duration: 0.2}}
               className="tooltip-content"
-              aria-hidden={!isOpen}
+              aria-hidden={!visible}
               ref={ref}
               hide={!title}
               data-placement={placement}

+ 25 - 4
static/app/utils/theme.tsx

@@ -229,12 +229,22 @@ const lightAliases = {
   /**
    * Search filter "token" background
    */
-  searchTokenBackground: '#E8F3FE',
+  searchTokenBackground: {
+    valid: '#E8F3FE',
+    validActive: color('#E8F3FE').darken(0.02).string(),
+    invalid: colors.red100,
+    invalidActive: color(colors.red100).darken(0.02).string(),
+  },
 
   /**
    * Search filter "token" border
    */
-  searchTokenBorder: '#B5DAFF',
+  searchTokenBorder: {
+    valid: '#B5DAFF',
+    validActive: color('#B5DAFF').darken(0.15).string(),
+    invalid: colors.red300,
+    invalidActive: color(colors.red300).darken(0.15).string(),
+  },
 
   /**
    * Count on button when active
@@ -647,8 +657,19 @@ const darkAliases = {
   tagBar: colors.gray400,
   businessIconColors: [colors.pink100, colors.pink300],
   badgeText: colors.black,
-  searchTokenBackground: '#1F1A3D',
-  searchTokenBorder: '#554E80',
+  searchTokenBackground: {
+    valid: '#1F1A3D',
+    validActive: color('#1F1A3D').lighten(0.05).string(),
+    invalid: color(colors.red300).darken(0.8).string(),
+    invalidActive: color(colors.red300).darken(0.7).string(),
+  },
+  searchTokenBorder: {
+    valid: '#554E80',
+    validActive: color('#554E80').lighten(0.15).string(),
+    invalid: color(colors.red300).darken(0.5).string(),
+    invalidActive: color(colors.red300).darken(0.4).string(),
+  },
+
   buttonCountActive: colors.gray100,
   buttonCount: colors.gray400,
 };

+ 1 - 0
tests/js/spec/components/events/searchBar.spec.jsx

@@ -21,6 +21,7 @@ const selectFirstAutocompleteItem = async el => {
 };
 
 const setQuery = async (el, query) => {
+  el.find('textarea').simulate('focus');
   el.find('textarea')
     .simulate('change', {target: {value: query}})
     .getDOMNode()

+ 7 - 7
tests/js/spec/components/smartSearchBar/index.spec.jsx

@@ -304,11 +304,11 @@ describe('SmartSearchBar', function () {
         />,
         options
       ).instance();
-      expect(searchBar.state.dropdownVisible).toBe(false);
+      expect(searchBar.state.inputHasFocus).toBe(false);
 
       searchBar.onQueryFocus();
 
-      expect(searchBar.state.dropdownVisible).toBe(true);
+      expect(searchBar.state.inputHasFocus).toBe(true);
     });
 
     it('displays dropdown in hasPinnedSearch mode', function () {
@@ -322,11 +322,11 @@ describe('SmartSearchBar', function () {
         />,
         options
       ).instance();
-      expect(searchBar.state.dropdownVisible).toBe(false);
+      expect(searchBar.state.inputHasFocus).toBe(false);
 
       searchBar.onQueryFocus();
 
-      expect(searchBar.state.dropdownVisible).toBe(true);
+      expect(searchBar.state.inputHasFocus).toBe(true);
     });
   });
 
@@ -340,13 +340,13 @@ describe('SmartSearchBar', function () {
         />,
         options
       ).instance();
-      searchBar.state.dropdownVisible = true;
+      searchBar.state.inputHasFocus = true;
 
       jest.useFakeTimers();
       searchBar.onQueryBlur({target: {value: 'test'}});
       jest.advanceTimersByTime(201); // doesn't close until 200ms
 
-      expect(searchBar.state.dropdownVisible).toBe(false);
+      expect(searchBar.state.inputHasFocus).toBe(false);
     });
   });
 
@@ -361,7 +361,7 @@ describe('SmartSearchBar', function () {
           />,
           options
         );
-        wrapper.setState({dropdownVisible: true});
+        wrapper.setState({inputHasFocus: true});
 
         const instance = wrapper.instance();
         jest.spyOn(instance, 'blur');

+ 1 - 0
tests/js/spec/views/issueList/searchBar.spec.jsx

@@ -171,6 +171,7 @@ describe('IssueListSearchBar', function () {
       jest.useRealTimers();
       const wrapper = mountWithTheme(<IssueListSearchBar {...props} />, routerContext);
 
+      wrapper.find('textarea').simulate('focus');
       wrapper.find('textarea').simulate('change', {target: {value: 'is:'}});
       await tick();
       wrapper.update();

+ 1 - 0
tests/js/spec/views/projectDetail/projectFilters.spec.jsx

@@ -39,6 +39,7 @@ describe('ProjectDetail > ProjectFilters', () => {
       'sentry.semver:'
     );
 
+    wrapper.find('SmartSearchBar textarea').simulate('focus');
     wrapper
       .find('SmartSearchBar textarea')
       .simulate('change', {target: {value: 'sentry.semver:'}});

+ 1 - 0
tests/js/spec/views/releases/list/index.spec.jsx

@@ -426,6 +426,7 @@ describe('ReleasesList', function () {
       'sentry.semver:'
     );
 
+    wrapper.find('SmartSearchBar textarea').simulate('focus');
     wrapper
       .find('SmartSearchBar textarea')
       .simulate('change', {target: {value: 'sentry.semver:'}});