Add Lucene query syntax help #10928

Bargs · 2017-03-29T15:54:45Z

Initially I was going to create a separate dropdown for this help text but integrating it with the typeahead felt like a more elegant solution. The help text will appear when the query bar is empty and has focus. When the user starts typing the help text will go away and the usual typeahead suggestions appear. For now the help text is static, but I think this paves the way for more dynamic help similar to Slack's search box.

The help content still needs some design polish, after which I'll also add it to the Visualize and Dashboard query bars.

epixa · 2017-03-29T15:58:01Z

With this implementation, users won't be able to reference help if they've already started building the query. I don't think that's a deal breaker upfront, but it's something we should address at some point. This certainly still has value as-is, though.

Bargs · 2017-03-29T15:58:02Z

@cjcenizal I could use some advice on how to best go about getting the design closer to the mockup you posted. I applied some of the kui styles you suggested but I think there's still some room for improvement.

Bargs · 2017-03-29T16:04:28Z

@epixa my long term plan to address that is to make the help and suggestions more dynamic, similar to slack's search bar. In the short term I'm assuming if someone wants to copy one of the these static examples, they'll copy paste rather than try to type it in manually. That was just my line of thinking at least.

cjcenizal

Nice work! Just a couple suggestions.

cjcenizal · 2017-03-29T18:21:56Z

src/ui/public/query_help/index.html

@@ -0,0 +1,82 @@
+<div class="queryHelp">


Here are some tweaks we can make to polish the UI a bit:

<div class="queryHelp"> <h3 class="kuiSubTitle kuiVerticalRhythm"> Query Tips </h3> <p class="kuiText kuiVerticalRhythm"> The search bar at the top uses Elasticsearch’s support for Lucene <a class="kuiLink" href="http://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#query-string-syntax" target="_blank" > Query String syntax </a>. </p> <p class="kuiText kuiVerticalRhythm"> Here are some examples of how the syntax works. </p> <table class="kuiTable kuiTable--fluid kuiVerticalRhythm"> <thead> <tr> <th class="kuiTableHeaderCell">Example query</th> <th class="kuiTableHeaderCell">What it will do</th> </tr> </thead> <tbody> <tr class="kuiTableRow"> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> 200 </div> </td> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> Find requests that contain the number 200, in any field: </div> </td> </tr> <tr class="kuiTableRow"> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> status:200 </div> </td> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> Or we can search in a specific field. Find 200 in the status field: </div> </td> </tr> <tr class="kuiTableRow"> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> status:[400 TO 499] </div> </td> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> Find all status codes between 400-499: </div> </td> </tr> <tr class="kuiTableRow"> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> status:[400 TO 499] AND extension:PHP </div> </td> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> Find status codes 400-499 with the extension php: </div> </td> </tr> <tr class="kuiTableRow"> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> status:[400 TO 499] AND (extension:php OR extension:html) </div> </td> <td class="kuiTableRowCell"> <div class="kuiTableRowCell__liner"> Or HTML </div> </td> </tr> </tbody> </table> </div>

cjcenizal · 2017-03-29T19:07:58Z

src/ui/public/typeahead/partials/typeahead-items.html

@@ -5,7 +5,10 @@
  class="typeahead-items"
  >

+  <div ng-if="!typeahead.query.length" ng-transclude></div>


If you remove this ng-if and put the transclude after the suggestions, then users can access the help even when the typeahead is offering suggestions. We might need to figure out a way to let the use collapse/expand the help. I think we could do that in a subsequent PR.

<div ng-show="typeahead.isVisible()" ng-mouseenter="typeahead.setMouseover(true);" ng-mouseleave="typeahead.setMouseover(false);" class="typeahead-items" > <div ng-if="typeahead.query.length" ng-repeat="item in typeahead.getItems()" ng-class="{active: item === typeahead.active}" ng-click="typeahead.selectItem(item, $event);" ng-mouseenter="typeahead.activateItem(item);" class="typeahead-item" > {{item}} </div> <div ng-transclude></div> </div>

I played around with that idea before, I was afraid it might be too noisy. I'm down with it if you think it looks ok though.

I think it's better than hiding it. It's definitely not perfect though... could use some of @snide's magic.

Bargs · 2017-04-03T16:18:44Z

@cjcenizal I updated the styles and changed the transclude to always be present below the typeahead items.

kuiTable--fluid looked a bit odd on wide screens so I tried removing it. Let me know what you think.

With kuiTable--fluid:

Without kuiTable--fluid:

cjcenizal · 2017-04-03T16:34:54Z

@Bargs I chose kuiTable--fluid because without it the table will become less usable as screen-width grows. Here's how it looks on my 27":

I think it's OK to have empty space to the right-side of the table.

cjcenizal · 2017-04-03T16:35:39Z

@snide Thoughts on this? ^

snide · 2017-04-03T16:56:06Z

@cjcenizal Yeah, I'd keep off the full width since that page itself is full width and it'll expand into unreadability.

Bargs · 2017-04-03T17:01:57Z

Fair enough, re-added. Assuming tests pass, I believe I've taken care of all the outstanding items.

cjcenizal

Just a couple small formatting requests, and then 👍 👍 from me.

cjcenizal · 2017-04-03T17:06:37Z

src/core_plugins/kibana/public/dashboard/dashboard.html

@@ -48,7 +47,7 @@
              <span class="fa fa-search" aria-hidden="true"></span>
            </button>
          </div>
-          <kbn-typeahead-items></kbn-typeahead-items>
+          <kbn-typeahead-items><query-help></query-help></kbn-typeahead-items>


I internally debated over asking for this because it's kind of nitpicky, but I might as well... can we format this to use indentation to reflect the nested natured of these elements?

<kbn-typeahead-items> <query-help></query-help> </kbn-typeahead-items>

Certainly, not a problem. Updated!

cjcenizal · 2017-04-03T17:06:43Z

src/core_plugins/kibana/public/discover/index.html

@@ -45,7 +44,7 @@
              <span class="fa fa-search" aria-hidden="true"></span>
            </button>
          </div>
-          <kbn-typeahead-items></kbn-typeahead-items>
+          <kbn-typeahead-items><query-help></query-help></kbn-typeahead-items>


cjcenizal · 2017-04-03T17:06:46Z

src/core_plugins/kibana/public/visualize/editor/editor.html

@@ -59,7 +58,7 @@
                <span aria-hidden="true" class="fa fa-search"></span>
              </button>
            </div>
-            <kbn-typeahead-items></kbn-typeahead-items>
+            <kbn-typeahead-items><query-help></query-help></kbn-typeahead-items>


Bargs · 2017-04-03T20:58:47Z

@lukasolson mind giving this a quick look as well?

lukasolson · 2017-04-03T22:25:35Z

src/core_plugins/kibana/public/dashboard/dashboard.html

@@ -28,7 +28,6 @@
          <div class="kuiLocalSearch">
            <input
              parse-query
-              input-focus


Hmm, I'm not a huge fan of this. Now, when Discover loads, I can't get to work immediately without using my mouse to focus in on the search input.

What if, instead, we put a help icon to the right side of the search, and when you clicked on it, it brought up this same help dialog?

/cc @cjcenizal

Ultimately, if/when #10976 gets in, we could move to something more like how Google Drive does this:

Notice how there's a "Learn More" button in the bottom of the pop-up. Thoughts?

I don't think removing input-focus is actually necessary to accomplish the current experience, I just removed it when I was playing around with things. I left it out because I remember this auto-focus came up as something that hurts accessibility in the accessibility study that was done awhile back. If we want to discuss that outside of this PR, I can try putting it back in.

If auto-focus is a critical usability improvement, then I think we should keep it and figure out other ways to address the accessibility concerns, e.g. providing context to screen readers with aria attributes.

FYI @snide is working on some designs this week that incorporate both these changes and the ones in #10976.

I think ultimately if we do show this help every time the user has focus on the search bar, we should at least have a mechanism to hide it, because I can see it getting in the way, and potentially frustrating for users if they don't want to see the help any more.

To do that I think we'll need to store the user's choice in local or session storage, which we don't have a great way to do in a consistent manner in Kibana at the moment.

The only issues I have with putting a help link in the dropdown added by #10976 is that 1. it will be kind of buried and 2. this help text doesn't relate to the rest of the content of the dropdown. The dropdown manages filters while this help text is about the query bar.

It sounds like the help should be hidden by default, but easy to find. Should we put a help link in or near the query bar?

Or, we could go back to the original implementation that hid the help after the user starts typing. I think there was significantly less risk of that becoming annoying since it wasn't in the user's face any time the query bar had focus.

Fair enough. Let's just stick with the original implementation for now.

lukasolson · 2017-04-05T00:18:37Z

Hmm, looks like the query help is sticking around for me even after I start typing.

Bargs · 2017-04-05T14:39:07Z

@lukasolson yes, it was changed to be persistent at @cjcenizal's request. I can revert it if he's ok with that solution.

tbragin · 2017-04-12T13:41:14Z

I tried it out, and I think I prefer the persistent behavior.

@Bargs this is really awesome! With feature freeze having passed for 5.4, I assume this will have to go into 5.5?

Bargs · 2017-04-12T17:21:52Z

@tbragin @lukasolson @cjcenizal @snide

New update based on our convo:

I have to say I quite like it, simple and gets the job done. Let me know what you all think.

A couple notes:

If you pull down the PR, the doc link will be a 404 at the moment because there are no 6.0 docs. The link changes dynamically based on the version of Kibana you're running. Just switch 6.0 to current in the url to see which page it's pointing at.
I've only updated the discover page so far, if we can reach agreement on the design I'll go ahead and update the other pages.

cjcenizal

I think this is a great improvement!

Don't forget to update the UI Framework examples in https://github.com/elastic/kibana/tree/master/ui_framework/doc_site/src/views/local_nav. Right now they look like this:

@snide Thoughts on the use of em and various values in the SCSS?

alexfrancoeur · 2017-04-12T18:47:39Z

@Bargs what are your thoughts on adding a "don't show me this again" option for power users? I could see this being a bit annoying if you are comfortable with lucense

This is a bit minor but any thoughts on how previous queries and this in-line help could play better together?

Bargs · 2017-04-12T20:15:20Z

Thanks for the reminder @cjcenizal, I didn't know about those files.

@alexfrancoeur I removed that inline help, the newest version just provides a link to the docs in the query bar.

cjcenizal · 2017-05-03T15:09:07Z

It might be nice to establish a common icon for helping people identify links to helpful resources, but this is definitely a @snide decision.

snide · 2017-05-03T16:03:01Z

@tbragin

Do we need "Uses Lucene query syntax" always there? I can see how that can get annoying in the long run. This phrase is going to be in every screenshot of Kibana Discover going forward, think about it. Can we just have a little (i) icon or something with a mouseover that pulls up the query help?

You can always make this appear on focus of the input so that it doesn't show until it matters. If you're targeting BI users even partially, id suggest keeping that element there, otherwise people will just assume it's a general google search bar.

Id prefer we stop using the "i" buttons when we can. More specifically here because it needs to link to the docs.

Bargs · 2017-05-03T22:21:52Z

@lukasolson @cjcenizal just pushed some updates. I believe I've covered everything we've talked about. Ready for another review.

rebase, use new components
update placeholder with parenthesis
make help link show up conditionally on input focus
update no results page help link
Update dashboard and visualize pages

Bargs · 2017-05-04T14:56:30Z

jenkins, test this

cjcenizal

This is looking great! I have one suggestion for CSS organization and one for improving accessibility.

Also, would you mind updating the UI Framework examples so they demonstrate the new UX of hiding/revealing the link? You just need to add the kuiLocalSearchAssistedInput__assistanceLink class to the markup in ui_framework/doc_site/src/views/local_nav/local_nav_search.html.

cjcenizal · 2017-05-04T16:04:21Z

ui_framework/components/local_nav/_local_search.scss

@@ -28,6 +28,10 @@
  }
 }

+.kuiLocalLuceneSearchInput:focus {
+  padding-right: 13em;
+}


I'd characterize this as a custom class, specific to the way we're using the LocalSearch components in Discover and elsewhere in Kibana. Looking at it from that perspective, I think it's probably best to rename this to something like localLuceneSearchInput (reserving the kui prefix for the UI Framework classes), and put it in src/core_plugins/kibana/public/kibana.less (or somewhere within that directory).

I think the counter-point is that we want to use this help link in every instance of the LocalSearch, but I think we should still draw a line and use the UI Framework to provide generic components and guidance on patterns for using them, and use src to define more concrete rules on how these generic components are used in Kibana. What do you think @snide ?

Or just more generically name it as a modifier, which can let it still live here and be reused (since it seems pretty generic).

Normally I'd agree, but the 13em padding is really tightly coupled with the length of the string that goes in there, i.e. "Uses Lucene query syntax" is 13em wide.

But it's no biggie, we can keep it here as a modifier. It'd be easy to move if it becomes confusing.

Will other plugins be able to use this class if we put it in src/core_plugins/kibana/public/kibana.less? If yes, then I think that's fine. If no, it would be nice to keep it somewhere reusable.

By name it as a modifier, do you mean something like kuiLocalSearchInput--lucene?

If someone disables the Kibana plugin then I guess those styles might not be available. If that's a concern, then how about putting them into ui/public/styles/local_search.less?

Yep, that uses the correct modifier format. Looks good to me.

Maybe add a comment to the styles noting that 13em is the width of 'Uses Lucene query syntax' so it isn't just a magic number.

ui/public/styles/local_search.less sounds fine to me. Just to be clear, which name do you prefer in that file, localLuceneSearchInput or localSearchInput-lucene? Or something else?

Let's use the modifier format to indicate how it's related to the base class: kuiLocalSearchInput--lucene.

cjcenizal · 2017-05-04T16:17:54Z

src/core_plugins/kibana/public/discover/partials/no_results.html

@@ -26,7 +26,7 @@

    <h3>Refine your query</h3>
    <p>
-      The search bar at the top uses Elasticsearch's support for Lucene <a href="http://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#query-string-syntax" target="_blank">Query String syntax</a>. Let's say we're searching web server logs that have been parsed into a few fields.
+      The search bar at the top uses Elasticsearch's support for Lucene <a ng-href="{{queryDocLinks.luceneQuerySyntax}}" target="_blank">Query String syntax</a>. Let's say we're searching web server logs that have been parsed into a few fields.


Can you add a kuiLink class here?

👍 8d8818e

cjcenizal · 2017-05-04T16:19:16Z

ui_framework/components/local_nav/_local_search.scss

+  .kuiLocalSearchAssistedInput__assistanceLink:active {
+    display: inherit;
+  }
+


Can we replace lines 31 through 69 with this:

/** * 1. Visually hide this until the parent is focused, so that it's accessible to screen readers. */ .kuiLocalSearchInput:focus { & ~ .kuiLocalSearchAssistedInput__assistance { .kuiLocalSearchAssistedInput__assistanceLink { opacity: 1; /* 1 */ } } } .kuiLocalSearchAssistedInput { display: flex; flex: 1 1 100%; position: relative; } /** * 1. Vertically center the assistance, regardless of its height. */ .kuiLocalSearchAssistedInput__assistance { position: absolute; right: $globalFormControlHorizontalPadding; top: 50%; /* 1 */ transform: translateY(-50%); /* 1 */ } /** * 1. Visually hide this until it's focused, so that it's accessible to screen readers, and is * also keyboard-accessible. */ .kuiLocalSearchAssistedInput__assistanceLink { opacity: 0; /* 1 */ &:focus { opacity: 1; /* 1 */ } }

These changes will ensure the link is visible to screen readers, and can also be tabbed to by users who are using the keyboard to navigate.

The issue with this approach is that the link will now interfere with interactions with the input:

Good point. How about if we change the help to show up on hover as well as focus:

.kuiLocalSearchInput:hover, .kuiLocalSearchInput:focus { & ~ .kuiLocalSearchAssistedInput__assistance { .kuiLocalSearchAssistedInput__assistanceLink { opacity: 1; /* 1 */ } } }

That seems like a weird interaction to me. If the user wants to modify something at the end of their query they're going to mouse over it in order to click the spot they want to edit, but then that spot is going to suddenly disappear under the help link. The only way for them to get to what they wanted at that point is to click another part of the query and shift the cursor with the keyboard. I'd find that infuriating as a user.

It seems like we have that problem anyway:

As long as we hide that link by default, it's going to create a strange interaction when users try to click the text that it invisibly occupies. Maybe we should go back to showing it all the time? @snide what do you think?

You're right, that interaction stinks as well. Unless @snide has any good ideas, tomorrow I'll switch it back to showing all the time so this doesn't become a big time sink.

cjcenizal · 2017-05-04T16:22:02Z

ui_framework/dist/ui_framework.css

@@ -706,6 +706,7 @@ body {
  /* 2 */
  font-size: 10px !important;
  /* 2 */
+  -webkit-transition: background-color 0.1s linear;


There's something funny going on where some people have been generating CSS that adds these properties, and yet I've been generating CSS that removes them. Are you using yarn to install your NPM deps by any chance? Which version of postcss do you have installed? I have 5.2.17.

Not using yarn but I was on 5.2.11. I installed fresh, now I have 5.2.17 and I'm generating the same css as you (with the lines removed).

😅 Thank you!

…ar help link

Bargs · 2017-05-05T21:53:58Z

@cjcenizal @lukasolson alright, ready for another look.

Reverted show/hide on focus/unfocus behaviour
Moved .kuiLocalSearchInput--lucene to ui/public/styles/local_search.less
Added kuiLink class to no results page doc link

cjcenizal

LGTM! Really nice improvement.

lukasolson

LGTM!

Adds link to query syntax portion of query string query docs in the Kibana query bar.

Bargs added :Discovery v5.4.0 v6.0.0 labels Mar 29, 2017

Bargs requested a review from cjcenizal March 29, 2017 15:54

cjcenizal suggested changes Mar 29, 2017

View reviewed changes

Bargs force-pushed the queryHelp branch from 7b32bef to 5097e38 Compare April 3, 2017 16:14

cjcenizal suggested changes Apr 3, 2017

View reviewed changes

Bargs requested a review from lukasolson April 3, 2017 20:58

lukasolson reviewed Apr 3, 2017

View reviewed changes

Bargs changed the title ~~[WIP] Add inline query help text~~ Add inline query help text Apr 6, 2017

tbragin added the review label Apr 12, 2017

Bargs added v5.5.0 and removed v5.4.0 labels Apr 12, 2017

Bargs force-pushed the queryHelp branch from b947566 to 5d307bf Compare April 12, 2017 17:16

cjcenizal reviewed Apr 12, 2017

View reviewed changes

Bargs changed the title ~~Add inline query help text~~ Add Lucene query syntax help May 3, 2017

Bargs force-pushed the queryHelp branch from a22465a to 1fd0e6c Compare May 3, 2017 22:18

cjcenizal suggested changes May 4, 2017

View reviewed changes

Bargs added 12 commits May 5, 2017 17:20

Add inline help link to query bar

220c96c

fix doc link

9c06739

Update placeholder text and improve formatting

b09ec71

Add comment about em usage

3851e08

fix up classes

e1ca5ba

Only display help link when input is in focus

fb29eae

Update no results page lucene query syntax doc link to match search b…

475ecd0

…ar help link

add help link to dashboard

dfeffaa

add query help to visualize

e66293b

Don't let the link disappear as soon as the user clicks it

28a7a93

Remove show/hide on focus/unfocus behavior

30b8622

Add kui class to link

8d8818e

Bargs force-pushed the queryHelp branch from 1fd0e6c to 8d8818e Compare May 5, 2017 21:49

cjcenizal approved these changes May 5, 2017

View reviewed changes

lukasolson approved these changes May 8, 2017

View reviewed changes

Bargs merged commit 817c4b4 into elastic:master May 9, 2017

Bargs added a commit to Bargs/kibana that referenced this pull request May 9, 2017

Add Lucene query syntax help (elastic#10928)

ff4707c

Adds link to query syntax portion of query string query docs in the Kibana query bar.

Bargs mentioned this pull request May 9, 2017

[5.x] Add Lucene query syntax help (#10928) #11669

Merged

Bargs added a commit that referenced this pull request May 9, 2017

Add Lucene query syntax help (#10928) (#11669)

ab5133b

Adds link to query syntax portion of query string query docs in the Kibana query bar.

cjcenizal mentioned this pull request May 23, 2017

[UI Framework] Add support for multi-line queries to LocalNavSearch component of UI Framework. #11975

Closed

snide pushed a commit to snide/kibana that referenced this pull request May 30, 2017

Add Lucene query syntax help (elastic#10928)

e7e3d30

Adds link to query syntax portion of query string query docs in the Kibana query bar.

jimgoodwin added the release_note:enhancement label Jun 26, 2017

Add Lucene query syntax help #10928

Add Lucene query syntax help #10928

Conversation

Bargs commented Mar 29, 2017

epixa commented Mar 29, 2017 • edited Loading

Bargs commented Mar 29, 2017

Bargs commented Mar 29, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bargs commented Apr 3, 2017

cjcenizal commented Apr 3, 2017 • edited Loading

cjcenizal commented Apr 3, 2017

snide commented Apr 3, 2017

Bargs commented Apr 3, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bargs commented Apr 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasolson commented Apr 5, 2017

Bargs commented Apr 5, 2017

tbragin commented Apr 12, 2017

Bargs commented Apr 12, 2017

cjcenizal left a comment • edited Loading

Choose a reason for hiding this comment

alexfrancoeur commented Apr 12, 2017

Bargs commented Apr 12, 2017

cjcenizal commented May 3, 2017

snide commented May 3, 2017

Bargs commented May 3, 2017

Bargs commented May 4, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal May 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal May 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bargs commented May 5, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

lukasolson left a comment

Choose a reason for hiding this comment

epixa commented Mar 29, 2017 •

edited

Loading

cjcenizal commented Apr 3, 2017 •

edited

Loading

cjcenizal left a comment •

edited

Loading

cjcenizal May 4, 2017 •

edited

Loading

cjcenizal May 4, 2017 •

edited

Loading