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

[ILM] Rollover min age tooltip and copy fixes #91110

Merged
merged 12 commits into from
Feb 15, 2021

Conversation

jloleysens
Copy link
Contributor

Summary

Adds a new tooltip to fulfil the function of the old link on min age fields.

Prior:

Link

Screenshot 2021-02-11 at 12 17 15

Today:

Tooltip

Screenshot 2021-02-11 at 11 37 33

Link

Screenshot 2021-02-11 at 12 12 34

Checklist

Delete any items that are not applicable to this PR.

@jloleysens jloleysens added Feature:ILM v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Feb 11, 2021
@jloleysens jloleysens requested review from yuliacech and a team February 11, 2021 11:20
@jloleysens jloleysens requested a review from a team as a code owner February 11, 2021 11:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, @jloleysens!
I left one comment about rollover info tooltip, otherwise code LGTM! 👍

@jloleysens
Copy link
Contributor Author

@yuliacech I think I have addressed your comment :). Do you think you could take another look?

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comment, @jloleysens! Changes LGTM :)

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

Thanks for raising this @jloleysens. I don't have an issue with the tooltip itself, but I do have some concerns regarding how the new ILM UI uses "data." I also wonder if the new UI presents phase timing clearly.

I wrote those concerns up here: #91100 (review)

Let me know the best place to address those.

cc @debadair

@jrodewig jrodewig dismissed their stale review February 12, 2021 18:36

Stale review

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

LGTM. My concerns re: the "data" vs. "index" usage were addressed in #91100.

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
indexLifecycleManagement 240.0KB 240.6KB +627.0B

History

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

@jloleysens jloleysens merged commit fb25526 into elastic:master Feb 15, 2021
@jloleysens jloleysens deleted the ilm/rollover-min-age-tooltip branch February 15, 2021 11:04
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 15, 2021
* removed an unnecessary tooltip in rollover field, added a tooltip to min age field when rollover is enabled

* slight update to copy, added jest test and added comment about testing

* page title and timeline title to sentence case

* added link to learn more about timing to phase timeline component

* fix jest test copy

* remove unused import

* fix i18n

* remove unused translations

* slight refactor to conditional for clarity

* slight refactor to i18n text naming

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit that referenced this pull request Feb 15, 2021
* removed an unnecessary tooltip in rollover field, added a tooltip to min age field when rollover is enabled

* slight update to copy, added jest test and added comment about testing

* page title and timeline title to sentence case

* added link to learn more about timing to phase timeline component

* fix jest test copy

* remove unused import

* fix i18n

* remove unused translations

* slight refactor to conditional for clarity

* slight refactor to i18n text naming

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 15, 2021
…ndition-for-hiding-recommded-allocation

* 'master' of github.com:elastic/kibana:
  [Discover] Fix toggling multi fields from doc view table (elastic#91121)
  [ML] Data Frame Analytics: ROC Curve Chart (elastic#89991)
  skip flaky suite (elastic#86948)
  skip flaky suite (elastic#91191)
  Fix date histogram time zone for rollup index (elastic#90632)
  [Search Source] Fix retrieval of unmapped fields; Add field filters (elastic#89837)
  [Logs UI] Use useMlHref hook for ML links (elastic#90935)
  Fix values of `products.min_price` field in Kibana sample ecommerce data set (elastic#90428)
  [APM] Darker shade for Error group details labels (elastic#91349)
  [Lens] Adjust new copy for 7.12 (elastic#90413)
  [ML] Unskip test. Fix modelMemoryLimit value. (elastic#91280)
  [Lens] Fix empty display name issue in XY chart (elastic#91132)
  [Lens] Improves error messages when in Dashboard (elastic#90668)
  [Lens] Keyboard-selected items follow user traversal of drop zones (elastic#90546)
  [Lens] Improves ranking feature in Top values (elastic#90749)
  [ILM] Rollover min age tooltip and copy fixes (elastic#91110)

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants