Multi-line kql bar

[snide]

Summary

Fixes #36140

Makes the kql bar use a textarea isntead of a text input. Does some juggling to manage the focus state and change the height dynamically as people edit long form queries. Also hides Discover's date picker when the query bar is focused. The later change requires the need to no longer auto focus the query bar, which seems like an acceptable trade off.''

Focus hides the date picker

Content can now wrap multiple lines. The height adjusts appropriately

Tests

Dunno what would need to be added test wize. This should be a pretty simple change from an interface level. If there are tests to be added, I'll likely need help from engineering.

Checklist

Delete any items that are not applicable to this PR.

• [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
• This was checked for cross-browser compatibility, including a check against IE11

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[cchaos]

This is pretty slick and overall I think a good solution for long queries.

A couple things that I found:

1. The whole input steals the up/down arrow keys for traversing the auto-complete dropdown. Kind of annoying when you have more than one line. Is it possible to check if the auto-complete is open before steeling the up/down arrow keys?
2. Should the enter key also collapse the input? Maybe moves the focus to the "Update" button?
3. Tabbing out of the input doesn't reduce the width back. Meaning, the time picker doesn't come back when you tab to it.
4. Super minor, but there's an interesting spacing difference between the date/picker and the Update button versus when the Kquery bar is fully open.
   
   Don't mind the styles, I am in the K8 theme and didn't want to switch back yet
5. There's just a slight weirdness with the right border. Maybe the KQL button also needs a border to keep it separate from the cut off text.

[snide]

All good feedback. I think I can solve most of them. The up/down one is interesting.

[cchaos]

Yeah I think it would just be moving the preventDefault() here into the if (isSuggestionsVisible) block.

kibana/src/plugins/data/public/ui/query_string_input/query_string_input.tsx

Lines 266 to 273 in 4c227b2

	case KEY_CODES.DOWN:
	event.preventDefault();
	if (isSuggestionsVisible && index !== null) {
	this.incrementIndex(index);
	} else {
	this.setState({ isSuggestionsVisible: true, index: 0 });
	}
	break;

[snide]

@cchaos I think I got all your issues. Here's a vid so you don't need to pull it down. https://snid.es/2020JUN/eJ00p18NjH5jOQ48.mp4

[snide]

@XavierM @spong

I HIGHLY suggest taking a look at the state changes in this code when you run through here. At best this state is improperly named. isSuggestionsVisible is more accurately "mightHaveSuggestions". There are times when it is true when the dropdown suggestions are not actually visible. My guess is that no suggestions are present, so nothing renders. So again, "mightHaveSuggestions" seems more accurate.

My logic works, but I wouldn't be suprised if the next person who reads this code will likely run into an hour of headscratching like I did trying to figure out what is wrong.

I did not change anything here related to the state change, because this is a big codebase and I didn't want to muck it up, but one of you should check it out. I could only diagnose the problem by adding a bunch of console logs and seeing how the state changed in various keyEvents

[snide]

This is done on the design code / functionality side. I'm passing this over to @XavierM to fix the tests (it has to do with value/child changes now that it's a textarea). Please let me know before Friday if you notice errors in the actual functionality as I'll be gone the two weeks before FF.

[XavierM]

This is done on the design code / functionality side. I'm passing this over to @XavierM to fix the tests (it has to do with value/child changes now that it's a textarea). Please let me know before Friday if you notice errors in the actual functionality as I'll be gone the two weeks before FF.

Thanks a lot, @snide, I hope that this commit 7ce67a9 will make everything green for us. If not I will come back to it ;)

[snide]

@elasticmachine merge upstream

[lizozom]

While playing around with the feature, I managed to get to a state where instead of expanding, the text area showed a scroll bar.

Is this OK?

[lizozom]

@snide
Seems like we need a auto sizing text area for #24647 too, why don't we add an autosize option to EUI? See this as an example https://www.jacklmoore.com/autosize/

[randomuserid]

While playing around with the feature, I managed to get to a state where instead of expanding, the text area showed a scroll bar.

Is this OK?

I think if there were a scroll bar people would still copy & paste between an editor in order to be able to see the search when there is branching or complex logic or a long series of tests.

[cchaos]

@lizozom There is a WIP PR for adding this behavior directly to EuiTextarea, but there's a lot more to it than this "simpler" version because of all the permutations EuiTextarea supports. Eventually it'll get picked back up, but not before 7.9.

[XavierM]

@elasticmachine merge upstream

[XavierM]

@elasticmachine merge upstream

[lizozom]

Tested and working well now.
Also resolved merge conflicts 🙇

[XavierM]

@elasticmachine merge upstream

[XavierM]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: fc9852d

Build metrics

✅ unchanged

History

• 💔 Build #59754 failed 101b10e
• 💚 Build #59331 succeeded 709caaf
• 💔 Build #59239 failed 8a0ba6b
• 💚 Build #59039 succeeded 95683af
• 💔 Build #58763 failed 98b256c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream