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

[Vis: Default editor] EUIficate agg-select #31892

Merged
merged 59 commits into from
Mar 28, 2019

Conversation

maryia-lapata
Copy link
Contributor

@maryia-lapata maryia-lapata commented Feb 25, 2019

EUIfication of agg-select witch is a part of Data tab on Default Editor.

Part of #30922

feb-27-2019 16-41-49

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@maryia-lapata maryia-lapata added the WIP Work in progress label Feb 25, 2019
@maryia-lapata maryia-lapata requested a review from a team as a code owner February 26, 2019 09:48
@timroes timroes mentioned this pull request Feb 26, 2019
29 tasks
@cchaos
Copy link
Contributor

cchaos commented Feb 27, 2019

FYI, I've got a PR up in EUI that will create this pattern of appending nodes to EuiFormRow labels. It's known that you shouldn't put clickable elements inside of <label>s which is how the current implementation renders.

@timroes
Copy link
Contributor

timroes commented Feb 27, 2019

The EuiComboBox currently behaves buggy (and breaking accessibility) in that singleSelection={{ asPlainText: true}} mode, that we're needing all over the editor. I'll mark this PR as blocked on elastic/eui#1615 for now, but we should still try to get this PR as ready as possible once the EUI fixed are ready.

@elastic elastic deleted a comment from elasticmachine Feb 27, 2019
@elastic elastic deleted a comment from elasticmachine Feb 27, 2019
@elastic elastic deleted a comment from elasticmachine Feb 27, 2019
@elastic elastic deleted a comment from elasticmachine Feb 27, 2019
@elastic elastic deleted a comment from elasticmachine Feb 27, 2019
@elastic elastic deleted a comment from elasticmachine Feb 27, 2019
@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

I checked changes related to FTR and tested it locally. LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Tested on Chrome Linux, everything seems to work fine. Left a minor suggestion, but besides that LGTM

src/legacy/ui/public/vis/editors/default/agg_select.js Outdated Show resolved Hide resolved
@timroes timroes added the release_note:skip Skip the PR/issue when compiling release notes label Mar 28, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@maryia-lapata maryia-lapata merged commit b03f6c3 into elastic:master Mar 28, 2019
@maryia-lapata maryia-lapata deleted the vis-editor-agg-select branch March 28, 2019 10:02
maryia-lapata added a commit to maryia-lapata/kibana that referenced this pull request Mar 28, 2019
* EUIficate agg-select

* Improve validation; update TS

* Apply styles for helpLink

* Fix unit test

* Update functional tests

* Adjust comboBox service to chose the item where the text mates exactly

* Update vis page object

* Add default value for agg

* Move aggs grouping function to a separate file

* Use labelAppend prop for help link node

* Add watcher for aggType to manage to discard changes

* Add default value for agg type title

* Fix defining selected option when aggType is defined

* Fix validation issues

* Remove a bootstrap specific class

* Change css selector in test

* Update according to SASS guidelines

* Update functinal comboBox service

* Added check for undefined

* Add jsdoc for groupAggregationsBy function

* Add unit tests for groupAggregationsBy

* Move setValidity invocation to DefaultEditorAggSelect component

* Wrap setValidity into useEffect due to react warning when select is cleaned at the first time

* Move help link definition to select component
maryia-lapata added a commit that referenced this pull request Mar 28, 2019
* EUIficate agg-select

* Improve validation; update TS

* Apply styles for helpLink

* Fix unit test

* Update functional tests

* Adjust comboBox service to chose the item where the text mates exactly

* Update vis page object

* Add default value for agg

* Move aggs grouping function to a separate file

* Use labelAppend prop for help link node

* Add watcher for aggType to manage to discard changes

* Add default value for agg type title

* Fix defining selected option when aggType is defined

* Fix validation issues

* Remove a bootstrap specific class

* Change css selector in test

* Update according to SASS guidelines

* Update functinal comboBox service

* Added check for undefined

* Add jsdoc for groupAggregationsBy function

* Add unit tests for groupAggregationsBy

* Move setValidity invocation to DefaultEditorAggSelect component

* Wrap setValidity into useEffect due to react warning when select is cleaned at the first time

* Move help link definition to select component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Feature:Vis Editor Visualization editor issues release_note:skip Skip the PR/issue when compiling release notes v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants