EUI input controls visualization #16210

nreese · 2018-01-22T20:00:53Z

Migrate input controls visualization from KUI to EUI

cjcenizal

🍾 Woohoo! This is rad. I have a few suggestions in terms of UI/UX.

Validation

Is it possible to validate the fields and display them in an error state when the user hits the "Play" button but they're not configured properly? Like this:

Prefilled inputs

By the same token, it might be nice to prefill the inputs with the default index pattern and the first available field. This way the user gets some momentum by immediately being in a working state.

Form layout

@snide @cchaos Help me out here -- it feels like we should put the "Apply changes" on the far right of the form, with the "Cancel changes" button immediately to the left of it, and the "Clear form" button on the far left side of the form.

Misc spacing issues

Can we add a small spacer beneath the title of each accordion?

Can we also add a small spacer between each accordion? And it looks like the select clips the text when "Range slider" is selected.

Up and down sorting

Can we replace the up and down carets with sortUp and sortDown icons? This will distinguish these buttons from popover buttons.

nreese · 2018-01-25T18:32:06Z

@cjcenizal Thanks for the feed back. Things look much nicer with those changes

validation: There is no mechanism currently in place to tell the visualize editor that there are errors in the form and play can not be pressed. This will need to be figured out and could be complicated. I think this should be done in a separate PR to avoid doing too many things at once.

prefilled inputs: I am not a big fan of prefilled inputs unless there are default values that make sense more often than not. Why intentionally fill forms with incorrect information? Be preselecting a field, the default results could be really ugly if a field with very long values is selected. Also, you have an almost zero percent chance of preselecting the correct field.
Pre-selecting the default index does make sense. I will wait for PR #16235 to get merged. That PR contains a new method in indexPatterns for easily getting the default index that I could use here.

cjcenizal · 2018-01-25T18:36:43Z

Sure thing sounds good. In terms of prefilled inputs, I'd ping @Adrian-J, @alexfrancoeur and @elastic/kibana-visualizations to see how they're thinking about handling this in the future. Definitely not something we need to do in the near term, but if we want to run with the idea of "new visualizations are created in a working, valid state" then prefilling input controls sounds like a good idea. I don't think the goal is to necessarily guess the user's intention, but more so to keep the user's momentum going. For a new user this is nice because they can play with the control immediately without having to "figure it out". At least that's the theory... we should try testing it on real users to see how they take it. 😄

nreese · 2018-01-25T18:38:41Z

Or confuse users with values that don't make sense for their use case.

https://discuss.elastic.co/t/how-do-gauges-and-ranges-work/116797

Adrian-J · 2018-01-26T15:23:05Z

@nreese I agree with @cjcenizal on this one. The current form is very confusing to me, it's fine to start with empty input fields but it would be nice to see both handles on the slider. What if you started with the entire range selected? That seems like a reasonable starting point.

nreese · 2018-01-26T18:12:17Z

After talking with @cjcenizal, we have decided that this PR will not address any default values and should move forward just as is.

cjcenizal

Looks fantastic! 🚒

kobelb · 2018-01-29T21:32:57Z

src/core_plugins/input_control_vis/public/components/editor/control_editor.less

@@ -0,0 +1,4 @@
+.controlEditorPanel {
+  z-index: 1;


Why are we setting this z-index? We're also setting it here:
<EuiPanel grow={false} className="controlEditorPanel" style={{ zIndex: 1 }}>

I will remove the style block from the html - that one is extra.

The z-index in the css is needed otherwise the select menu for index pattern or field is behind the "control types" select

kobelb · 2018-01-29T21:33:39Z

src/core_plugins/input_control_vis/public/components/editor/controls_tab.js

+          <EuiFlexGroup>
+            <EuiFlexItem>
+              <EuiFormRow
+                id="slectControlType"


nit: "select" is misspelled

kobelb · 2018-01-29T21:51:42Z

src/core_plugins/input_control_vis/public/components/vis/range_control.js

-    } else if (inputValue > this.props.control.max) {
-      inputValue = this.props.control.max;
-    }
+  handleMinChange = (evt) => {


Did the migration to EUI cause this to need to change?

I found a bug in the master. Setting the min and max is totally broken in master. I fixed it in this PR because it was hard to judge how the min/max inputs worked/looked with the existing bug

Gotcha, seems to work well now, so looks good to me!

…be addressed

elasticmachine · 2018-01-30T03:30:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

kobelb

LGTM

* update jest tests as best as possible - found some bugs that need to be addressed * use EuiAccordion to hide/show control editor * set control editor to be open on load * small changes * fix editor jest tests * update visualization components to EUI * fix jest tests and updated range control to EUI * fix bug in range control input logic * fix button layout * fix dashboard grid resize test * add space between panels, change button order, switch to sort icons * remove style attribute from component, fix spelling of id

nreese added the Feature:Input Control Input controls visualization label Jan 22, 2018

nreese requested review from cjcenizal and kobelb January 23, 2018 01:07

cjcenizal reviewed Jan 23, 2018

View reviewed changes

nreese force-pushed the euiInputControls branch from 7a68d72 to cafd5aa Compare January 25, 2018 17:08

chrisronline mentioned this pull request Jan 25, 2018

[META] EUIfication #15721

Closed

47 tasks

cjcenizal approved these changes Jan 26, 2018

View reviewed changes

kobelb reviewed Jan 29, 2018

View reviewed changes

nreese added 11 commits January 29, 2018 19:37

update jest tests as best as possible - found some bugs that need to …

95d5eac

…be addressed

use EuiAccordion to hide/show control editor

8753ee3

set control editor to be open on load

4c5a0e3

small changes

e8c6d42

fix editor jest tests

062aa73

update visualization components to EUI

71145a0

fix jest tests and updated range control to EUI

2fb55de

fix bug in range control input logic

54d4b03

fix button layout

ad2a85a

fix dashboard grid resize test

c26d486

add space between panels, change button order, switch to sort icons

be20324

nreese force-pushed the euiInputControls branch from c713184 to be20324 Compare January 30, 2018 02:37

remove style attribute from component, fix spelling of id

97cd91b

kobelb approved these changes Jan 30, 2018

View reviewed changes

nreese merged commit 270455d into elastic:master Jan 30, 2018

nreese mentioned this pull request Jan 30, 2018

[6.x] EUI input controls visualization (#16210) #16404

Merged

nreese deleted the euiInputControls branch February 13, 2018 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EUI input controls visualization #16210

EUI input controls visualization #16210

nreese commented Jan 22, 2018

cjcenizal left a comment

nreese commented Jan 25, 2018 •

edited

Loading

cjcenizal commented Jan 25, 2018

nreese commented Jan 25, 2018 •

edited

Loading

Adrian-J commented Jan 26, 2018

nreese commented Jan 26, 2018

cjcenizal left a comment

kobelb Jan 29, 2018

nreese Jan 29, 2018

kobelb Jan 29, 2018

kobelb Jan 29, 2018

nreese Jan 29, 2018

kobelb Jan 30, 2018

elasticmachine commented Jan 30, 2018

kobelb left a comment

EUI input controls visualization #16210

EUI input controls visualization #16210

Conversation

nreese commented Jan 22, 2018

cjcenizal left a comment

Choose a reason for hiding this comment

Validation

Prefilled inputs

Form layout

Misc spacing issues

Up and down sorting

nreese commented Jan 25, 2018 • edited Loading

cjcenizal commented Jan 25, 2018

nreese commented Jan 25, 2018 • edited Loading

Adrian-J commented Jan 26, 2018

nreese commented Jan 26, 2018

cjcenizal left a comment

Choose a reason for hiding this comment

kobelb Jan 29, 2018

Choose a reason for hiding this comment

nreese Jan 29, 2018

Choose a reason for hiding this comment

kobelb Jan 29, 2018

Choose a reason for hiding this comment

kobelb Jan 29, 2018

Choose a reason for hiding this comment

nreese Jan 29, 2018

Choose a reason for hiding this comment

kobelb Jan 30, 2018

Choose a reason for hiding this comment

elasticmachine commented Jan 30, 2018

💚 Build Succeeded

kobelb left a comment

Choose a reason for hiding this comment

nreese commented Jan 25, 2018 •

edited

Loading

nreese commented Jan 25, 2018 •

edited

Loading