Fix new workspace page re-render loop and false negative error messag…

…e for ws class (gitpod-io#19492) * Add loading state for allowed workspace class memo * Fix rerender max
ZDSRKJ · Mar 1, 2024 · 0aafc58 · 0aafc58
1 parent 767c80f
commit 0aafc58
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 49 deletions.
diff --git a/components/dashboard/src/components/SelectWorkspaceClassComponent.tsx b/components/dashboard/src/components/SelectWorkspaceClassComponent.tsx
@@ -34,9 +34,12 @@ export default function SelectWorkspaceClassComponent({
     const enabledWorkspaceClassRestrictionOnConfiguration = useFeatureFlag(
         "configuration_workspace_class_restrictions",
     );
-    const { data: workspaceClasses } = useAllowedWorkspaceClassesMemo(selectedConfigurationId, {
-        filterOutDisabled: true,
-    });
+    const { data: workspaceClasses, isLoading: workspaceClassesLoading } = useAllowedWorkspaceClassesMemo(
+        selectedConfigurationId,
+        {
+            filterOutDisabled: true,
+        },
+    );
 
     const getElements = useCallback((): ComboboxElement[] => {
         return (workspaceClasses || [])?.map((c) => ({
@@ -47,7 +50,7 @@ export default function SelectWorkspaceClassComponent({
     }, [workspaceClasses]);
 
     useEffect(() => {
-        if (!workspaceClasses) {
+        if (!workspaceClasses || loading || disabled || workspaceClassesLoading) {
             return;
         }
 
@@ -79,6 +82,9 @@ export default function SelectWorkspaceClassComponent({
             setError?.(`The workspace class '${selectedWorkspaceClass}' is not supported.`);
         }
     }, [
+        loading,
+        workspaceClassesLoading,
+        disabled,
         workspaceClasses,
         selectedWorkspaceClass,
         setError,
@@ -108,9 +114,12 @@ export default function SelectWorkspaceClassComponent({
             searchPlaceholder="Select class"
             disableSearch={true}
             initialValue={selectedWsClass?.id}
-            disabled={workspaceClasses.length === 0 || loading || disabled}
+            disabled={workspaceClassesLoading || loading || disabled}
         >
-            <WorkspaceClassDropDownElementSelected wsClass={selectedWsClass} loading={loading} />
+            <WorkspaceClassDropDownElementSelected
+                wsClass={selectedWsClass}
+                loading={workspaceClassesLoading || loading}
+            />
         </Combobox>
     );
 }

diff --git a/components/dashboard/src/components/WorkspaceClassesOptions.tsx b/components/dashboard/src/components/WorkspaceClassesOptions.tsx
@@ -15,14 +15,23 @@ import { AllowedWorkspaceClass, DEFAULT_WS_CLASS } from "../data/workspaces/work
 import { MiddleDot } from "./typography/MiddleDot";
 import { useToast } from "./toasts/Toasts";
 import Modal, { ModalBaseFooter, ModalBody, ModalHeader } from "./Modal";
+import { LoadingState } from "@podkit/loading/LoadingState";
 
 interface WorkspaceClassesOptionsProps {
     classes: AllowedWorkspaceClass[];
     defaultClass?: string;
     className?: string;
+    emptyState?: React.ReactNode;
+    isLoading?: boolean;
 }
 
 export const WorkspaceClassesOptions = (props: WorkspaceClassesOptionsProps) => {
+    if (props.isLoading) {
+        return <LoadingState />;
+    }
+    if (props.classes.length === 0 && props.emptyState) {
+        return <>{props.emptyState}</>;
+    }
     return (
         <div className={cn("space-y-2", props.className)}>
             {props.classes.map((cls) => (
@@ -46,6 +55,7 @@ export const WorkspaceClassesOptions = (props: WorkspaceClassesOptionsProps) =>
 };
 
 export interface WorkspaceClassesModifyModalProps {
+    isLoading: boolean;
     defaultClass?: string;
     restrictedWorkspaceClasses: string[];
     showSetDefaultButton: boolean;
@@ -98,26 +108,30 @@ export const WorkspaceClassesModifyModal = ({
         <Modal visible onClose={onClose} onSubmit={handleUpdate}>
             <ModalHeader>Available workspace classes</ModalHeader>
             <ModalBody>
-                {allowedClasses.map((wsClass) => (
-                    <WorkspaceClassSwitch
-                        showSetDefaultButton={showSetDefaultButton}
-                        restrictedClasses={restrictedClasses}
-                        wsClass={wsClass}
-                        isDefault={defaultClass === wsClass.id}
-                        checked={!restrictedClasses.includes(wsClass.id)}
-                        onSetDefault={() => {
-                            setDefaultClass(wsClass.id);
-                        }}
-                        onCheckedChange={(checked) => {
-                            const newVal = !checked
-                                ? restrictedClasses.includes(wsClass.id)
-                                    ? [...restrictedClasses]
-                                    : [...restrictedClasses, wsClass.id]
-                                : restrictedClasses.filter((id) => id !== wsClass.id);
-                            setRestrictedClasses(newVal);
-                        }}
-                    />
-                ))}
+                {props.isLoading ? (
+                    <LoadingState />
+                ) : (
+                    allowedClasses.map((wsClass) => (
+                        <WorkspaceClassSwitch
+                            showSetDefaultButton={showSetDefaultButton}
+                            restrictedClasses={restrictedClasses}
+                            wsClass={wsClass}
+                            isDefault={defaultClass === wsClass.id}
+                            checked={!restrictedClasses.includes(wsClass.id)}
+                            onSetDefault={() => {
+                                setDefaultClass(wsClass.id);
+                            }}
+                            onCheckedChange={(checked) => {
+                                const newVal = !checked
+                                    ? restrictedClasses.includes(wsClass.id)
+                                        ? [...restrictedClasses]
+                                        : [...restrictedClasses, wsClass.id]
+                                    : restrictedClasses.filter((id) => id !== wsClass.id);
+                                setRestrictedClasses(newVal);
+                            }}
+                        />
+                    ))
+                )}
             </ModalBody>
             <ModalBaseFooter className="justify-between">
                 <div className="text-red-500">
@@ -127,7 +141,11 @@ export const WorkspaceClassesModifyModal = ({
                     <Button variant="secondary" onClick={onClose}>
                         Cancel
                     </Button>
-                    <LoadingButton disabled={!!computedError} type="submit" loading={updateMutation.isLoading}>
+                    <LoadingButton
+                        disabled={!!computedError || props.isLoading}
+                        type="submit"
+                        loading={updateMutation.isLoading}
+                    >
                         Save
                     </LoadingButton>
                 </div>

diff --git a/components/dashboard/src/data/workspaces/workspace-classes-query.ts b/components/dashboard/src/data/workspaces/workspace-classes-query.ts
@@ -116,12 +116,14 @@ export const useAllowedWorkspaceClassesMemo = (
     configurationId?: Configuration["id"],
     options?: { filterOutDisabled: boolean; ignoreScope?: DisableScope[] },
 ) => {
-    const { data: orgSettings } = useOrgSettingsQuery();
-    const { data: installationClasses } = useWorkspaceClasses();
+    const { data: orgSettings, isLoading: isLoadingOrgSettings } = useOrgSettingsQuery();
+    const { data: installationClasses, isLoading: isLoadingInstallationCls } = useWorkspaceClasses();
     // empty configurationId will return undefined
-    const { data: configuration } = useConfiguration(configurationId ?? "");
+    const { data: configuration, isLoading: isLoadingConfiguration } = useConfiguration(configurationId ?? "");
 
-    return useMemo(() => {
+    const isLoading = isLoadingOrgSettings || isLoadingInstallationCls || isLoadingConfiguration;
+
+    const data = useMemo(() => {
         return getAllowedWorkspaceClasses(
             installationClasses,
             orgSettings,
@@ -136,4 +138,5 @@ export const useAllowedWorkspaceClassesMemo = (
         configuration?.workspaceSettings?.restrictedWorkspaceClasses,
         configuration?.workspaceSettings?.workspaceClass,
     ]);
+    return { ...data, isLoading };
 };
diff --git a/...nts/dashboard/src/repositories/detail/workspaces/ConfigurationWorkspaceClassesOptions.tsx b/...nts/dashboard/src/repositories/detail/workspaces/ConfigurationWorkspaceClassesOptions.tsx
@@ -23,13 +23,19 @@ export const ConfigurationWorkspaceClassesOptions = ({ configuration }: { config
     const [showModal, setShowModal] = useState(false);
     const configurationMutation = useConfigurationMutation();
 
-    const { data: allowedClassesInConfiguration } = useAllowedWorkspaceClassesMemo(configuration.id, {
-        filterOutDisabled: true,
-    });
-    const { data: allowedClassesInOrganization } = useAllowedWorkspaceClassesMemo(undefined, {
-        filterOutDisabled: false,
-        ignoreScope: ["configuration"],
-    });
+    const { data: allowedClassesInConfiguration, isLoading: isLoadingInConfig } = useAllowedWorkspaceClassesMemo(
+        configuration.id,
+        {
+            filterOutDisabled: true,
+        },
+    );
+    const { data: allowedClassesInOrganization, isLoading: isLoadingInOrg } = useAllowedWorkspaceClassesMemo(
+        undefined,
+        {
+            filterOutDisabled: false,
+            ignoreScope: ["configuration"],
+        },
+    );
 
     const updateMutation: WorkspaceClassesModifyModalProps["updateMutation"] = useMutation({
         mutationFn: async ({ restrictedWorkspaceClasses, defaultClass }) => {
@@ -50,21 +56,22 @@ export const ConfigurationWorkspaceClassesOptions = ({ configuration }: { config
             <Subheading>Limit the available workspace classes for this repository.</Subheading>
 
             <div className="mt-4">
-                {allowedClassesInConfiguration.length === 0 ? (
-                    <div className="font-semibold text-pk-content-primary flex gap-2 items-center">
-                        <AlertTriangleIcon size={20} className="text-red-500" />
-                        <span>This repository doesn't have any available workspace classes.</span>
-                    </div>
-                ) : (
-                    <WorkspaceClassesOptions
-                        classes={allowedClassesInConfiguration}
-                        defaultClass={configuration.workspaceSettings?.workspaceClass}
-                    />
-                )}
+                <WorkspaceClassesOptions
+                    isLoading={isLoadingInConfig}
+                    emptyState={
+                        <div className="font-semibold text-pk-content-primary flex gap-2 items-center">
+                            <AlertTriangleIcon size={20} className="text-red-500" />
+                            <span>This repository doesn't have any available workspace classes.</span>
+                        </div>
+                    }
+                    classes={allowedClassesInConfiguration}
+                    defaultClass={configuration.workspaceSettings?.workspaceClass}
+                />
             </div>
 
             {showModal && (
                 <WorkspaceClassesModifyModal
+                    isLoading={isLoadingInOrg}
                     showSetDefaultButton
                     defaultClass={configuration.workspaceSettings?.workspaceClass}
                     restrictedWorkspaceClasses={configuration.workspaceSettings?.restrictedWorkspaceClasses ?? []}