[ILM] Revisit searchable snapshot field after new redesign by jloleysens · Pull Request #90793 · elastic/kibana

jloleysens · 2021-02-09T15:32:30Z

Summary

After merging #88671 we restructured the ILM policy form aggressively so that timing inputs would be top-level and all other fields hidden. This un-did the effort to keep searchable snapshot visible at a top level. This PR resurfaces searchable snapshots (SS) at a top level in the cold phase.

The pattern followed is the same as the "Wait for snapshot policy" on the delete phase.

Additionally, includes a fix for form error state not clearing when a field is unmounted. Also added a test for this.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

…rop down to sit flush with side of panel

- the error state of the form would not clear correctly if the erroring field was unmounted. The logic for clearing form errors was also incorrectly using "keys" instead of "values". - updated the width of wait for snapshot policy field to be the same as other fields

elasticmachine · 2021-02-09T15:32:32Z

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

jloleysens · 2021-02-09T15:34:45Z

@yuliacech I updated your styling in 3391770 to make the "Advanced settings" section sit a bit more flush to with the left of the comment panel. Let me know what you think! I assume it was added to offset the height diff of the button group?

yuliacech

Thanks for adding this, @jloleysens ! Looks good to me, just left a comment about snapshot policy in delete phase.

yuliacech · 2021-02-09T18:56:08Z

...ecycle_management/public/application/sections/edit_policy/components/phases/phase/phase.scss

+    padding-top: $euiSizeS;
+    padding-bottom: $euiSizeS;


Thanks for adding this, @jloleysens !

yuliacech · 2021-02-09T19:18:18Z

...application/sections/edit_policy/components/phases/shared_fields/snapshot_policies_field.tsx

                  'data-test-subj': 'snapshotPolicyCombobox',
+                  fullWidth: false,
                  options: policies,
-                  singleSelection: { asPlainText: true },


I think singleSelection prop is still needed as policy name otherwise looks a bit different (not like a text and suggesting that multiple values are possible)

right, this was definitely an accident 😅

jloleysens · 2021-02-10T08:57:47Z

@elasticmachine merge upstream

yuliacech

Thanks for adding singleSelection back to the wait for snapshot field, @jloleysens !
Other changes LGTM 👍

kibanamachine · 2021-02-10T11:37:52Z

💚 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`	239.3KB	240.0KB	+718.0B

History

💚 Build #105494 succeeded 5db0998
💔 Build #105208 failed 48e1bab
💚 Build #105202 succeeded b0eeb9b

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

mdefazio

LGTM!

…0793) * moved searchable snapshot field out of cold phase accordian * refactor styling to padding top and bottom to get advanced settings drop down to sit flush with side of panel * Error clearing fix and cosmetic changes - the error state of the form would not clear correctly if the erroring field was unmounted. The logic for clearing form errors was also incorrectly using "keys" instead of "values". - updated the width of wait for snapshot policy field to be the same as other fields * fix hook dependency causing clearError to be called * slight improvement to component integration test * re-add singleSelection to snapshot policiy field config * refactored Phase component API and fixed typo in comment Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: (99 commits) [Fleet] Use Fleet Server indices in the search bar (elastic#90835) [Search Sessions] added an info flyout to session management (elastic#90559) [ILM] Revisit searchable snapshot field after new redesign (elastic#90793) [Alerting] License Errors on Alert List View (elastic#89920) RFC Improve saved object migrations algorithm (elastic#84333) [Lens] (Accessibility) Fix focus on drag and drop actions (elastic#90561) Use new shortcut links to Fleet discuss forums. (elastic#90786) Do not generate an ephemeral encryption key in production. (elastic#81511) [Fleet] Use staging registry for snapshot builds (elastic#90327) Actually deleting x-pack/tsconfig.refs.json (elastic#90898) Add deprecation warning to all Beats CM pages. (elastic#90741) skip flaky suite (elastic#90136) Revert "Revert "[Metrics UI] Add Metrics Anomaly Alert Type (elastic#89244)"" (elastic#90889) remove ref to removed tsconfig file [core.logging] Uses host timezone as default (elastic#90368) [Maps] remove maps_file_upload plugin and fold public folder into file_upload plugin (elastic#90292) Revert "[Metrics UI] Add Metrics Anomaly Alert Type (elastic#89244)" [dev-utils/ci-stats] support disabling ship errors (elastic#90851) Prefix with / (elastic#90836) [Metrics UI] Add Metrics Anomaly Alert Type (elastic#89244) ...

kibanamachine · 2021-02-12T13:27:08Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…90937) * moved searchable snapshot field out of cold phase accordian * refactor styling to padding top and bottom to get advanced settings drop down to sit flush with side of panel * Error clearing fix and cosmetic changes - the error state of the form would not clear correctly if the erroring field was unmounted. The logic for clearing form errors was also incorrectly using "keys" instead of "values". - updated the width of wait for snapshot policy field to be the same as other fields * fix hook dependency causing clearError to be called * slight improvement to component integration test * re-add singleSelection to snapshot policiy field config * refactored Phase component API and fixed typo in comment Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

jloleysens added 4 commits February 9, 2021 15:15

moved searchable snapshot field out of cold phase accordian

4f4f521

refactor styling to padding top and bottom to get advanced settings d…

3391770

…rop down to sit flush with side of panel

fix hook dependency causing clearError to be called

b0eeb9b

jloleysens added Feature:ILM v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Feb 9, 2021

jloleysens requested review from mdefazio and yuliacech February 9, 2021 15:32

jloleysens requested review from a team as code owners February 9, 2021 15:32

slight improvement to component integration test

48e1bab

yuliacech reviewed Feb 9, 2021

View reviewed changes

re-add singleSelection to snapshot policiy field config

8f1c36e

Merge branch 'master' into ilm/revisit-searchable-snapshot-field

5db0998

yuliacech self-requested a review February 10, 2021 09:07

yuliacech approved these changes Feb 10, 2021

View reviewed changes

refactored Phase component API and fixed typo in comment

82bf024

mdefazio approved these changes Feb 10, 2021

View reviewed changes

jloleysens merged commit e17878e into elastic:master Feb 10, 2021

jloleysens deleted the ilm/revisit-searchable-snapshot-field branch February 10, 2021 13:25

jloleysens mentioned this pull request Feb 10, 2021

[7.x] [ILM] Revisit searchable snapshot field after new redesign (#90793) #90937

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 12, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ILM] Revisit searchable snapshot field after new redesign#90793

[ILM] Revisit searchable snapshot field after new redesign#90793
jloleysens merged 8 commits intoelastic:masterfrom
jloleysens:ilm/revisit-searchable-snapshot-field

jloleysens commented Feb 9, 2021

Uh oh!

elasticmachine commented Feb 9, 2021

Uh oh!

jloleysens commented Feb 9, 2021

Uh oh!

yuliacech left a comment

Uh oh!

yuliacech Feb 9, 2021

Uh oh!

yuliacech Feb 9, 2021

Uh oh!

jloleysens Feb 10, 2021

Uh oh!

jloleysens commented Feb 10, 2021

Uh oh!

yuliacech left a comment

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

mdefazio left a comment

Uh oh!

kibanamachine commented Feb 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jloleysens commented Feb 9, 2021

Summary

Checklist

Uh oh!

elasticmachine commented Feb 9, 2021

Uh oh!

jloleysens commented Feb 9, 2021

Uh oh!

yuliacech left a comment

Choose a reason for hiding this comment

Uh oh!

yuliacech Feb 9, 2021

Choose a reason for hiding this comment

Uh oh!

yuliacech Feb 9, 2021

Choose a reason for hiding this comment

Uh oh!

jloleysens Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

jloleysens commented Feb 10, 2021

Uh oh!

yuliacech left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 10, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

mdefazio left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants