Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Create index pattern" wizard. #13454

Merged

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Aug 10, 2017

Replaces #13154
Resolves #12867

This PR builds upon #13154 and adds in the data searching. It does not change any part of the UI or UX.

TODO

  • Rebase and verify functionality
  • Test attaching an ID to an index pattern
  • Re-add the time fields error on the second step
  • Ensure accessibility concerns are handled, like this one

Release Note: Introducing a new wizard to create index patterns that makes index discovery and matching much easier than before. create_index_pattern_wizard_searching

From the other PR

Changes

  • Create a directive for each step in the wizard.
  • Reorganize files into conventional folder structure.
  • Rename files with more conventional and consistent naming patterns.
  • Remove all translations.
  • Display indices, partial matches, exact matches.
  • Add loading, empty, and success states.
  • Add option to include system indices.

Changes to the UX

Loading indices

create_index_pattern_wizard_loading

Searching for matching indices

create_index_pattern_wizard_searching

Submitting the new index pattern

create_index_pattern_wizard_submit

@cjcenizal
Copy link
Contributor

Looks like we need to fix the functional tests for this stuff, too. Let me know if you need any help with this!

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slow_clap_picard

Looks great man!

@alexfrancoeur
Copy link

This PR has hopped around a lot 😄 Love the changes described above. I wanted to make sure my comments and feedback were carried over to the new PR. I believe some of it is still relevant. Linking here: #13154 (comment)

Also adding @skearns64 to the thread.

@cjcenizal
Copy link
Contributor

@alexfrancoeur Would you mind copying over just the specific comments you think should block this?

@alexfrancoeur
Copy link

alexfrancoeur commented Aug 11, 2017

I already synced with Chris on this and quickly categorized as a Google doc. I split my original feedback into three categories - High priority, usability enhancements and future. I would consider high priority closest to "blockers". Personally I find the usability enhancements equally as important to make this a good experience for new and existing users. Some are likely easier to implement than others. Categorized feedback below.

Highest Priority

Defining an index pattern + filtering

I'm struggling with how this can best work. If you look at the below screenshot, we are saying that an index pattern matches in it's current state. While the text matches the indices, the index pattern won't actually match the indices without a wildcard

The fact that we are showing success and allowing a user to move on to the next step with an error is not the focus here. The focus of this "blocker" should be on how we're filtering the table & creating an index pattern simultaneously. Please see next comment

screen shot 2017-08-04 at 2 36 46 pm

screen shot 2017-08-04 at 2 41 39 pm

What if we automatically appended a wildcard as soon as they start typing? A user could remove it if they'd like but at least the initial experience would work as expected. Otherwise, we are specifying a single index which I believe is used much less than an index pattern with a wild card (but may be wrong).

Hide optional ID

I'm not sure we want to advertise the optional ID so much. We prefer users use our unique ID. Can we add an advanced settings option here to hide it a bit? I believe this was in the original PR.

screen shot 2017-08-04 at 2 42 22 pm

Usability Enhancements

Refreshing of table

There seems to be a refresh of the entire table as I type for an index pattern. Is there any way we can dynamically filter the results in the index table without doing a complete refresh?

aug-04-2017 14-25-17

Highlighting where index patterns match the indices

It'd be great to show where the pattern matches the index. Any chance we can bold where it matches?

screen shot 2017-08-04 at 2 33 20 pm

How we surface time filters and present as "optional"

I'm not sure "Optional Time filter field name" is the best header here. Especially given the fact that making a selection here is a blocker for me moving forward. Also, the option to "not use a time filter" is pretty hidden. I wouldn't think to click into this drop down to create an index without a time filter. This is an issue with the current index pattern creation view as well.

screen shot 2017-08-04 at 2 44 10 pm

screen shot 2017-08-04 at 2 44 26 pm

What if we called the title "Select time field" where the dropdown provides all available options, with a checkbox that would disable this drop down and allow a user to create an index without a time field. @cjcenizal @snide I believe there is some history here for why this option is in the dropdown. Any additional info you can provide or suggestions for a better approach?

Future PR's

There have been a number of designs from @alt74 around the index pattern management page for K7. On the design side of things, we seem to be in agreement that this information architecture is a good approach. There are still some kinks to work out but at a high level, the single step / side by side approach is preferred. What are your thoughts around the work effort to implement for K6? I'll set aside some time early next week to discuss in more detail.

screen shot 2017-08-04 at 2 57 15 pm

@alexfrancoeur
Copy link

@chrisronline @cjcenizal

I've taken a look at the most recent PR. I'd say all of my feedback still stands and I want to add some additional color around the top "blocker" I had mentioned in my comment above.

screen shot 2017-08-11 at 11 40 31 am

In the above image, I've typed in an index pattern "logs" and at first glance I do not understand why I can't click next. The table below appears to be filtered on text that I've typed. This is clearly due to the fact that I only have logstash indices but I imagine a new user will be in a similar situation. As we all know, the first reaction from a user is not to read. The warning in blue does not stand out to me as an erroneous index pattern, which I'm assuming is intentional. We shouldn't be showing failures as soon as a user lands in Kibana.

I'd like to propose a solution that I think may work. Upon typing an index pattern initially, we could automatically append the wildcard that is constantly shown as a pre-selected text element. If you were to hit backspace, the first character deleted would be the asterisk and at that point you're on your own and can customize accordingly.

image

The indices in the table below would simply be filtered as they are today and the text in the input field would be your index pattern. If they choose to remove the asterisk, the table below would be filtered on whatever is in that input box. This promotes a good getting started experience, a full understanding of the index pattern they've created and a quick and easy escape hatch for the edge cases / "advanced" index patterns. With this approach, we could also begin to highlight where the index pattern matches an index in the table.

@chrisronline
Copy link
Contributor Author

Pushed up a couple changes:

  1. Added the wildcard to the search query after the user starts typing
    out

  2. Highlighting the search query in the result set:

screen shot 2017-08-11 at 2 08 24 pm

Let's see how these feel

@cjcenizal
Copy link
Contributor

Nice! Great idea @alexfrancoeur. I really like how the user falls into a successful state from the get-go.

@chrisronline I found a minor bug. In the screenshot below you can see I've typed "log" but it looks like only "lo" has been bolded.

image

Also, let me know if you need help resolving the merge conflicts. Looks like it's just UI Framework stuff.

@chrisronline
Copy link
Contributor Author

Nice find @cjcenizal! I'll fix that up

@chrisronline
Copy link
Contributor Author

Another update.

Added the request to auto hide the index pattern id controls:

screen shot 2017-08-11 at 3 08 12 pm

screen shot 2017-08-11 at 3 08 16 pm

<a
class="kuiLink"
ng-click="stepTimeField.toggleAdvancedOptions()"
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't keyboard accessible (can't be tabbed to) because it doesn't have an href on it. We just need to add kbn-accessible-click as an attribute to solve this problem.

I'd suggest removing the <small> element and placing this link and the "index pattern name" field beneath the time field filter name field, but I'd also like to hear @snide's thoughts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I'll wait to hear back from @snide on the latter, but I'll fix up the former.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually really like the second suggestion too. I went ahead and added it.

Do you think we should toggle the text between Show and Hide?

screen shot 2017-08-14 at 11 01 27 am

screen shot 2017-08-14 at 11 01 31 am

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Good idea about switching between "Show" and "Hide" language. If we're going that far, how about using the ToggleButton component from the UI framework instead of a link?

image

@chrisronline
Copy link
Contributor Author

Added @cjcenizal suggestion around where to put the advanced fields on the second page.

Do you think we should toggle the text between Show and Hide?

screen shot 2017-08-14 at 11 01 27 am

screen shot 2017-08-14 at 11 01 31 am

@chrisronline
Copy link
Contributor Author

Per @cjcenizal's suggestion, we're going to use the toggle button from the ui framework:

screen shot 2017-08-14 at 2 07 06 pm

screen shot 2017-08-14 at 2 07 01 pm

@alexfrancoeur
Copy link

alexfrancoeur commented Aug 14, 2017

Hey @chrisronline - loving the progress! I wanted feedback from @cjcenizal and yourself on a thought I had.

aug-14-2017 15-04-57

In the above gif, you'll notice that I entered an index pattern and then the next screen I see is immediately red because I hadn't selected a time filter from the drop down. It don't believe it's a good experience to see red with one of your firs time in the product. What do you think of either of these approaches?

  • Intelligently choose a time filter as the time series visual builder does? @timestamp or next possible datetime field. This could then be adjusted if need be.
  • Don't highlight red or create a warning until you hit "create index pattern"

@chrisronline
Copy link
Contributor Author

@alexfrancoeur That's a great point! I just pushed up a fix so the red border will not appear. It shouldn't ever appear because the field is optional

@cjcenizal
Copy link
Contributor

cjcenizal commented Aug 14, 2017

@chrisronline One issue I just noticed is that with this change to remove required from the time field select, is that the "Submit" button defaults to being clickable.

image

If we go down this path, I think we should remove the "I don't want to use a Time Field" option, which would help this UX make a little more sense.

I think it might also be helpful to add more information to help text for this control when there's no time field selected: "The Time Filter will use this field to filter your data by time. Without a time field selected, you won't be able to use the date picker or create time-based visualizations with this index pattern's data." Then when a time field is selected, this text can revert back to the current text, "The Time Filter will use this field to filter your data by time." @alexfrancoeur What do you think?

@alexfrancoeur
Copy link

alexfrancoeur commented Aug 15, 2017

@chrisronline

That's a great point! I just pushed up a fix so the red border will not appear. It shouldn't ever appear because the field is optional

So oddly enough, the input itself is not optional. A user needs to define a timeframe or choose not to use a timeframe. The initial experience & labeling of this is honestly a bit confusing in Kibana today. I had mentioned this in my usability feedback (one of the last bullet points). @cjcenizal also touches upon this in his most recent comment.

What do you two think of this proposal?

  • Make this input box required and remove the I don't want to use the Time Filter option
  • Only turn border of the drop down red if the "Create index pattern" page is clicked with no input selected
  • Under advanced options, add a check box that says "Ignore time filter", "I don't want to use a time filter" or something along those lines
  • If there is only one datetime field available, we can make it easy for a user and automatically select it

A few other comments:

  • Thoughts on formatting the title, description and input box a bit differently? I feel as if normally, these types of forms will normally show a title, provide sub-text that describes what goes in the dropdown/input box and then the interactive component
  • Advanced options needs additional descriptive text, example below.

Custom index pattern ID
Kibana will provide a unique identifier for each index pattern. If you do not want to use this unique ID, customize the ID below.
[ input box ]

  • We have users with thousands of indices. If we haven't already, we should test how this feature performs with a ridiculous amount of indices to filter through
  • I know we aren't tackling the auto-suggested index templates for index patterns with this issue, do we have another open for tracking this request?

@cjcenizal
Copy link
Contributor

@alexfrancoeur I think that introducing another control to the UI will add more complexity to the form. I think the problem we're trying to solve is to communicate to the user that they are required to choose either a time field or no time field. I think the wording "Optional" in the label is confusing, and the messaging around this control doesn't communicate this requirement. I think we can solve the problem and keep things simple by removing the word "Optional" from the label and by updating the help text with a note like "You can choose not to have a time field, but... {insert caveats here}". What do you think?

In terms of the layout of the form, the "Label -> control -> help text" pattern is what we've established in K6 and we're carrying through to K7. I think we should stick with this for consistency.

I think we should also stick with the pattern of making the submit button active only after the required options have been selected. This is how the first step in the wizard behaves, so I think it should behave the same here, too.

@alexfrancoeur
Copy link

@cjcenizal sounds good to me. Assuming @chrisronline is on board, I'm 👍 on all 3 points.

👍 I agree that adding additional options in the advanced settings adds more complexity. I'm all for modifying to the text to be more descriptive and am comfortable keeping the behavior as is. We have not had many complaints from the community about this directly it's just always been an awkward experience for myself.

👍 on consistency with the layout in K6 and K7

👍 disabling the submit button if nothing is selected from the dropdown menu

@chrisronline
Copy link
Contributor Author

Great thoughts! Made some updates.

Here is how it will look for an index without any date field:
screen shot 2017-08-16 at 10 29 43 am

Here is the flow for an index with at least one date field:
screen shot 2017-08-16 at 10 33 50 am
screen shot 2017-08-16 at 10 33 55 am
screen shot 2017-08-16 at 10 34 00 am

@chrisronline
Copy link
Contributor Author

Tagging @tylersmalley to take a look

@chrisronline chrisronline force-pushed the enhancement/index_pattern_page branch from 46508a4 to 9b27cf1 Compare August 16, 2017 18:13
@cjcenizal
Copy link
Contributor

@chrisronline Looks great! For the first screenshot, I'd just suggest changing the copy to "The indices which match this pattern don't contain any time fields. Add documents to these indices to specify a Time Filter field name." This way the user has an idea of what they're missing out on and what they can do about it.

@chrisronline
Copy link
Contributor Author

@tylersmalley Great feedback! Fixed up and I updated from master so we'll see how the build goes.

@chrisronline
Copy link
Contributor Author

jenkins, test this

@alexfrancoeur
Copy link

@chrisronline I hope I'm not too late here, and I'm sure your sick of my feedback but I noticed two more issues I'd like to comment on.

We worked so hard to remove the red border on the time filter selection, any chance we do the same for the index pattern page? Same scenario, we're showing an error before the user has even done anything.

aug-22-2017 16-02-42

I think we can remove "Optional" from the header here. We removed the text everywhere else. "Settings" or "Configure settings" could be some of the options. I prefer "Configure settings" or something along those lines as it's a call to action. Similar to "Define index pattern" seen on the previous step.

screen shot 2017-08-22 at 4 03 48 pm

@chrisronline
Copy link
Contributor Author

@alexfrancoeur Never too late! Thanks for the feedback. I'll clean that up

@tylersmalley
Copy link
Contributor

tylersmalley commented Aug 23, 2017

Addresses #10438 and #10440.

@alexfrancoeur
Copy link

Is #5053 really addressed? Seems like this is more related to #13582

@tylersmalley
Copy link
Contributor

Ah, you're right @alexfrancoeur. Removing from the comment so Github doesn't auto-close.

@tylersmalley
Copy link
Contributor

tylersmalley commented Aug 24, 2017

@chrisronline, changes look good. Just one thing I previously missed, can we debounce the index list? Even 100ms would help.

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - debouncing the index list would be nice, but I don't see it being a requirement.

@chrisronline chrisronline merged commit 9175137 into elastic:master Aug 24, 2017
chrisronline added a commit that referenced this pull request Aug 24, 2017
* Create Index Pattern Creation wizard.
- Create a directive for each step in the wizard.
- Reorganize files into conventional folder structure.
- Rename files with more conventional and consistent naming patterns.
- Display indices, partial matches, exact matches.
- Add loading, empty, and success states.
- Add option to include system indices.

* Temp: comment out and mock getIndices functionality.

* Hook up data seaching

* Add for/id connections for form inputs and labels

* Automatically append a wildcard

* Highlight the index pattern in the results

* Ensure we only remove the last character if it's a `*`

* Auto hide index pattern id controls

* Ensure this link is tabbable

* Move the advanced fields down

* Use toggle button

* This shouldn't ever be required

* Revert "This shouldn't ever be required"

This reverts commit b6e31e79308271e7f04ea1d42ce66e32e7aa0612.

* Update based on comments in PR

* Ffew more changes

* Port changes from Tyler's PR, #13353

* Remove unnecessary file

* Fix broken functional tests

* Copy changes

* Fix functional tests

* Remove loading from the main select, and move to an additional select

* Show help text when loading

* Fix sorting

* Fixing broken functional tests

* Couple changes from PR review

* Ensure input field does not show a red border until touched

* More descriptive and consistent copy
@chrisronline
Copy link
Contributor Author

Backport
6.x: 4885ca1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants