[ILM] Delete phase redesign (rework)

[yuliacech]

Summary

This PR is a rework of closed #89451 and continuation of phases redesign in #88671.

Added design changes:

• Vertical timeline with checkbox icons for active phases
• Phase settings are hidden in an accordion
• A button group to enabled/disabled delete phase
• Delete phase style

Screenshots

Hot - warm - cold phases

Delete phase

Phases video

Feb-08-2021.15-46-19.mp4

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

[elasticmachine]

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

[cjcenizal]

This is looking great! @mdefazio About the phase bottom blocks adding empty space to the bottom of the panel -- I'm curious about the benefits here. What's the intention behind that? Same with the gray background of the delete panel. Do users need this additional treatment? Are we concerned that they'll be confused without it, or is there another problem we're trying to solve?

[snide]

Just passing through. Very minor sass changes I noticed. Looks great. Thanks for the screenshots / video!

[snide]

Suggested change

	width: 32px;
	height: 32px;
	width: $euiSizeXL;
	height: $euiSizeXL;

[snide]

Suggested change

	background-color: $euiColorEmptyShade;
	background-color: $euiColorEmptyShade;

[snide]

Suggested change

	background-color: $euiColorLightestShade;
	background-color: $euiColorLightestShade;

[jloleysens]

Great work @yuliacech ! Thanks for refactoring to use the EUI comment component, I think we are getting really close barring some final UI/UX decisions we should make regarding management of vertical space.

I'd like to provide some feedback in the meantime:

UI/UX

As we discussed, the min age input should not remain editable once a phase is disabled. I would say it can be hidden, but we should probably follow the designs for the specific look.

As discussed, I think we can move the searchable snapshot action in cold phase back to the top-level (i.e., outside of advanced settings).

Code

I left some suggestions in the code, only the one regarding the PhaseTimingConfiguration I'd really like to get your thoughts on!

[jloleysens]

nit; here and elsewhere. the convention for fixed string-literal props is:

className="ilmPhaseFooter" // without curly braces, with double quotes

I think this is perhaps webstorm specific behaviour as I recall running into it there. If you're using webstorm I managed to disable auto-bracket insertion for JSX by following this issue: https://youtrack.jetbrains.com/issue/WEB-31288?_ga=2.14889842.555971730.1612525382-1881428346.1584373905

[jloleysens]

I think this approach may be tying too much overlapping state into one value. For instance; if a phase is forever that implies that it is also last and enabled - which does not really sit well with the individual union of types.

I propose we extract the enabled | disabled state from the last | forever. I iterated on this a little bit and came up with the following:

diff of proposed changes

diff --git i/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phase_footer/phase_footer.tsx w/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phase_footer/phase_footer.tsx
index 880a354b4b0..0bc97e8f250 100644
--- i/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phase_footer/phase_footer.tsx
+++ w/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phase_footer/phase_footer.tsx
@@ -23,11 +23,13 @@ interface Props {
 }
 
 export const PhaseFooter: FunctionComponent<Props> = ({ phase }) => {
-  const phaseTimings = usePhaseTimings();
-  const phaseConfiguration = phaseTimings[phase];
-  const setValue = phaseTimings.setDeletePhaseEnabled;
+  const {
+    isDeletePhaseEnabled,
+    setDeletePhaseEnabled,
+    [phase]: phaseConfiguration,
+  } = usePhaseTimings();
 
-  if (phaseConfiguration === 'disabled' || phaseConfiguration === 'enabled') {
+  if (!phaseConfiguration.isFinalDataPhase) {
     return (
       <EuiPanel className={'ilmPhaseFooter'} hasShadow={false}>
         <EuiText size={'s'}>&nbsp;</EuiText>
@@ -35,7 +37,7 @@ export const PhaseFooter: FunctionComponent<Props> = ({ phase }) => {
     );
   }
 
-  if (phaseConfiguration === 'forever') {
+  if (!isDeletePhaseEnabled) {
     return (
       <EuiPanel color={'subdued'} className={'ilmPhaseFooter'} hasShadow={false}>
         <InfinityIcon size={'s'} />{' '}
@@ -44,7 +46,10 @@ export const PhaseFooter: FunctionComponent<Props> = ({ phase }) => {
             id="xpack.indexLifecycleMgmt.editPolicy.phaseTiming.foreverTimingDescription"
             defaultMessage="Data will remain in this phase forever."
           />{' '}
-          <EuiLink onClick={() => setValue(true)} data-test-subj={'enableDeletePhaseLink'}>
+          <EuiLink
+            onClick={() => setDeletePhaseEnabled(true)}
+            data-test-subj={'enableDeletePhaseLink'}
+          >
             <FormattedMessage
               id="xpack.indexLifecycleMgmt.editPolicy.deletePhase.enablePhaseButtonLabel"
               defaultMessage="Set delete age"
@@ -63,7 +68,10 @@ export const PhaseFooter: FunctionComponent<Props> = ({ phase }) => {
           id="xpack.indexLifecycleMgmt.editPolicy.phaseTiming.beforeDeleteDescription"
           defaultMessage="Data will be deleted after this phase."
         />{' '}
-        <EuiLink onClick={() => setValue(false)} data-test-subj={'disableDeletePhaseLink'}>
+        <EuiLink
+          onClick={() => setDeletePhaseEnabled(false)}
+          data-test-subj={'disableDeletePhaseLink'}
+        >
           <FormattedMessage
             id="xpack.indexLifecycleMgmt.editPolicy.deletePhase.disablePhaseButtonLabel"
             defaultMessage="Keep data"
diff --git i/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/phase_timings_context.tsx w/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/phase_timings_context.tsx
index 796cab1b404..fdfaec35e5b 100644
--- i/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/phase_timings_context.tsx
+++ w/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/phase_timings_context.tsx
@@ -5,12 +5,19 @@
  * 2.0.
  */
 
-import React, { createContext, FunctionComponent, useContext } from 'react';
+import React, { createContext, FunctionComponent, useContext, useCallback } from 'react';
 import { useFormData } from '../../../../shared_imports';
 import { FormInternal } from '../types';
 import { UseField } from './index';
 
-export type PhaseTimingConfiguration = 'forever' | 'last' | 'disabled' | 'enabled';
+// export type PhaseTimingConfiguration = 'forever' | 'last' | 'disabled' | 'enabled';
+
+export interface PhaseTimingConfiguration {
+  /**
+   * Whether this is the final, non-delete, phase.
+   */
+  isFinalDataPhase: boolean;
+}
 
 const getPhaseTimingConfiguration = (
   formData: FormInternal
@@ -21,23 +28,23 @@ const getPhaseTimingConfiguration = (
 } => {
   const isWarmPhaseEnabled = formData?._meta?.warm?.enabled;
   const isColdPhaseEnabled = formData?._meta?.cold?.enabled;
-  if (formData?._meta?.delete?.enabled) {
-    return {
-      hot: !isWarmPhaseEnabled && !isColdPhaseEnabled ? 'last' : 'enabled',
-      warm: isWarmPhaseEnabled ? (isColdPhaseEnabled ? 'enabled' : 'last') : 'disabled',
-      cold: isColdPhaseEnabled ? 'last' : 'disabled',
-    };
-  }
   return {
-    hot: !isWarmPhaseEnabled && !isColdPhaseEnabled ? 'forever' : 'enabled',
-    warm: isWarmPhaseEnabled ? (isColdPhaseEnabled ? 'enabled' : 'forever') : 'disabled',
-    cold: isColdPhaseEnabled ? 'forever' : 'disabled',
+    hot: {
+      isFinalDataPhase: !isWarmPhaseEnabled && !isColdPhaseEnabled,
+    },
+    warm: {
+      isFinalDataPhase: isWarmPhaseEnabled && !isColdPhaseEnabled,
+    },
+    cold: {
+      isFinalDataPhase: isColdPhaseEnabled,
+    },
   };
 };
 export interface PhaseTimings {
   hot: PhaseTimingConfiguration;
   warm: PhaseTimingConfiguration;
   cold: PhaseTimingConfiguration;
+  isDeletePhaseEnabled: boolean;
   setDeletePhaseEnabled: (enabled: boolean) => void;
 }
 
@@ -49,12 +56,13 @@ export const PhaseTimingsProvider: FunctionComponent = ({ children }) => {
   });
 
   return (
-    <UseField path="_meta.delete.enabled">
+    <UseField<boolean> path="_meta.delete.enabled">
       {(field) => {
         return (
           <PhaseTimingsContext.Provider
             value={{
               ...getPhaseTimingConfiguration(formData),
+              isDeletePhaseEnabled: !!field.value,
               setDeletePhaseEnabled: field.setValue,
             }}
           >

Let me know what you think!

[jloleysens]

Really creative use of UseField inside a context component! I had not thought of doing this before.

[yuliacech]

@elasticmachine merge upstream

[jloleysens]

Tested again locally, these changes are in really good shape!

We will address surfacing the searchable snapshot field in a follow up PR 👍

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 9851d48

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
indexLifecycleManagement	177	204	+27

Async chunks

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

id	before	after	diff
indexLifecycleManagement	228.3KB	244.4KB	+16.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
indexLifecycleManagement	63.9KB	63.9KB	+1.0B

History

• 💚 Build #104679 succeeded ad70c1b
• 💛 Build #104604 was flaky 73ed835
• 💛 Build #103924 was flaky 0b4035c
• 💔 Build #103903 failed 47edd7b
• 💔 Build #103856 failed 9342150

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

[mdefazio]

This looks awesome! Really nice work!