Move form view to edit mode only

Signed-off-by: Charles Thao <cthao@redhat.com>
2025-06-30 13:41:46 -04:00 · 2025-06-30 13:41:46 -04:00 · 51a7fbec69
parent df265d4888
commit 51a7fbec69
7 changed files with 127 additions and 97 deletions
--- a/workspaces/frontend/src/app/AppRoutes.tsx
+++ b/workspaces/frontend/src/app/AppRoutes.tsx
@ -68,6 +68,7 @@ const AppRoutes: React.FC = () => {
      <Route path={AppRoutePaths.workspaceKindSummary} element={<WorkspaceKindSummaryWrapper />} />
      <Route path={AppRoutePaths.workspaceKinds} element={<WorkspaceKinds />} />
      <Route path={AppRoutePaths.workspaceKindCreate} element={<WorkspaceKindForm />} />
+      <Route path={AppRoutePaths.workspaceKindEdit} element={<WorkspaceKindForm />} />
      <Route path="/" element={<Navigate to={AppRoutePaths.workspaces} replace />} />
      <Route path="*" element={<NotFound />} />
      {
--- a/workspaces/frontend/src/app/hooks/useWorkspaceFormLocationData.ts
+++ b/workspaces/frontend/src/app/hooks/useWorkspaceFormLocationData.ts
@ -3,12 +3,17 @@ import { useCurrentRouteKey } from '~/app/hooks/useCurrentRouteKey';
 import { useTypedLocation } from '~/app/routerHelper';
 import { AppRouteKey, RouteStateMap } from '~/app/routes';

-type WorkspaceFormLocationState = RouteStateMap['workspaceEdit'] | RouteStateMap['workspaceCreate'];
+type WorkspaceFormLocationState =
+  | RouteStateMap['workspaceEdit']
+  | RouteStateMap['workspaceCreate']
+  | RouteStateMap['workspaceKindEdit']
+  | RouteStateMap['workspaceKindCreate'];

 interface WorkspaceFormLocationData {
  mode: 'edit' | 'create';
  namespace: string;
  workspaceName?: string;
+  workspaceKindName?: string;
 }

 function getRouteStateIfMatch<K extends AppRouteKey>(
@ -25,10 +30,13 @@ function getRouteStateIfMatch<K extends AppRouteKey>(

 export function useWorkspaceFormLocationData(): WorkspaceFormLocationData {
  const { selectedNamespace } = useNamespaceContext();
-  const location = useTypedLocation<'workspaceEdit' | 'workspaceCreate'>();
+  const location = useTypedLocation<
+    'workspaceEdit' | 'workspaceCreate' | 'workspaceKindEdit' | 'workspaceKindCreate'
+  >();
  const routeKey = useCurrentRouteKey();
  const rawState = location.state as WorkspaceFormLocationState | undefined;

+  // Workspace Edit Mode
  if (routeKey === 'workspaceEdit') {
    const editState = getRouteStateIfMatch('workspaceEdit', routeKey, rawState);
    const namespace = editState?.namespace ?? selectedNamespace;
@ -45,6 +53,7 @@ export function useWorkspaceFormLocationData(): WorkspaceFormLocationData {
    };
  }

+  // Workspace Create Mode
  if (routeKey === 'workspaceCreate') {
    const createState = getRouteStateIfMatch('workspaceCreate', routeKey, rawState);
    const namespace = createState?.namespace ?? selectedNamespace;
@ -55,5 +64,35 @@ export function useWorkspaceFormLocationData(): WorkspaceFormLocationData {
    };
  }

+  // Workspace Kind Edit Mode
+  if (routeKey === 'workspaceKindEdit') {
+    const editState = getRouteStateIfMatch('workspaceKindEdit', routeKey, rawState);
+    const namespace = editState?.namespace ?? selectedNamespace;
+    // TODO: remove default jupyterlab from workspace
+    const workspaceKindName = editState?.workspaceKindName || 'jupyterlab';
+
+    if (!workspaceKindName) {
+      throw new Error('Workspace kind name is required for edit mode');
+    }
+
+    return {
+      mode: 'edit',
+      namespace,
+      workspaceKindName,
+    };
+  }
+
+  // Workspace Kind Create Mode
+  if (routeKey === 'workspaceKindCreate') {
+    const createState = getRouteStateIfMatch('workspaceKindCreate', routeKey, rawState);
+    const namespace = createState?.namespace ?? selectedNamespace;
+
+    return {
+      mode: 'create',
+      namespace,
+      // formType: 'workspaceKind',
+    };
+  }
+
  throw new Error('Unknown workspace form route');
 }
--- a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/WorkspaceKindForm.tsx
+++ b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/WorkspaceKindForm.tsx
@ -8,13 +8,12 @@ import {
  PageGroup,
  PageSection,
  Stack,
-  ToggleGroup,
-  ToggleGroupItem,
 } from '@patternfly/react-core';
 import { useTypedNavigate } from '~/app/routerHelper';
 import useGenericObjectState from '~/app/hooks/useGenericObjectState';
 import { useNotebookAPI } from '~/app/hooks/useNotebookAPI';
 import { WorkspaceKindFormData } from '~/app/types';
+import { useWorkspaceFormLocationData } from '~/app/hooks/useWorkspaceFormLocationData';
 import { WorkspaceKindFileUpload } from './fileUpload/WorkspaceKindFileUpload';
 import { WorkspaceKindFormProperties } from './properties/WorkspaceKindFormProperties';
 import { WorkspaceKindFormImage } from './image/WorkspaceKindFormImage';
@ -31,12 +30,10 @@ export const WorkspaceKindForm: React.FC = () => {
  const navigate = useTypedNavigate();
  const { api } = useNotebookAPI();
  // TODO: Detect mode by route
-  const [mode] = useState('create');
+  const { mode } = useWorkspaceFormLocationData();
  const [yamlValue, setYamlValue] = useState('');
  const [isSubmitting, setIsSubmitting] = useState(false);
-  const [view, setView] = useState<WorkspaceKindFormView>(WorkspaceKindFormView.FileUpload);
  const [validated, setValidated] = useState<ValidationStatus>('default');
-  const workspaceKindFileUploadId = 'workspace-kind-form-fileupload-view';

  const [data, setData, resetData] = useGenericObjectState<WorkspaceKindFormData>({
    properties: {
@ -58,18 +55,6 @@ export const WorkspaceKindForm: React.FC = () => {
    },
  });

-  const handleViewClick = useCallback(
-    (event: React.MouseEvent<unknown> | React.KeyboardEvent | MouseEvent) => {
-      const { id } = event.currentTarget as HTMLElement;
-      setView(
-        id === workspaceKindFileUploadId
-          ? WorkspaceKindFormView.FileUpload
-          : WorkspaceKindFormView.Form,
-      );
-    },
-    [],
-  );
-
  const handleSubmit = useCallback(async () => {
    setIsSubmitting(true);
    // TODO: Complete handleCreate with API call to create a new WS kind
@ -106,37 +91,18 @@ export const WorkspaceKindForm: React.FC = () => {
                  {`${mode === 'create' ? 'Create' : 'Edit'} workspace kind`}
                </Content>
                <Content component={ContentVariants.p}>
-                  {view === WorkspaceKindFormView.FileUpload
+                  {mode === 'create'
                    ? `Please upload or drag and drop a Workspace Kind YAML file.`
                    : `View and edit the Workspace Kind's information. Some fields may not be
                      represented in this form`}
                </Content>
              </FlexItem>
-              {mode === 'edit' && (
-                <FlexItem>
-                  <ToggleGroup className="workspace-kind-form-header" aria-label="Toggle form view">
-                    <ToggleGroupItem
-                      text="YAML Upload"
-                      buttonId={workspaceKindFileUploadId}
-                      isSelected={view === WorkspaceKindFormView.FileUpload}
-                      onChange={handleViewClick}
-                    />
-                    <ToggleGroupItem
-                      text="Form View"
-                      buttonId="workspace-kind-form-form-view"
-                      isSelected={view === WorkspaceKindFormView.Form}
-                      onChange={handleViewClick}
-                      isDisabled={yamlValue === '' || validated === 'error'}
-                    />
-                  </ToggleGroup>
-                </FlexItem>
-              )}
            </Flex>
          </Stack>
        </PageSection>
      </PageGroup>
      <PageSection isFilled>
-        {view === WorkspaceKindFormView.FileUpload && (
+        {mode === 'create' && (
          <WorkspaceKindFileUpload
            setData={setData}
            resetData={resetData}
@ -146,7 +112,7 @@ export const WorkspaceKindForm: React.FC = () => {
            setValidated={setValidated}
          />
        )}
-        {view === WorkspaceKindFormView.Form && (
+        {mode === 'edit' && (
          <>
            <WorkspaceKindFormProperties
              mode={mode}
--- a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/podConfig/ResourceInputWrapper.tsx
+++ b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/podConfig/ResourceInputWrapper.tsx
@ -7,6 +7,7 @@ import {
  SplitItem,
 } from '@patternfly/react-core';
 import { CPU_UNITS, MEMORY_UNITS_FOR_SELECTION, UnitOption } from '~/shared/utilities/valueUnits';
+import { extractNumericValue, extractUnit } from '~/shared/utilities/WorkspaceUtils';

 interface ResourceInputWrapperProps {
  value: string;
@ -29,10 +30,15 @@ const unitMap: {

 const DEFAULT_STEP = 1;

+const DEFAULT_UNITS = {
+  memory: 'Mi',
+  cpu: '',
+};
+
 export const ResourceInputWrapper: React.FC<ResourceInputWrapperProps> = ({
  value,
  onChange,
-  min = 0,
+  min = 1,
  max,
  step = DEFAULT_STEP,
  type,
@ -44,37 +50,23 @@ export const ResourceInputWrapper: React.FC<ResourceInputWrapperProps> = ({
  const [unit, setUnit] = useState<string>('');

  useEffect(() => {
-    if (type === 'memory') {
-      // Extract numeric value and unit from memory string (e.g., "512Mi" -> "512" and "Mi")
-      const match = value.match(/^(\d+)([MGTP]i)?$/i);
-      if (match) {
-        setInputValue(match[1]);
-        setUnit(match[2] || 'Mi');
-      } else {
-        setInputValue('');
-        setUnit('Mi');
-      }
-    } else if (type === 'cpu') {
-      const match = value.match(/^(\d+)([m])?$/i);
-      if (match) {
-        setInputValue(match[1]);
-        setUnit(match[2] || '');
-      } else {
-        setInputValue('');
-        setUnit('');
-      }
-    } else {
+    if (type === 'custom') {
      setInputValue(value);
+      return;
    }
+    const numericValue = extractNumericValue(value, type);
+    const extractedUnit = extractUnit(value, type);
+    setInputValue(numericValue);
+    setUnit(extractedUnit || DEFAULT_UNITS[type]);
  }, [value, type]);

  const handleInputChange = useCallback(
    (newValue: string) => {
      setInputValue(newValue);
-      if (type === 'memory' || type === 'cpu') {
-        onChange(newValue ? `${newValue}${unit}` : '');
-      } else {
+      if (type === 'custom') {
        onChange(newValue);
+      } else {
+        onChange(newValue ? `${newValue}${unit}` : '');
      }
    },
    [onChange, type, unit],
@ -110,7 +102,6 @@ export const ResourceInputWrapper: React.FC<ResourceInputWrapperProps> = ({
    [handleInputChange],
  );

-  // Memoize the unit options to prevent unnecessary re-renders
  const unitOptions = useMemo(
    () =>
      type !== 'custom'
@ -123,7 +114,7 @@ export const ResourceInputWrapper: React.FC<ResourceInputWrapperProps> = ({
    <Split className="workspacekind-form-resource-input">
      <SplitItem>
        <NumberInput
-          value={parseFloat(inputValue) || 0}
+          value={parseFloat(inputValue) || 1}
          placeholder={placeholder}
          onMinus={handleDecrement}
          onChange={handleNumberInputChange}
--- a/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/podConfig/WorkspaceKindFormPodConfigResource.tsx
+++ b/workspaces/frontend/src/app/pages/WorkspaceKinds/Form/podConfig/WorkspaceKindFormPodConfigResource.tsx
@ -8,6 +8,8 @@ import {
  FormFieldGroupHeader,
  TextInput,
  Checkbox,
+  Content,
+  ContentVariants,
 } from '@patternfly/react-core';
 import { PlusCircleIcon, TrashAltIcon } from '@patternfly/react-icons';
 import { ResourceInputWrapper } from './ResourceInputWrapper';
@ -32,12 +34,10 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
  custom,
 }) => {
  // State for tracking limit toggles
-  const [cpuRequestEnabled, setCpuRequestEnabled] = useState<boolean>(cpu.request.length > 0);
-  const [memoryRequestEnabled, setMemoryRequestEnabled] = useState<boolean>(
-    memory.request.length > 0,
-  );
-  const [cpuLimitEnabled, setCpuLimitEnabled] = useState<boolean>(cpu.limit.length > 0);
-  const [memoryLimitEnabled, setMemoryLimitEnabled] = useState<boolean>(memory.limit.length > 0);
+  const [cpuRequestEnabled, setCpuRequestEnabled] = useState(cpu.request.length > 0);
+  const [memoryRequestEnabled, setMemoryRequestEnabled] = useState(memory.request.length > 0);
+  const [cpuLimitEnabled, setCpuLimitEnabled] = useState(cpu.limit.length > 0);
+  const [memoryLimitEnabled, setMemoryLimitEnabled] = useState(memory.limit.length > 0);
  const [customLimitsEnabled, setCustomLimitsEnabled] = useState<Record<number, boolean>>(() => {
    const customToggles: Record<number, boolean> = {};
    custom.forEach((res, idx) => {
@ -67,7 +67,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
  const handleAddCustom = useCallback(() => {
    setResources((resources: PodResourceEntry[]) => [
      ...resources,
-      { type: '', request: '', limit: '' },
+      { type: '', request: '1', limit: '' },
    ]);
  }, [setResources]);

@ -97,16 +97,18 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
  const handleCpuLimitToggle = useCallback(
    (enabled: boolean) => {
      setCpuLimitEnabled(enabled);
+      handleChange('cpu', 'limit', cpu.request);
      if (!enabled) {
        handleChange('cpu', 'limit', '');
      }
    },
-    [handleChange],
+    [cpu.request, handleChange],
  );

  const handleCpuRequestToggle = useCallback(
    (enabled: boolean) => {
      setCpuRequestEnabled(enabled);
+      handleChange('cpu', 'request', '1');
      if (!enabled) {
        handleChange('cpu', 'request', '');
        handleCpuLimitToggle(enabled);
@ -118,16 +120,18 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
  const handleMemoryLimitToggle = useCallback(
    (enabled: boolean) => {
      setMemoryLimitEnabled(enabled);
+      handleChange('memory', 'limit', memory.request);
      if (!enabled) {
        handleChange('memory', 'limit', '');
      }
    },
-    [handleChange],
+    [handleChange, memory.request],
  );

  const handleMemoryRequestToggle = useCallback(
    (enabled: boolean) => {
      setMemoryRequestEnabled(enabled);
+      handleChange('memory', 'request', '1Mi');
      if (!enabled) {
        handleChange('memory', 'request', '');
        handleMemoryLimitToggle(enabled);
@ -159,9 +163,9 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
            id: 'workspace-kind-podconfig-resource',
          }}
          titleDescription={
-            <p style={{ fontSize: '12px' }}>
-              Optional: Configure k8s Pod Resource Requests & Limits
-            </p>
+            <Content component={ContentVariants.p} style={{ fontSize: '12px' }}>
+              Optional: Configure k8s Pod Resource Requests & Limits.
+            </Content>
          }
        />
      }
@ -190,7 +194,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
            value={cpu.request}
            onChange={(value) => handleChange('cpu', 'request', value)}
            placeholder="e.g. 1"
-            min={0}
+            min={1}
            aria-label="CPU request"
            isDisabled={!cpuRequestEnabled}
          />
@ -201,7 +205,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
            value={memory.request}
            onChange={(value) => handleChange('memory', 'request', value)}
            placeholder="e.g. 512Mi"
-            min={0}
+            min={1}
            aria-label="Memory request"
            isDisabled={!memoryRequestEnabled}
          />
@ -232,7 +236,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
            value={cpu.limit}
            onChange={(value) => handleChange('cpu', 'limit', value)}
            placeholder="e.g. 2"
-            min={0}
+            min={parseFloat(cpu.request)}
            step={1}
            aria-label="CPU limit"
            isDisabled={!cpuRequestEnabled || !cpuLimitEnabled}
@ -244,7 +248,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
            value={memory.limit}
            onChange={(value) => handleChange('memory', 'limit', value)}
            placeholder="e.g. 1Gi"
-            min={0}
+            min={parseFloat(memory.request)}
            aria-label="Memory limit"
            isDisabled={!memoryRequestEnabled || !memoryLimitEnabled}
          />
@ -278,7 +282,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
              value={res.request}
              onChange={(value) => handleChange(res.type, 'request', value)}
              placeholder="Request"
-              min={0}
+              min={1}
              aria-label="Custom resource request"
            />
          </GridItem>
@ -287,7 +291,10 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
              id={`custom-limit-switch-${idx}`}
              label="Set Limit"
              isChecked={customLimitsEnabled[idx] || false}
-              onChange={(_event, checked) => handleCustomLimitToggle(idx, checked)}
+              onChange={(_event, checked) => {
+                handleChange(res.type, 'limit', res.request);
+                handleCustomLimitToggle(idx, checked);
+              }}
              aria-label={`Enable limit for ${res.type || 'custom resource'}`}
            />
          </GridItem>
@ -297,7 +304,7 @@ export const WorkspaceKindFormPodConfigResource: React.FC<Props> = ({
              value={res.limit}
              onChange={(value) => handleChange(res.type, 'limit', value)}
              placeholder="Limit"
-              min={0}
+              min={parseFloat(res.request)}
              isDisabled={!customLimitsEnabled[idx]}
              aria-label={`${res.type || 'Custom resource'} limit`}
            />
--- a/workspaces/frontend/src/app/routes.ts
+++ b/workspaces/frontend/src/app/routes.ts
@ -6,6 +6,7 @@ export const AppRoutePaths = {
  workspaceKinds: '/workspacekinds',
  workspaceKindSummary: '/workspacekinds/:kind/summary',
  workspaceKindCreate: '/workspacekinds/create',
+  workspaceKindEdit: '/workspacekinds/edit',
 } satisfies Record<string, `/${string}`>;

 export type AppRoute = (typeof AppRoutePaths)[keyof typeof AppRoutePaths];
@ -31,6 +32,7 @@ export type RouteParamsMap = {
    kind: string;
  };
  workspaceKindCreate: undefined;
+  workspaceKindEdit: undefined;
 };

 /**
@ -62,6 +64,10 @@ export type RouteStateMap = {
  workspaceKindCreate: {
    namespace: string;
  };
+  workspaceKindEdit: {
+    namespace: string;
+    workspaceKindName: string;
+  };
 };

 /**
@ -82,4 +88,5 @@ export type RouteSearchParamsMap = {
  workspaceKinds: undefined;
  workspaceKindSummary: undefined;
  workspaceKindCreate: undefined;
+  workspaceKindEdit: undefined;
 };
--- a/workspaces/frontend/src/shared/utilities/WorkspaceUtils.ts
+++ b/workspaces/frontend/src/shared/utilities/WorkspaceUtils.ts
@ -13,6 +13,30 @@ export enum YesNoValue {
  No = 'No',
 }

+const RESOURCE_UNIT_CONFIG = {
+  cpu: CPU_UNITS,
+  memory: MEMORY_UNITS_FOR_PARSING,
+  gpu: OTHER,
+};
+
+export const parseResourceValue = (
+  value: string,
+  resourceType: ResourceType,
+): [number | undefined, { name: string; unit: string } | undefined] => {
+  const units = RESOURCE_UNIT_CONFIG[resourceType];
+  return splitValueUnit(value, units);
+};
+
+export const extractNumericValue = (value: string, resourceType: ResourceType): string => {
+  const [numericValue] = parseResourceValue(value, resourceType);
+  return String(numericValue || '');
+};
+
+export const extractUnit = (value: string, resourceType: ResourceType): string => {
+  const [, unit] = parseResourceValue(value, resourceType);
+  return unit?.unit || '';
+};
+
 export const extractResourceValue = (
  workspace: Workspace,
  resourceType: ResourceType,
@ -24,18 +48,13 @@ export const formatResourceValue = (v: string | undefined, resourceType?: Resour
  if (v === undefined) {
    return '-';
  }
-  switch (resourceType) {
-    case 'cpu': {
-      const [cpuValue, cpuUnit] = splitValueUnit(v, CPU_UNITS);
-      return `${cpuValue ?? ''} ${cpuUnit.name}`;
-    }
-    case 'memory': {
-      const [memoryValue, memoryUnit] = splitValueUnit(v, MEMORY_UNITS_FOR_PARSING);
-      return `${memoryValue ?? ''} ${memoryUnit.name}`;
-    }
-    default:
+
+  if (!resourceType) {
    return v;
  }
+
+  const [value, unit] = parseResourceValue(v, resourceType);
+  return `${value || ''} ${unit?.name || ''}`.trim();
 };

 export const formatResourceFromWorkspace = (