address review comments

Signed-off-by: Hailong Cui <ihailong@amazon.com>
ruanyl · Apr 17, 2024 · 4ae41ee · 4ae41ee
1 parent 00b1307
commit 4ae41ee
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 75 deletions.
diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx
@@ -320,16 +320,22 @@ export class ChromeService {
       setBreadcrumbs: (newBreadcrumbs: ChromeBreadcrumb[]) => {
         if (workspaces.currentWorkspace$.getValue()) {
           const currentWorkspace = workspaces.currentWorkspace$.getValue()!;
-          const workspaceCrumb: ChromeBreadcrumb = {
-            text: currentWorkspace.name,
-            onClick: () => {
-              application.navigateToApp('workspace_overview');
-            },
-          };
-          breadcrumbs$.next([workspaceCrumb, ...newBreadcrumbs]);
-        } else {
-          breadcrumbs$.next(newBreadcrumbs);
+          const workspaceInBreadcrumbs =
+            newBreadcrumbs.findIndex((breadcrumb: ChromeBreadcrumb) => {
+              return breadcrumb.text === currentWorkspace.name;
+            }) !== -1;
+          if (!workspaceInBreadcrumbs) {
+            const workspaceCrumb: ChromeBreadcrumb = {
+              text: currentWorkspace.name,
+              onClick: () => {
+                application.navigateToApp('workspace_overview');
+              },
+            };
+            breadcrumbs$.next([workspaceCrumb, ...newBreadcrumbs]);
+            return;
+          }
         }
+        breadcrumbs$.next(newBreadcrumbs);
       },
 
       getHelpExtension$: () => helpExtension$.pipe(takeUntil(this.stop$)),

diff --git a/src/core/utils/default_app_categories.ts b/src/core/utils/default_app_categories.ts
@@ -77,35 +77,35 @@ export const DEFAULT_APP_CATEGORIES: Record<string, AppCategory> = Object.freeze
   getStarted: {
     id: 'getStarted',
     label: i18n.translate('core.ui.managementNavList.label', {
-      defaultMessage: 'get started',
+      defaultMessage: 'Get started',
     }),
     order: 10000,
   },
   dashboardAndReport: {
     id: 'dashboardReport',
     label: i18n.translate('core.ui.managementNavList.label', {
-      defaultMessage: 'dashboard and report',
+      defaultMessage: 'Dashboard and report',
     }),
     order: 11000,
   },
   investigate: {
     id: 'investigate',
     label: i18n.translate('core.ui.managementNavList.label', {
-      defaultMessage: 'investigate',
+      defaultMessage: 'Investigate',
     }),
     order: 12000,
   },
   detect: {
     id: 'detect',
     label: i18n.translate('core.ui.managementNavList.label', {
-      defaultMessage: 'detect',
+      defaultMessage: 'Detect',
     }),
     order: 13000,
   },
   searchSolution: {
     id: 'searchSolution',
     label: i18n.translate('core.ui.managementNavList.label', {
-      defaultMessage: 'build search solution',
+      defaultMessage: 'Build search solution',
     }),
     order: 14000,
   },

diff --git a/src/plugins/workspace/public/components/workspace_overview/getting_start_card.tsx b/src/plugins/workspace/public/components/workspace_overview/getting_start_card.tsx
@@ -25,15 +25,20 @@ export const WorkspaceOverviewCard = ({
   return (
     <EuiCard
       layout="vertical"
-      titleSize="xs"
       textAlign="left"
-      title={<EuiText>{card.title}</EuiText>}
+      title={
+        <EuiText size="s">
+          <p>{card.title}</p>
+        </EuiText>
+      }
       description={''}
       footer={
         <>
           <EuiFlexGroup justifyContent="flexStart">
             <EuiFlexItem grow={false}>
-              <EuiTextColor color="subdued">{card.description}</EuiTextColor>
+              <EuiTextColor color="subdued">
+                <EuiText size="s">{card.description}</EuiText>
+              </EuiTextColor>
             </EuiFlexItem>
           </EuiFlexGroup>
         </>

diff --git a/src/plugins/workspace/public/components/workspace_overview/getting_start_modal.tsx b/src/plugins/workspace/public/components/workspace_overview/getting_start_modal.tsx
@@ -30,7 +30,7 @@ interface Props extends Omit<WorkspaceOverviewCardProps, 'card'> {
 }
 
 export const WorkspaceOverviewGettingStartModal = (props: Props) => {
-  const ALL = 'all';
+  const ALL = 'All';
   const [selectedItemName, setSelectedItem] = useState(ALL);
   const scrollToRefs = useRef({} as Record<string, HTMLDivElement | null>);
   const allRef = useRef(null as HTMLDivElement | null);
@@ -66,48 +66,55 @@ export const WorkspaceOverviewGettingStartModal = (props: Props) => {
 
   const sideNav = [ALL, ...categories].map((category) => createItem(category));
 
-  const cardList: ReactNode[] = categories.map((category) => {
-    const cards = availableCards.filter((card) => {
-      return card.category.label === category;
-    });
+  const cardList: ReactNode[] = categories
+    // .filter((category) => category === selectedItemName || selectedItemName === ALL)
+    .map((category) => {
+      const cards = availableCards.filter((card) => {
+        return card.category.label === category;
+      });
 
-    return (
-      <div id={slugify(category)} ref={(ref) => (scrollToRefs.current[slugify(category)] = ref)}>
-        <EuiTitle>
-          <h2>{category}</h2>
-        </EuiTitle>
-        <EuiSpacer size="m" />
-        <EuiFlexGrid columns={4}>
-          {cards.map((card) => {
-            return (
-              <EuiFlexItem key={card.title}>
-                <WorkspaceOverviewCard {...props} card={card} />
-              </EuiFlexItem>
-            );
-          })}
-        </EuiFlexGrid>
-        <EuiSpacer size="m" />
-      </div>
-    );
-  });
+      return (
+        <div id={slugify(category)} ref={(ref) => (scrollToRefs.current[slugify(category)] = ref)}>
+          <EuiTitle>
+            <h2>{category}</h2>
+          </EuiTitle>
+          <EuiSpacer size="m" />
+          <EuiFlexGrid columns={4}>
+            {cards.map((card) => {
+              return (
+                <EuiFlexItem key={card.title}>
+                  <WorkspaceOverviewCard {...props} card={card} />
+                </EuiFlexItem>
+              );
+            })}
+          </EuiFlexGrid>
+          <EuiSpacer size="m" />
+        </div>
+      );
+    });
 
   return (
     <EuiModal onClose={onCloseModal} maxWidth={false}>
       <EuiModalHeader>
         <EuiModalHeaderTitle>
-          <h2>what are you looking to do?</h2>
-          <EuiText color="subdued">find a tool based on your task or goal</EuiText>
+          <h2>What are you looking to do?</h2>
+          <EuiText color="subdued">Find a tool based on your task or goal</EuiText>
         </EuiModalHeaderTitle>
       </EuiModalHeader>
 
       <EuiModalBody>
-        <EuiPage restrictWidth={1200}>
-          <EuiPageSideBar paddingSize="s" sticky>
+        <EuiPage restrictWidth={1300} style={{ background: 'transparent' }}>
+          <EuiPageSideBar paddingSize="s">
             <EuiSideNav items={sideNav} style={{ width: 170 }} />
           </EuiPageSideBar>
           <EuiPageBody paddingSize="s">
-            <EuiPageContent verticalPosition="center" horizontalPosition="center" paddingSize="m">
-              <div ref={allRef}>{cardList}</div>
+            <EuiPageContent paddingSize="m">
+              <div
+                style={{ maxHeight: '50vh', overflowY: 'auto', overflowX: 'hidden' }}
+                ref={allRef}
+              >
+                {cardList}
+              </div>
             </EuiPageContent>
           </EuiPageBody>
         </EuiPage>

diff --git a/src/plugins/workspace/public/components/workspace_overview/index.tsx b/src/plugins/workspace/public/components/workspace_overview/index.tsx
@@ -18,6 +18,7 @@ import {
   EuiBadge,
   EuiFlexGroup,
   EuiPanel,
+  EuiToolTip,
 } from '@elastic/eui';
 
 import { useObservable } from 'react-use';
@@ -34,7 +35,7 @@ import { featureMatchesConfig } from '../../utils';
 import { WorkspaceOverviewCard } from './getting_start_card';
 import { WorkspaceOverviewGettingStartModal } from './getting_start_modal';
 import { convertPermissionsToPermissionSettings } from '../workspace_form/utils';
-import { WORKSPACE_UPDATE_APP_ID } from '../../../../workspace/common/constants';
+import { WORKSPACE_UPDATE_APP_ID } from '../../../common/constants';
 
 export const WorkspaceOverview = () => {
   const {
@@ -70,19 +71,25 @@ export const WorkspaceOverview = () => {
   if (isPermissionEnabled && currentWorkspace && currentWorkspace.permissions) {
     const permissions = convertPermissionsToPermissionSettings(currentWorkspace.permissions);
 
-    const users: string[] = permissions
+    const users = permissions
       .filter((permission) => permission.type === WorkspacePermissionItemType.User)
-      .map((permission) => (permission as { userId: string }).userId);
-    const groups: string[] = permissions
+      .map((permission) => {
+        return { type: permission.type, id: (permission as { userId: string }).userId };
+      });
+    const groups = permissions
       .filter((permission) => permission.type === WorkspacePermissionItemType.Group)
-      .map((permission) => (permission as { group: string }).group);
+      .map((permission) => {
+        return { type: permission.type, id: (permission as { group: string }).group };
+      });
 
     const allUserAndGroup = [...new Set(users.concat(groups))];
 
     const userList = allUserAndGroup.slice(0, 3).map((user) => {
       return (
-        <EuiFlexItem key={user}>
-          <EuiAvatar size="m" name={user} />
+        <EuiFlexItem key={user.id}>
+          <EuiToolTip content={user.id} position="bottom">
+            <EuiAvatar size="m" name={user.id} type={user.type === 'group' ? 'space' : 'user'} />
+          </EuiToolTip>
         </EuiFlexItem>
       );
     });
@@ -119,7 +126,7 @@ export const WorkspaceOverview = () => {
     {
       id: 'settings',
       name: i18n.translate('workspace.setting.title', {
-        defaultMessage: 'Setting',
+        defaultMessage: 'Settings',
       }),
       content: <WorkspaceOverviewSettings />,
     },
@@ -131,12 +138,12 @@ export const WorkspaceOverview = () => {
       style={{ backgroundColor: 'transparent', padding: '8px' }}
       iconType={isGettingStartVisible ? 'arrowUp' : 'arrowDown'}
       iconSide="right"
-      onClickAriaLabel="Click to toggle start working cards"
+      onClickAriaLabel={isGettingStartVisible ? 'Collapse' : 'Expand'}
       onClick={() => {
         setIsStartWorkingVisible(!isGettingStartVisible);
       }}
     >
-      {isGettingStartVisible ? 'collapse' : 'expand'}
+      {isGettingStartVisible ? 'Collapse' : 'Expand'}
     </EuiBadge>
   );
 
@@ -182,7 +189,7 @@ export const WorkspaceOverview = () => {
                         setIsModalVisible(true);
                       }}
                     >
-                      <EuiText>see more ways to get started</EuiText>
+                      <EuiText size="s">see more ways to get started</EuiText>
                     </EuiPanel>
                   </EuiFlexItem>
                 ) : null}

diff --git a/src/plugins/workspace/public/components/workspace_overview/types.ts b/src/plugins/workspace/public/components/workspace_overview/types.ts
@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { AppCategory } from '../../../../../core/types';
+
 export interface GetStartCard {
   /**
    * application id that used for filter, if app id is not speicified, that means it will always display

diff --git a/src/plugins/workspace/public/components/workspace_overview/workspace_overview_content.tsx b/src/plugins/workspace/public/components/workspace_overview/workspace_overview_content.tsx
@@ -3,11 +3,10 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { EuiTitle, EuiFlexItem, EuiCard, EuiSpacer, EuiPanel, EuiFlexGroup } from '@elastic/eui';
+import { EuiTitle, EuiFlexItem, EuiCard, EuiPanel, EuiFlexGroup } from '@elastic/eui';
 import React from 'react';
 import { useObservable } from 'react-use';
 import { of } from 'rxjs';
-import { i18n } from '@osd/i18n';
 import { useOpenSearchDashboards } from '../../../../../plugins/opensearch_dashboards_react/public';
 
 export const WorkspaceOverviewContent = () => {
@@ -19,14 +18,6 @@ export const WorkspaceOverviewContent = () => {
 
   return (
     <EuiPanel hasBorder={false} color="transparent">
-      <EuiTitle>
-        <span>
-          {i18n.translate('workspace.overview.title', {
-            defaultMessage: 'Overview',
-          })}
-        </span>
-      </EuiTitle>
-      <EuiSpacer size="l" />
       <EuiFlexGroup>
         <EuiFlexItem grow={2}>
           <EuiCard

diff --git a/src/plugins/workspace/public/components/workspace_overview_app.tsx b/src/plugins/workspace/public/components/workspace_overview_app.tsx
@@ -5,27 +5,34 @@
 
 import React, { useEffect } from 'react';
 import { I18nProvider } from '@osd/i18n/react';
-import { i18n } from '@osd/i18n';
+import { CoreStart } from 'opensearch-dashboards/public';
+import { useObservable } from 'react-use';
 import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public';
 import { WorkspaceOverview } from './workspace_overview';
 
 export const WorkspaceOverviewApp = () => {
   const {
-    services: { chrome },
-  } = useOpenSearchDashboards();
+    services: { workspaces, chrome, application },
+  } = useOpenSearchDashboards<CoreStart>();
+
+  const currentWorkspace = useObservable(workspaces.currentWorkspace$);
 
   /**
    * set breadcrumbs to chrome
    */
   useEffect(() => {
     chrome?.setBreadcrumbs([
       {
-        text: i18n.translate('workspace.workspaceOverviewTitle', {
-          defaultMessage: 'Workspace overview',
-        }),
+        text: 'Home',
+        onClick: () => {
+          application.navigateToApp('home');
+        },
+      },
+      {
+        text: currentWorkspace?.name,
       },
     ]);
-  }, [chrome]);
+  }, [chrome, currentWorkspace, application]);
 
   return (
     <I18nProvider>