Euify and Reactify Query Bar Component

[Bargs]

Implements query bar portion of https://elastic.github.io/eui/#/layout/header. Filter bar will come in another PR.

Fixes #14086

Re-implements our query bar component in React using some EUI components. Existing typeahead and suggestion styles were copied over 1:1 for now after talking with Dave about it. In this PR I focused on reaching feature parity with the existing query bar. Some additional work would be needed before we could move this into EUI as a generic component that could be consumed by other plugins.

Still needs some new tests and I suspect some old tests will need to be updated, but other than that this PR is functionally complete and ready for reviews.

[trevan]

Could you "virtualize" the suggestions with something like react-virtualized?

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[Bargs]

@trevan was already working on it ;) 3e8ad02

[Bargs]

BTW I just tested this on my large fields test case and it resolved the issues we discussed in #14086

Let me know if you still see any issues.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[cjcenizal]

Could I make a suggestion? (Pardon the pun). Here's how I've seen a lot of components formatted both in EUI and in Kibana:

export function Suggestion({
  selected,
  suggestion,
  onMouseEnter,
  onClick,
  innerRef,
  ariaId,
}) {
  const {
    text,
    description,
    type,
  } = suggestion;

  const classes = classNames('typeahead-item', { active: selected });
  const itemClasses = `suggestionItem suggestionItem--${type}`;

  return (
    <div
      className={classes}
      role="option"
      onClick={() => onClick(suggestion)}
      onMouseEnter={onMouseEnter}
      ref={innerRef}
      id={ariaId}
    >
      <div className={itemClasses}>
        <div className="suggestionItem__type" type={type}>
          <EuiIcon type={getEuiIconType(type)}/>
        </div>
        <div className="suggestionItem__text">{text}</div>
        <div
          className="suggestionItem__description"
          // Description currently always comes from us and we escape any potential user input
          // at the time we're generating the description text
          // eslint-disable-next-line react/no-danger
          dangerouslySetInnerHTML={{ __html: description }}
        />
      </div>
    </div>
  );
}

The notable differences are the use of destructuring and the extraction of calculated values out of the JSX itself. At first I didn't like the use of destructuring but it grew on me because it helped me identify which props I was using up-front, and it also reduced noise in the JSX. Just a suggestion, feel free to disregard.

[Bargs]

I'm mostly cool with destructuring and happy to follow whatever conventions folks are using for react stuff already, will make the update 👍

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[snide]

Hey @Bargs nice work. Want me to pass you a PR to convert the Less to Sass? Should be a pretty quick, non-breaking switch.

[Bargs]

@snide for sure!

[timroes]

Out of interest: Where are those descriptions actually set/coming from? Could we potentially write descriptions in JSX directly, so we wouldn't need to use dangerouslySetInnerHTML here?

[Bargs]

They come from the autocomplete provider types

kibana/x-pack/plugins/kuery_autocomplete/public/autocomplete_providers/field.js

Line 16 in ecc0f5e

return `<p>Filter results that contain <span class="suggestionItem__callout">${escape(fieldName)}</span></p>`;

[timroes]

Accessibility looks fine for me. aria-activedescendant is set to a value suggestion-null even if no element is selected, but since that is not a valid id, a screenreader won't break on that. Of course it would be nicer not to set it at all if no autocompletion is selected, but that's a minor nitpick (and most likely not even related to this PR).

I notice two other kind of strange behaviors, but both of them might actually also be EUI issues:

1. If you focus the query bar, than press Tab the Options button is focused (so far so good). If you know press Enter I would expect the popover to open, but instead the search is executed, even though the Options button was focused.

2. If you open the popover, and then click into the Query Bar, the query bar is only shortly focused, before losing its focus again.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[Bargs]

Looks like the failure is a flaky test, going to retry if github will let me right now.

[timroes]

Since this exported from the actual suggestions as @Bargs showed, couldn't we just change those suggestions classes to export a JSX element instead of a string?

getDescription() {
  return (<React.Fragment>Some description that <b>could contain</b> HTML.</React.Fragment>);
}

That way we would not need to use dangerouslySetInnerHTML here and could just use that description directly. I think every place we could get rid of it, would be one less place that could expose a vulnerability in the end :-)

[Bargs]

We definitely could, but then I think we would need to update every single consumer of these suggestions. That includes APM, beats_management, and infra, in addition to this query bar component. I'm hesitant to make that many changes to code I'm not familiar with this late in the game. Since this issue predates this PR (it was "dangerously set" in angular as well) I think it would be ok to handle it in a separate PR, do you agree?

[timroes]

Yeah totally fine for me doing this in a separate PR and and making other teams aware.

[lukasolson]

GLTMTLGMLTLMGLMTLGLGLGMTLMLTMLGGLMTL!

[Bargs]

jenkins, test this

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[stacey-gammon]

Looks good. I have one comment that doesn't have to go in now, but that I find a bit annoying. I type in a new query and hit enter and the next thing that happens is the auto complete box pops up:

Almost seems like hitting enter should close that box and not re-open it until the user starts typing again. Otherwise I just executed that query, why do I need to see that box, and it kinda hides the filter bar.

One other minor comment that I find annoying. After I select the toggle to turn on Kuery, I then click into the query bar, but the first thing that is highlighted is the Options button, not the query bar itself, so it requires two clicks to start typing.

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[Bargs]

Thanks for the reviews everyone! I'm going to make a list of the non-critical suggestions to follow up on in another PR.

[trevan]

@Bargs, @cchaos

Also I noticed that there's no longer a search icon to the right of the query bar. I'm assuming this is by intention.

Yeah this is following the new design from @cchaos : https://elastic.github.io/eui/#/layout/header

I tried out the changes and I think the missing search icon is a problem. I understand it is the new design but I think it should be changed. To "refresh" the data, you could just click the search icon. Now, I have to click into the search bar and then hit enter. There needs to be some sort of icon on the bar that reruns the query.

[cchaos]

The final K7 design with the Query, Filter and Timepicker will incorporate a "Refresh/Update" button that will take into account the "dirty" states of all three or just act as an overall refresh. Including an icon isn't obvious that you need to click it to submit the query as we use a magnify icon to prefix all search inputs but they do nothing when clicked.

@Bargs Maybe we should add this "Refresh/Update" button after the query bar now, but only have it listen to the state of the query bar for now?

[trevan]

@cchaos, that works and it really is needed before this hits release.

[trevan]

@Bargs, you should also call toUser() on recentSearch if it is an object. Otherwise, react doesn't like it when the object is used in the Suggestion component.

[Bargs]

@stacey-gammon I was just going through some of the feedback and creating issues for the minor things I didn't have time for. However I couldn't reproduce this one issue you mentioned:

I type in a new query and hit enter and the next thing that happens is the auto complete box pops up

Do you have any additional details about that? Browser you were using maybe? It's not happening for me in master in Safari at least.