Simplify conditional checks and move instance selector above

elastic · Dec 2, 2024 · 6d04e55 · 6d04e55
1 parent 5165c30
commit 6d04e55
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 42 deletions.
diff --git a/...ervability_solution/slo/public/components/slo/auto_refresh_button/auto_refresh_button.tsx b/...ervability_solution/slo/public/components/slo/auto_refresh_button/auto_refresh_button.tsx
@@ -32,7 +32,7 @@ export function AutoRefreshButton({ disabled, isAutoRefreshing, onClick }: Props
       data-test-subj="autoRefreshButton"
       disabled={disabled}
       iconSide="left"
-      iconType="play"
+      iconType="refresh"
       onClick={onClick}
     >
       {i18n.translate('xpack.slo.slosPage.autoRefreshButtonLabel', {

diff --git a/...plugins/observability_solution/slo/public/pages/slo_details/components/header_control.tsx b/...plugins/observability_solution/slo/public/pages/slo_details/components/header_control.tsx
@@ -22,21 +22,20 @@ import { SloDeleteModal } from '../../../components/slo/delete_confirmation_moda
 import { SloResetConfirmationModal } from '../../../components/slo/reset_confirmation_modal/slo_reset_confirmation_modal';
 import { useCloneSlo } from '../../../hooks/use_clone_slo';
 import { useFetchRulesForSlo } from '../../../hooks/use_fetch_rules_for_slo';
+import { useKibana } from '../../../hooks/use_kibana';
 import { usePermissions } from '../../../hooks/use_permissions';
 import { useResetSlo } from '../../../hooks/use_reset_slo';
-import { useKibana } from '../../../hooks/use_kibana';
 import { convertSliApmParamsToApmAppDeeplinkUrl } from '../../../utils/slo/convert_sli_apm_params_to_apm_app_deeplink_url';
 import { isApmIndicatorType } from '../../../utils/slo/indicator';
 import { EditBurnRateRuleFlyout } from '../../slos/components/common/edit_burn_rate_rule_flyout';
 import { useGetQueryParams } from '../hooks/use_get_query_params';
 import { useSloActions } from '../hooks/use_slo_actions';
 
 export interface Props {
-  slo?: SLOWithSummaryResponse;
-  isLoading: boolean;
+  slo: SLOWithSummaryResponse;
 }
 
-export function HeaderControl({ isLoading, slo }: Props) {
+export function HeaderControl({ slo }: Props) {
   const {
     application: { navigateToUrl, capabilities },
     http: { basePath },
@@ -58,10 +57,10 @@ export function HeaderControl({ isLoading, slo }: Props) {
   const { mutateAsync: resetSlo, isLoading: isResetLoading } = useResetSlo();
 
   const { data: rulesBySlo, refetchRules } = useFetchRulesForSlo({
-    sloIds: slo ? [slo.id] : undefined,
+    sloIds: [slo.id],
   });
 
-  const rules = slo ? rulesBySlo?.[slo?.id] ?? [] : [];
+  const rules = rulesBySlo?.[slo.id] ?? [];
 
   const handleActionsClick = () => setIsPopoverOpen((value) => !value);
   const closePopover = () => setIsPopoverOpen(false);
@@ -92,10 +91,6 @@ export function HeaderControl({ isLoading, slo }: Props) {
   });
 
   const handleNavigateToApm = () => {
-    if (!slo) {
-      return undefined;
-    }
-
     const url = convertSliApmParamsToApmAppDeeplinkUrl(slo);
     if (url) {
       navigateToUrl(basePath.prepend(url));
@@ -105,10 +100,8 @@ export function HeaderControl({ isLoading, slo }: Props) {
   const navigateToClone = useCloneSlo();
 
   const handleClone = async () => {
-    if (slo) {
-      setIsPopoverOpen(false);
-      navigateToClone(slo);
-    }
+    setIsPopoverOpen(false);
+    navigateToClone(slo);
   };
 
   const handleDelete = () => {
@@ -140,11 +133,9 @@ export function HeaderControl({ isLoading, slo }: Props) {
   };
 
   const handleResetConfirm = async () => {
-    if (slo) {
-      await resetSlo({ id: slo.id, name: slo.name });
-      removeResetQueryParam();
-      setResetConfirmationModalOpen(false);
-    }
+    await resetSlo({ id: slo.id, name: slo.name });
+    removeResetQueryParam();
+    setResetConfirmationModalOpen(false);
   };
 
   const handleResetCancel = () => {
@@ -182,8 +173,6 @@ export function HeaderControl({ isLoading, slo }: Props) {
             iconType="arrowDown"
             iconSize="s"
             onClick={handleActionsClick}
-            isLoading={isLoading}
-            disabled={isLoading}
           >
             {i18n.translate('xpack.slo.sloDetails.headerControl.actions', {
               defaultMessage: 'Actions',
@@ -315,7 +304,7 @@ export function HeaderControl({ isLoading, slo }: Props) {
         refetchRules={refetchRules}
       />
 
-      {slo && isRuleFlyoutVisible ? (
+      {isRuleFlyoutVisible ? (
         <AddRuleFlyout
           consumer={sloFeatureId}
           ruleTypeId={SLO_BURN_RATE_RULE_TYPE_ID}
@@ -326,11 +315,11 @@ export function HeaderControl({ isLoading, slo }: Props) {
         />
       ) : null}
 
-      {slo && isDeleteConfirmationModalOpen ? (
+      {isDeleteConfirmationModalOpen ? (
         <SloDeleteModal slo={slo} onCancel={handleDeleteCancel} onSuccess={handleDeleteConfirm} />
       ) : null}
 
-      {slo && isResetConfirmationModalOpen ? (
+      {isResetConfirmationModalOpen ? (
         <SloResetConfirmationModal
           slo={slo}
           onCancel={handleResetCancel}

diff --git a/...k/plugins/observability_solution/slo/public/pages/slo_details/components/header_title.tsx b/...k/plugins/observability_solution/slo/public/pages/slo_details/components/header_title.tsx
@@ -21,11 +21,12 @@ export interface Props {
 
 export function HeaderTitle({ isLoading, slo }: Props) {
   if (isLoading || !slo) {
-    return <EuiSkeletonText lines={1} data-test-subj="loadingTitle" />;
+    return <EuiSkeletonText lines={2} data-test-subj="loadingTitle" />;
   }
 
   return (
-    <EuiFlexGroup direction="column" gutterSize="s">
+    <EuiFlexGroup direction="column" gutterSize="m">
+      <SLOGroupings slo={slo} />
       <EuiFlexGroup
         direction="row"
         gutterSize="s"
@@ -59,7 +60,6 @@ export function HeaderTitle({ isLoading, slo }: Props) {
           </EuiText>
         </EuiFlexItem>
       </EuiFlexGroup>
-      <SLOGroupings slo={slo} />
     </EuiFlexGroup>
   );
 }
diff --git a/...lugins/observability_solution/slo/public/pages/slo_details/hooks/use_slo_details_tabs.tsx b/...lugins/observability_solution/slo/public/pages/slo_details/hooks/use_slo_details_tabs.tsx
@@ -5,13 +5,13 @@
  * 2.0.
  */
 
-import { i18n } from '@kbn/i18n';
 import { EuiNotificationBadge, EuiToolTip } from '@elastic/eui';
-import React from 'react';
+import { i18n } from '@kbn/i18n';
 import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
+import React from 'react';
 import { paths } from '../../../../common/locators/paths';
-import { useKibana } from '../../../hooks/use_kibana';
 import { useFetchActiveAlerts } from '../../../hooks/use_fetch_active_alerts';
+import { useKibana } from '../../../hooks/use_kibana';
 import {
   ALERTS_TAB_ID,
   HISTORY_TAB_ID,

diff --git a/x-pack/plugins/observability_solution/slo/public/pages/slo_details/slo_details.tsx b/x-pack/plugins/observability_solution/slo/public/pages/slo_details/slo_details.tsx
@@ -21,10 +21,10 @@ import { HeaderMenu } from '../../components/header_menu/header_menu';
 import { AutoRefreshButton } from '../../components/slo/auto_refresh_button';
 import { useAutoRefreshStorage } from '../../components/slo/auto_refresh_button/hooks/use_auto_refresh_storage';
 import { useFetchSloDetails } from '../../hooks/use_fetch_slo_details';
+import { useKibana } from '../../hooks/use_kibana';
 import { useLicense } from '../../hooks/use_license';
 import { usePermissions } from '../../hooks/use_permissions';
 import { usePluginContext } from '../../hooks/use_plugin_context';
-import { useKibana } from '../../hooks/use_kibana';
 import PageNotFound from '../404';
 import { HeaderControl } from './components/header_control';
 import { HeaderTitle } from './components/header_title';
@@ -125,21 +125,23 @@ export function SloDetailsPage() {
       pageHeader={{
         pageTitle: slo?.name ?? <EuiSkeletonText lines={1} />,
         children: <HeaderTitle isLoading={isPerformingAction} slo={slo} />,
-        rightSideItems: [
-          <HeaderControl isLoading={isPerformingAction} slo={slo} />,
-          <AutoRefreshButton
-            disabled={isPerformingAction}
-            isAutoRefreshing={isAutoRefreshing}
-            onClick={handleToggleAutoRefresh}
-          />,
-        ],
+        rightSideItems: !isLoading
+          ? [
+              <HeaderControl slo={slo!} />,
+              <AutoRefreshButton
+                isAutoRefreshing={isAutoRefreshing}
+                onClick={handleToggleAutoRefresh}
+              />,
+            ]
+          : undefined,
         tabs,
       }}
       data-test-subj="sloDetailsPage"
     >
       <HeaderMenu />
-      {isLoading && <EuiLoadingSpinner data-test-subj="sloDetailsLoading" />}
-      {!isLoading && (
+      {isLoading ? (
+        <EuiLoadingSpinner data-test-subj="sloDetailsLoading" />
+      ) : (
         <SloDetails slo={slo!} isAutoRefreshing={isAutoRefreshing} selectedTabId={selectedTabId} />
       )}
     </ObservabilityPageTemplate>