[Vis: Default editor] EUIficate order_agg param editor #36984
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
💔 Build Failed |
💔 Build Failed |
sulemanof
force-pushed
the
EUIfication/order_agg
branch
from
5a8f942 to
d952546
Compare
💔 Build Failed |
💔 Build Failed |
sulemanof
commented
May 24, 2019
| return aggFilter.includes(`!${agg.type.name}`); | ||
| }; | ||
|
|
||
| $scope.$watch('agg.params.field.type', (type) => { |
There was a problem hiding this comment.
This watcher was moved to the missing_bucket.tsx useEffect function
sulemanof
commented
May 24, 2019
| @@ -174,24 +149,11 @@ export const termsBucketAgg = new BucketAggType({ | |||
| const orderBy = params.orderBy; | |||
| const paramDef = agg.type.params.byName.orderAgg; | |||
|
|
|||
| // setup the initial value of orderBy | |||
| if (!orderBy && prevOrderBy === INIT) { | |||
There was a problem hiding this comment.
This logic was moved to order_agg.tsx useEffect function (initialization step)
sulemanof
commented
May 24, 2019
| @@ -17,29 +17,23 @@ | |||
| * under the License. | |||
| */ | |||
|
|
|||
| import _ from 'lodash'; | |||
There was a problem hiding this comment.
Just get rid of lodash
maryia-lapata
suggested changes
May 27, 2019
29 tasks
sulemanof
added
Team:Visualizations
|
Pinging @elastic/kibana-app |
💔 Build Failed |
maryia-lapata
approved these changes
May 28, 2019
Co-Authored-By: Maryia Lapata <[email protected]>
💔 Build Failed |
cchaos
approved these changes
May 28, 2019
wylieconlon
approved these changes
May 28, 2019
There was a problem hiding this comment.
Changes themselves look good to me, tested locally. Please do not merge until #37064 is merged.
package.json
Outdated
| "enzyme": "^3.7.0", | ||
| "enzyme-adapter-react-16": "^1.9.0", | ||
| "enzyme": "^3.9.0", | ||
| "enzyme-adapter-react-16": "^1.13.1", |
There was a problem hiding this comment.
I saw you have another PR out for upgrading these, but the PRs have some slight differences. I'd like to get the other PR merged before this one, so the right people can review that dependency upgrade.
dmlemeshko
approved these changes
May 29, 2019
💚 Build Succeeded |
maryia-lapata
pushed a commit
to maryia-lapata/kibana
that referenced
this pull request
May 29, 2019
* EUIficate order_agg param editor * Update browser tests * Add types * Update enzyme version in x-pack * Fix functional tests * Changes due to comments * Update_terms_helper.tsx Co-Authored-By: Maryia Lapata <[email protected]> * Fix code review comments * Update yarn.lock
💔 Build Failed |
maryia-lapata
added a commit
that referenced
this pull request
May 29, 2019
* EUIficate order_agg param editor * Update browser tests * Add types * Update enzyme version in x-pack * Fix functional tests * Changes due to comments * Update_terms_helper.tsx Co-Authored-By: Maryia Lapata <[email protected]> * Fix code review comments * Update yarn.lock
jkakavas
pushed a commit
to jkakavas/kibana
that referenced
this pull request
May 30, 2019
* EUIficate order_agg param editor * Update browser tests * Add types * Update enzyme version in x-pack * Fix functional tests * Changes due to comments * Update_terms_helper.tsx Co-Authored-By: Maryia Lapata <[email protected]> * Fix code review comments * Update yarn.lock
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.
Summary
Waiting for merge #37064.
Part of #30922.
EUIfication of
order_aggcontrol for Terms aggregation parameter.Technically the PR only contains the EUIficated
Order byparam editor. Theorder_aggstill remains inangularjstemplate and will be done as a part ofvis-editor-agg-paramsEuification:Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers