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

Reordering fields to enable smoother path through form #9991

Merged
merged 5 commits into from Jan 30, 2017

Conversation

Projects
None yet
3 participants
@ycombinator
Contributor

ycombinator commented Jan 21, 2017

Fixes #9505.

As suggested in the issue:

Perhaps we should move the checkbox "index contains time based events" closer to the drop-down for time-fields. For example, right next to the "Time field name" title.

I started off this PR by trying exactly this suggestion. Turns out there are parts of this form that appear and disappear depending on various checkbox states so this simple move didn't feel sufficient to me. So I took a step back and tried my best to take a more principled approach the organization/flow of fields in this form.

The new ordering of fields in this PR tries to follow these principles:

  • The most commonly-used fields in the form appear earlier (towards the top) in the form. This is to help the user get through the most common use cases as quickly as possible.

  • Fields for not-recommended and deprecated choices, in that order, are placed towards the end of the form as a way of de-prioritizing these choices for the user.

  • Choices made from fields higher up in the form only affect fields further down to avoid disturbing the user's visual flow through the form too much. This is done as much as possible; the only exception is when the deprecated option is chosen.

Also, when "Index contains time-based events" is checked, the "Time-field name" field is always shown. The field is disabled (used to be hidden) when Kibana in unable to retrieve a mapping for the index pattern. IMO, this makes the form less "jittery" by not hiding and showing elements.

In other words, the interaction with the form is smooth and the flow of making various choices is linear from the top to bottom until the user chooses the deprecated option. At that point the form gets re-jiggered a bit which might be a bit jarring to the user (but might be acceptable given the user is choosing the deprecated option anyway?).

@ycombinator ycombinator changed the title from Reordering fields to closely follow previous field choices to Reordering fields to enable smooth path through form Jan 21, 2017

ycombinator added some commits Jan 21, 2017

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 21, 2017

Contributor

If either @cjcenizal or @alt74 have some time to spare (LOL) I'd love their feedback on this PR. One consideration: At this point I don't want to completely overhaul the current UX, but more just tweak it to make it better than the status quo. A more holistic overhaul to this UX probably needs to happen, but could wait till later.

Contributor

ycombinator commented Jan 21, 2017

If either @cjcenizal or @alt74 have some time to spare (LOL) I'd love their feedback on this PR. One consideration: At this point I don't want to completely overhaul the current UX, but more just tweak it to make it better than the status quo. A more holistic overhaul to this UX probably needs to happen, but could wait till later.

@ycombinator ycombinator changed the title from Reordering fields to enable smooth path through form to Reordering fields to enable smoother path through form Jan 21, 2017

@cjcenizal

This comment has been minimized.

Show comment
Hide comment
@cjcenizal

cjcenizal Jan 23, 2017

Contributor

I think you're approaching this problem with the right set of principles in mind! As I explored this UX, I encountered a few minor issues and one larger issue.

"Missing index" error reporting

image

The message "Unable to fetch mapping. Do you have indices matching the pattern?" is too disconnected from the source of the error (the bad index pattern field). I think the solution is:

  • This button should just become disabled without changing text.
  • The index pattern field should be outlined in red.
  • There should be an error message below it that states, "Do you have indices matching this pattern? We couldn't fetch the mapping." (And what does "couldn't fetch the mapping mean"?" Doesn't it make more sense to say, "couldn't find any matching indices?")

Disabled inputs shouldn't have error state

image

If you get the Time-field Name input into an error state by focusing it and then blurring without selecting an option, it takes on an error state. This is confusing for the user because s/he's being told to fix a problem, but is then prevented from doing so. When blurring, we should reset the error states to avoid this from happening.

Clarify "Do not expand index pattern" explanatory text

image

When the option is checked, two very similar messages are visible, but one is treated as a "notification" and the other is just regular text. When I read these two messages, it's hard for me to figure out which one applies to the checked state. I think this is partially because a) the notification treatment obscures the fact that the two messages are two sides of the same coin and b) the second message is broken up into two paragraphs, and the second paragraph makes a statement that doesn't apply to the checked state but doesn't make this clear.

I think we can fix this by removing the notification treatment, and adjusting the copy to clearly identify what happens when the option is selected and not selected. Let's change this copy and keep all of it visible regardless of whether the option is checked or not:

With this option not selected, searches against any time-based index pattern that contains a wildcard will automatically be expanded to query only the indices that contain data within the currently selected time range. Searching against the index pattern logstash-* will actually query elasticsearch for the specific matching indices (e.g. logstash-2015.12.21) that fall within the current time range.

With this option selected, this index pattern will be queried directly rather than being expanded into more performant searches against individual indices. Elasticsearch will receive a query against logstash-* and will have to search through all matching indices regardless of whether they have data that matches the current time range.

FYI, if you or anyone decides to massage this copy further, this comment #8128 (comment) has some suggestions on how it should be rephrased.

Split the form up into three forms

I think we can break this form up into three distinct forms, to reduce its complexity from a UI perspective.

It looks like a user has three ways to create an index pattern:

Non-time-based

image

Time-field-based

image

Event-time-based

image

If we got rid of the "Index contains time-based events" and "Use event times to create index names" checkboxes, and replaced them with tabs, then we could surface each of the above forms individually. This could also make the logic within this view a bit simpler.

Contributor

cjcenizal commented Jan 23, 2017

I think you're approaching this problem with the right set of principles in mind! As I explored this UX, I encountered a few minor issues and one larger issue.

"Missing index" error reporting

image

The message "Unable to fetch mapping. Do you have indices matching the pattern?" is too disconnected from the source of the error (the bad index pattern field). I think the solution is:

  • This button should just become disabled without changing text.
  • The index pattern field should be outlined in red.
  • There should be an error message below it that states, "Do you have indices matching this pattern? We couldn't fetch the mapping." (And what does "couldn't fetch the mapping mean"?" Doesn't it make more sense to say, "couldn't find any matching indices?")

Disabled inputs shouldn't have error state

image

If you get the Time-field Name input into an error state by focusing it and then blurring without selecting an option, it takes on an error state. This is confusing for the user because s/he's being told to fix a problem, but is then prevented from doing so. When blurring, we should reset the error states to avoid this from happening.

Clarify "Do not expand index pattern" explanatory text

image

When the option is checked, two very similar messages are visible, but one is treated as a "notification" and the other is just regular text. When I read these two messages, it's hard for me to figure out which one applies to the checked state. I think this is partially because a) the notification treatment obscures the fact that the two messages are two sides of the same coin and b) the second message is broken up into two paragraphs, and the second paragraph makes a statement that doesn't apply to the checked state but doesn't make this clear.

I think we can fix this by removing the notification treatment, and adjusting the copy to clearly identify what happens when the option is selected and not selected. Let's change this copy and keep all of it visible regardless of whether the option is checked or not:

With this option not selected, searches against any time-based index pattern that contains a wildcard will automatically be expanded to query only the indices that contain data within the currently selected time range. Searching against the index pattern logstash-* will actually query elasticsearch for the specific matching indices (e.g. logstash-2015.12.21) that fall within the current time range.

With this option selected, this index pattern will be queried directly rather than being expanded into more performant searches against individual indices. Elasticsearch will receive a query against logstash-* and will have to search through all matching indices regardless of whether they have data that matches the current time range.

FYI, if you or anyone decides to massage this copy further, this comment #8128 (comment) has some suggestions on how it should be rephrased.

Split the form up into three forms

I think we can break this form up into three distinct forms, to reduce its complexity from a UI perspective.

It looks like a user has three ways to create an index pattern:

Non-time-based

image

Time-field-based

image

Event-time-based

image

If we got rid of the "Index contains time-based events" and "Use event times to create index names" checkboxes, and replaced them with tabs, then we could surface each of the above forms individually. This could also make the logic within this view a bit simpler.

@thomasneirynck thomasneirynck self-assigned this Jan 23, 2017

@thomasneirynck

I actually think this reordering is a step forward already.

The only thing that I also noticed what @cjcenizal mentioned, was the error-outlining of unused input. e.g.:

image

It comes across as a little strange.

As for @cjcenizal's suggestion to split up in 3 forms, that seems to be like a larger effort. Note that that would also require updating all the tutorials/getting started doc, something I think you could get away with if we just keep it at this reordering.

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 23, 2017

Contributor

Wow @cjcenizal, amazing feedback. I agree that splitting this into 3 forms to reflect the 3 possible ways of index creation is the best approach here.

However, I also agree with @thomasneirynck that this PR may not be the place to tackle that change — not just because it's a larger effort but also because I suspect the entire index creation / getting started with data flow needs a rethink (@skearns, I know you've mentioned this before so any thoughts here?).

I will fix the error highlighting on the disabled form field now.

Contributor

ycombinator commented Jan 23, 2017

Wow @cjcenizal, amazing feedback. I agree that splitting this into 3 forms to reflect the 3 possible ways of index creation is the best approach here.

However, I also agree with @thomasneirynck that this PR may not be the place to tackle that change — not just because it's a larger effort but also because I suspect the entire index creation / getting started with data flow needs a rethink (@skearns, I know you've mentioned this before so any thoughts here?).

I will fix the error highlighting on the disabled form field now.

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 23, 2017

Contributor

Created #10026 to track @cjcenizal's suggested changes.

Contributor

ycombinator commented Jan 23, 2017

Created #10026 to track @cjcenizal's suggested changes.

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 24, 2017

Contributor

@thomasneirynck @cjcenizal What steps are you taking to make the error-outlining happen? I can't seem to make it do that:

jan-24-2017 03-39-41

I have the makelogs-created logstash-0 index in Elasticsearch:

$ curl -s -u elastic 'http://l:9200/_cat/indices/log*?v'
Enter host password for user 'elastic':
health status index      uuid                   pri rep docs.count docs.deleted store.size pri.store.size
green  open   logstash-0 J4zUE0NDTs65cKcliXc5hg   1   0      14005            0     69.4mb         69.4mb
Contributor

ycombinator commented Jan 24, 2017

@thomasneirynck @cjcenizal What steps are you taking to make the error-outlining happen? I can't seem to make it do that:

jan-24-2017 03-39-41

I have the makelogs-created logstash-0 index in Elasticsearch:

$ curl -s -u elastic 'http://l:9200/_cat/indices/log*?v'
Enter host password for user 'elastic':
health status index      uuid                   pri rep docs.count docs.deleted store.size pri.store.size
green  open   logstash-0 J4zUE0NDTs65cKcliXc5hg   1   0      14005            0     69.4mb         69.4mb
@cjcenizal

This comment has been minimized.

Show comment
Hide comment
@cjcenizal

cjcenizal Jan 24, 2017

Contributor

@ycombinator First you create the error by clicking the select dropdown and then deselecting it without choosing an option. It should now take on an error state. Then either change the index pattern to an invalid one (in my case) or check "Use time events to create index names" (Thomas's case).

Contributor

cjcenizal commented Jan 24, 2017

@ycombinator First you create the error by clicking the select dropdown and then deselecting it without choosing an option. It should now take on an error state. Then either change the index pattern to an invalid one (in my case) or check "Use time events to create index names" (Thomas's case).

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 24, 2017

Contributor

Okay, got it. Thanks! Working on a fix now...

Contributor

ycombinator commented Jan 24, 2017

Okay, got it. Thanks! Working on a fix now...

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 24, 2017

Contributor

@cjcenizal @thomasneirynck This is ready for your 👀 again. Thanks!

Contributor

ycombinator commented Jan 24, 2017

@cjcenizal @thomasneirynck This is ready for your 👀 again. Thanks!

@cjcenizal

LGTM!

@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 26, 2017

Contributor

@thomasneirynck Any chance you have some time to re-review this?

Contributor

ycombinator commented Jan 26, 2017

@thomasneirynck Any chance you have some time to re-review this?

@thomasneirynck

👍

@ycombinator ycombinator merged commit 48604eb into elastic:master Jan 30, 2017

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
kibana-ci Build finished.
Details

ycombinator added a commit to ycombinator/kibana that referenced this pull request Jan 30, 2017

Reordering fields to enable smoother path through form (#9991)
* Reordering fields to closely follow previous field choices

* Moving deprecated choice field to end of form.

* Forgot to move related section

* Always show "Time-field name" field when index is time-based

* Only require the field when it's not disabled
@ycombinator

This comment has been minimized.

Show comment
Hide comment
@ycombinator

ycombinator Jan 30, 2017

Contributor

Backported to:

Contributor

ycombinator commented Jan 30, 2017

Backported to:

@ycombinator ycombinator added the v5.3.0 label Jan 30, 2017

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