fix: not all resource pools should be labeled "default" [WEB-1600] (#…

…7744) * Fix label and CSS simplification * More CSS simplifications (cherry picked from commit e1675b2)
determined-ai · Aug 28, 2023 · 9fde7c7 · 9fde7c7
1 parent 154168f
commit 9fde7c7
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 91 deletions.
diff --git a/webui/react/src/components/ResourcePoolCard.module.scss b/webui/react/src/components/ResourcePoolCard.module.scss
@@ -1,33 +1,22 @@
-$font-size-content: 12px;
-
 .base {
-  display: flex;
-  flex-direction: column;
-  font-size: 14px;
+  font-size: 12px;
   line-height: 22px;
-  overflow: hidden;
-  text-overflow: ellipsis;
 
   .body {
-    display: flex;
-    flex-direction: column;
-    flex-grow: 1;
-
     .resoucePoolBoundContainer {
       align-items: center;
-      border-bottom: 1px solid var(--theme-stage-border);
-      border-top: 1px solid var(--theme-stage-border);
-      display: inline-flex;
-      font-size: $font-size-content;
+      border-block: 1px solid var(--theme-stage-border);
+      display: flex;
       font-weight: bold;
       justify-content: space-between;
       margin: 8px 0;
       padding: 8px 0;
-    }
-    .resoucePoolBoundCount {
-      align-items: center;
-      display: inline-flex;
-      gap: 4px;
+
+      .resoucePoolBoundCount {
+        align-items: center;
+        display: flex;
+        gap: 4px;
+      }
     }
   }
   .header {
@@ -37,37 +26,13 @@ $font-size-content: 12px;
     gap: 8px;
     padding-top: 8px;
 
-    .info {
-      margin: auto 0;
-
-      .name {
-        font-weight: bold;
-      }
+    .name {
+      font-size: 14px;
+      font-weight: bold;
     }
-    .default {
-      color: var(--theme-surface-on);
-
-      span {
-        font-size: $font-size-content;
-        margin-right: 4px;
-      }
-      i {
-        color: var(--theme-surface-on-weak);
-        font-weight: bold;
-      }
+    .details {
+      display: flex;
+      gap: 4px;
     }
   }
-  .empty {
-    color: var(--theme-surface-on-weak);
-  }
-}
-section.details {
-  display: flex;
-  flex-direction: column;
-  flex-grow: 1;
-  font-size: $font-size-content;
-
-  ul {
-    flex-grow: 1;
-  }
 }
diff --git a/webui/react/src/components/ResourcePoolCard.tsx b/webui/react/src/components/ResourcePoolCard.tsx
@@ -28,6 +28,7 @@ import { useObservable } from 'utils/observable';
 import { pluralizer } from 'utils/string';
 import { DarkLight } from 'utils/themes';
 
+import { ConditionalWrapper } from './ConditionalWrapper';
 import Json from './Json';
 import ResourcePoolBindingModalComponent from './ResourcePoolBindingModal';
 import css from './ResourcePoolCard.module.scss';
@@ -37,7 +38,8 @@ interface Props {
   poolStats?: V1RPQueueStat | undefined;
   resourcePool: ResourcePool;
   size?: ShirtSize;
-  descriptiveLabel?: string;
+  defaultAux?: boolean;
+  defaultCompute?: boolean;
 }
 
 const poolAttributes = [
@@ -94,24 +96,26 @@ export const PoolLogo: React.FC<{ type: V1ResourcePoolType }> = ({ type }) => {
 const ResourcePoolCard: React.FC<Props> = ({
   resourcePool: pool,
   actionMenu,
-  descriptiveLabel,
+  defaultAux,
+  defaultCompute,
 }: Props) => {
   const rpBindingFlagOn = useFeature().isOn('rp_binding');
   const { canManageResourcePoolBindings } = usePermissions();
   const ResourcePoolBindingModal = useModal(ResourcePoolBindingModalComponent);
-  const isDefaultPool = pool.defaultAuxPool || pool.defaultComputePool;
-  const descriptionClasses = [css.description];
-  const showDescriptiveLabel = !(canManageResourcePoolBindings && rpBindingFlagOn) ?? isDefaultPool;
   const resourcePoolBindingMap = useObservable(clusterStore.resourcePoolBindings);
   const resourcePoolBindings: number[] = resourcePoolBindingMap.get(pool.name, []);
   const workspaces = Loadable.getOrElse([], useObservable(workspaceStore.workspaces));
 
+  const defaultLabel = useMemo(() => {
+    if (defaultAux && defaultCompute) return 'Default';
+    if (defaultAux) return 'Default Aux';
+    if (defaultCompute) return 'Default Compute';
+  }, [defaultAux, defaultCompute]);
+
   useEffect(() => {
     return clusterStore.fetchResourcePoolBindings(pool.name);
   }, [pool.name]);
 
-  if (!pool.description) descriptionClasses.push(css.empty);
-
   const isAux = useMemo(() => {
     return pool.auxContainerCapacityPerAgent > 0;
   }, [pool]);
@@ -166,19 +170,18 @@ const ResourcePoolCard: React.FC<Props> = ({
         onDropdown={onDropdown}>
         <div className={css.base}>
           <div className={css.header}>
-            <div className={css.info}>
-              <div className={css.name}>{pool.name}</div>
-            </div>
-            <div className={css.default}>
-              {showDescriptiveLabel && <span>{descriptiveLabel}</span>}
-              {showDescriptiveLabel && pool.description && (
-                <Icon name="info" showTooltip title={pool.description} />
-              )}
-              {!showDescriptiveLabel && (
-                <Tooltip content="You cannot bind your default resource pool to a workspace.">
-                  <span>Default</span>
-                </Tooltip>
-              )}
+            <div className={css.name}>{pool.name}</div>
+            <div className={css.details}>
+              <ConditionalWrapper
+                condition={!!defaultLabel && canManageResourcePoolBindings}
+                wrapper={(children) => (
+                  <Tooltip content="You cannot bind your default resource pool to a workspace.">
+                    {children}
+                  </Tooltip>
+                )}>
+                <span>{defaultLabel}</span>
+              </ConditionalWrapper>
+              {pool.description && <Icon name="info" showTooltip title={pool.description} />}
             </div>
           </div>
           <Suspense fallback={<Spinner center spinning />}>
@@ -194,9 +197,7 @@ const ResourcePoolCard: React.FC<Props> = ({
                   </div>
                 </section>
               )}
-              <section className={css.details}>
-                <Json hideDivider json={shortDetails} />
-              </section>
+              <Json hideDivider json={shortDetails} />
               <div />
             </div>
           </Suspense>

diff --git a/webui/react/src/pages/Clusters/ClustersOverview.tsx b/webui/react/src/pages/Clusters/ClustersOverview.tsx
@@ -39,12 +39,6 @@ const ClusterOverview: React.FC = () => {
     [canManageResourcePoolBindings, rpBindingFlagOn],
   );
 
-  const renderDefaultLabel = useCallback((pool: ResourcePool) => {
-    if (pool.defaultAuxPool && pool.defaultComputePool) return 'Default';
-    if (pool.defaultComputePool) return 'Default Aux';
-    if (pool.defaultAuxPool) return 'Default Compute';
-  }, []);
-
   return (
     <>
       <ClusterOverallStats />
@@ -55,7 +49,8 @@ const ClusterOverview: React.FC = () => {
             resourcePools.data.map((rp) => (
               <ResourcePoolCard
                 actionMenu={actionMenu(rp)}
-                descriptiveLabel={renderDefaultLabel(rp)}
+                defaultAux={rp.defaultAuxPool}
+                defaultCompute={rp.defaultComputePool}
                 key={rp.name}
                 resourcePool={rp}
               />

diff --git a/webui/react/src/pages/WorkspaceDetails/ResourcePoolsBound.tsx b/webui/react/src/pages/WorkspaceDetails/ResourcePoolsBound.tsx
@@ -38,16 +38,6 @@ const ResourcePoolsBound: React.FC<Props> = ({ workspace }) => {
     );
   }, [resourcePools, boundResourcePoolNames, unboundResourcePools]);
 
-  const renderDefaultLabel = useCallback(
-    (pool: ResourcePool) => {
-      if (pool.name === workspace.defaultAuxPool && pool.name === workspace.defaultComputePool)
-        return 'Default';
-      if (pool.name === workspace.defaultAuxPool) return 'Default Aux';
-      if (pool.name === workspace.defaultComputePool) return 'Default Compute';
-    },
-    [workspace.defaultComputePool, workspace.defaultAuxPool],
-  );
-
   const actionMenu = useCallback(
     (pool: ResourcePool) =>
       canManageResourcePoolBindings
@@ -98,7 +88,8 @@ const ResourcePoolsBound: React.FC<Props> = ({ workspace }) => {
             {boundResourcePools.map((rp) => (
               <ResourcePoolCard
                 actionMenu={actionMenu(rp)}
-                descriptiveLabel={renderDefaultLabel(rp)}
+                defaultAux={rp.name === workspace.defaultAuxPool}
+                defaultCompute={rp.name === workspace.defaultComputePool}
                 key={rp.name}
                 resourcePool={rp}
               />