Remove react-select from kibana #18876
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nreese
force-pushed
the
remove_react_select
branch
from
d6967b4 to
742f1a0
Compare
nreese
added
the
Feature:Visualizations
💔 Build Failed |
💔 Build Failed |
nreese
force-pushed
the
remove_react_select
branch
from
9c849d4 to
fa629f3
Compare
💔 Build Failed |
nreese
force-pushed
the
remove_react_select
branch
from
7d39e77 to
9a450dc
Compare
💚 Build Succeeded |
nreese
force-pushed
the
remove_react_select
branch
from
344884d to
b82070b
Compare
💔 Build Failed |
nreese
force-pushed
the
remove_react_select
branch
from
b82070b to
1e07f0f
Compare
💚 Build Succeeded |
|
I find EuiComboBox a bit weird when there is only one value allowed in it. Maybe a separate clean up that could be done in EUI directly... Hitting the
|
|
I wish we had better tests for TSVB stuff, most of these calculations mean nothing to me so I can only compare against existing version. Anyway, I think I notice something that changed and is wrong in this version. The
|
| if (pipeline) { | ||
| note = ( | ||
| <span className="vis_editor__agg_select-note"> | ||
| (requires child aggregation) |
There was a problem hiding this comment.
Looks like this isn't migrated to the new code. We okay with that? I'd say the pros far outweigh the cons on losing this little help text, just want to point it out.
| options={durationOutputOptions} | ||
| <EuiComboBox | ||
| isClearable={false} | ||
| options={durationInputOptions} |
There was a problem hiding this comment.
Any reason why you changed this from outputOptions to inputOptions? Looks like previously we didn't want pico,nano or micro as an output option (no idea why...)
There was a problem hiding this comment.
This was removed by accident. I will replace with the original options list.
@stacey-gammon That is a bug introduced in the latest version of EUI(elastic/eui#829). I have a PR out that will fix the problem so the |
@stacey-gammon Looks like I was not setting |
💔 Build Failed |
|
looks like unrelated functional test error jenkins, test this |
💚 Build Succeeded |
nreese
force-pushed
the
remove_react_select
branch
from
b332b5c to
bd10dec
Compare
💚 Build Succeeded |
|
@stacey-gammon I have updated kibana with a new version of EUI that fixes the problems you found. This PR is ready for another look |
stacey-gammon
approved these changes
May 24, 2018
timroes
reviewed
May 30, 2018
| await input.clearValue(); | ||
| await input.type(value); | ||
| await find.clickByCssSelector('.euiComboBoxOption'); | ||
| await this.closeComboBoxOptionsList(comboBox); | ||
| await remote.pressKeys('\uE004'); |
There was a problem hiding this comment.
🎉 I like when we can remove stuff like that :D
nreese
force-pushed
the
remove_react_select
branch
from
bd10dec to
d1c3a2d
Compare
💔 Build Failed |
|
Looks like unrelated discover test failure jenkins, test this |
|
cc @Bargs ^^ looks like that flaky discover test was hit above, if you want to take a look at the debug output. |
|
Don't worry about that test too much, I removed the test altogether in the Inspector PR already, since it relied on a the spy panel in discover which doesn't exist anymore. |
💚 Build Succeeded |
nreese
added a commit
to nreese/kibana
that referenced
this pull request
May 30, 2018
* remove react-select from AggSelect * update field_select to use EuiComboBox * metric_select * moving_average * percentile * series_agg * std_deviation * removing some more react-select instances * icon_select and group_by_select * gauge type * markdown * set isClearable prop * remove react-select from timeseries config * remove react-select from timeseries panel options * remove react select from terms * remove react-select from table config * remove react-select from data_format_picker * fix create_select_handler mocha test * remove react-select from kibana * update tsvb functional tests * set isDisabled prop on EuiComboBox where disabld prop was set for Select * use durationOutputOptions for duration 'to' options
nreese
added a commit
that referenced
this pull request
May 30, 2018
* remove react-select from AggSelect * update field_select to use EuiComboBox * metric_select * moving_average * percentile * series_agg * std_deviation * removing some more react-select instances * icon_select and group_by_select * gauge type * markdown * set isClearable prop * remove react-select from timeseries config * remove react-select from timeseries panel options * remove react select from terms * remove react-select from table config * remove react-select from data_format_picker * fix create_select_handler mocha test * remove react-select from kibana * update tsvb functional tests * set isDisabled prop on EuiComboBox where disabld prop was set for Select * use durationOutputOptions for duration 'to' options
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace react-select with EuiComboBox in TSVB