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

[TIP] Replace indicator field-selector EuiSelect by EuiComboBox #140980

Merged
merged 3 commits into from
Sep 20, 2022

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Sep 19, 2022

Summary

The field selector next to the barchart on the Indicators page needed an update. The dropdown size was taking the whole screen and it wasn't searchable. To improve user experience, we're now using an EuiComboBox instead of an EuiSelect.

Screen.Recording.2022-09-19.at.10.34.19.AM.mov

https://github.com/elastic/security-team/issues/4628
and
https://github.com/elastic/security-team/issues/4749

Notes on the ACs: the height of the dropdown is now driven by the EuiCombobox component

Checklist

Delete any items that are not applicable to this PR.

@PhilippeOberti PhilippeOberti marked this pull request as ready for review September 19, 2022 17:31
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner September 19, 2022 17:31
@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes 8.5 candidate Team: Protections Experience labels Sep 19, 2022
prepend={i18n.translate('xpack.threatIntelligence.indicator.fieldSelector.label', {
defaultMessage: 'Stack by',
})}
value={selectedField}
singleSelection={{ asPlainText: true }}
onChange={(event) => selectedFieldChange(event)}
Copy link
Contributor

Choose a reason for hiding this comment

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

onChange={selectedFieldChange} should be enough

prepend={i18n.translate('xpack.threatIntelligence.indicator.fieldSelector.label', {
defaultMessage: 'Stack by',
})}
value={selectedField}
singleSelection={{ asPlainText: true }}
Copy link
Contributor

Choose a reason for hiding this comment

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

this object literal can be lifted outside the component scope, for perf.

Copy link
Contributor

@lgestc lgestc left a comment

Choose a reason for hiding this comment

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

LGTM! I've left some minor nitpicks though 👯

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #16 / apis Osquery Endpoints Packs create route should return 200 and multi line query, but single line query in packs config

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
threatIntelligence 199 200 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
threatIntelligence 73.1KB 73.2KB +119.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@PhilippeOberti PhilippeOberti merged commit e826b9c into elastic:main Sep 20, 2022
@PhilippeOberti PhilippeOberti deleted the field-selector branch September 20, 2022 18:43
@kibanamachine kibanamachine added v8.5.0 backport:skip This commit does not require backporting labels Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.5 candidate backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: Protections Experience v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants