From a709eeae5da4da23aa4407e1fba496f52076956e Mon Sep 17 00:00:00 2001
From: Ruben Thoms <69145689+rubenthoms@users.noreply.github.com>
Date: Mon, 11 Sep 2023 17:23:11 +0200
Subject: [PATCH] Fixed/improved multiple components (#299)

---
 .../selectEnsemblesDialog.tsx                 |  1 +
 .../Settings/private-components/setting.tsx   | 27 +++---
 .../private-components/syncSettings.tsx       | 97 ++++++++++---------
 .../src/lib/components/Checkbox/checkbox.tsx  |  2 +-
 .../src/lib/components/Dropdown/dropdown.tsx  |  9 +-
 .../lib/components/RadioGroup/radioGroup.tsx  | 16 +--
 .../private-components/tag.tsx                | 14 ++-
 .../SmartNodeSelector/smartNodeSelector.tsx   |  2 +-
 .../src/lib/hooks/useElementBoundingRect.ts   |  2 +
 9 files changed, 90 insertions(+), 80 deletions(-)

diff --git a/frontend/src/framework/internal/components/SelectEnsemblesDialog/selectEnsemblesDialog.tsx b/frontend/src/framework/internal/components/SelectEnsemblesDialog/selectEnsemblesDialog.tsx
index b53b73022..f3be24941 100644
--- a/frontend/src/framework/internal/components/SelectEnsemblesDialog/selectEnsemblesDialog.tsx
+++ b/frontend/src/framework/internal/components/SelectEnsemblesDialog/selectEnsemblesDialog.tsx
@@ -174,6 +174,7 @@ export const SelectEnsemblesDialog: React.FC<SelectEnsemblesDialogProps> = (prop
                         </Button>
                     </div>
                 }
+                showCloseCross
             >
                 <div className="flex gap-4 max-w-full">
                     <div className="flex flex-col gap-4 p-4 border-r bg-slate-100 h-full">
diff --git a/frontend/src/framework/internal/components/Settings/private-components/setting.tsx b/frontend/src/framework/internal/components/Settings/private-components/setting.tsx
index f01ab1765..1fe3f9530 100644
--- a/frontend/src/framework/internal/components/Settings/private-components/setting.tsx
+++ b/frontend/src/framework/internal/components/Settings/private-components/setting.tsx
@@ -7,6 +7,7 @@ import { ErrorBoundary } from "@framework/internal/components/ErrorBoundary";
 import { useImportState } from "@framework/internal/hooks/moduleHooks";
 import { Cog6ToothIcon } from "@heroicons/react/20/solid";
 import { CircularProgress } from "@lib/components/CircularProgress";
+import { resolveClassNames } from "@lib/utils/resolveClassNames";
 
 type SettingProps = {
     moduleInstance: ModuleInstance<any>;
@@ -71,10 +72,10 @@ export const Setting: React.FC<SettingProps> = (props) => {
     return (
         <div
             key={props.moduleInstance.getId()}
-            style={{
-                display: props.activeModuleId === props.moduleInstance.getId() ? "flex" : "none",
-            }}
-            className="flex-col"
+            className={resolveClassNames(
+                props.activeModuleId === props.moduleInstance.getId() ? "flex" : "hidden",
+                "flex-col h-full w-full"
+            )}
         >
             <ErrorBoundary moduleInstance={props.moduleInstance}>
                 <div className="flex justify-center items-center p-2 bg-slate-100 h-10">
@@ -86,14 +87,16 @@ export const Setting: React.FC<SettingProps> = (props) => {
                         {props.moduleInstance.getTitle()}
                     </span>
                 </div>
-                <div className="p-2 flex flex-col gap-4">
-                    <Settings
-                        moduleContext={props.moduleInstance.getContext()}
-                        workbenchSession={props.workbench.getWorkbenchSession()}
-                        workbenchServices={props.workbench.getWorkbenchServices()}
-                        workbenchSettings={props.workbench.getWorkbenchSettings()}
-                        initialSettings={props.moduleInstance.getInitialSettings() || undefined}
-                    />
+                <div className="flex flex-col gap-4 overflow-auto">
+                    <div className="p-2">
+                        <Settings
+                            moduleContext={props.moduleInstance.getContext()}
+                            workbenchSession={props.workbench.getWorkbenchSession()}
+                            workbenchServices={props.workbench.getWorkbenchServices()}
+                            workbenchSettings={props.workbench.getWorkbenchSettings()}
+                            initialSettings={props.moduleInstance.getInitialSettings() || undefined}
+                        />
+                    </div>
                 </div>
             </ErrorBoundary>
         </div>
diff --git a/frontend/src/framework/internal/components/Settings/private-components/syncSettings.tsx b/frontend/src/framework/internal/components/Settings/private-components/syncSettings.tsx
index 2f7c7d272..908b875c6 100644
--- a/frontend/src/framework/internal/components/Settings/private-components/syncSettings.tsx
+++ b/frontend/src/framework/internal/components/Settings/private-components/syncSettings.tsx
@@ -69,54 +69,59 @@ export const SyncSettings: React.FC<ModulesListProps> = (props) => {
         return true;
     }
 
+    function makeContent() {
+        const syncableSettingKeys = activeModuleInstance?.getModule().getSyncableSettingKeys() ?? [];
+
+        if (activeModuleId === "" || activeModuleInstance === undefined) {
+            return <div className="text-gray-500">No module selected</div>;
+        }
+
+        if (syncableSettingKeys.length === 0) {
+            return <div className="text-gray-500">No syncable settings</div>;
+        }
+
+        return (
+            <table className="w-full">
+                <thead>
+                    <tr className="border-b">
+                        <th className="border-r p-2 w-6">
+                            <GlobeAltIcon className="w-4 h-4" title="Sync for all module instances" />
+                        </th>
+                        <th className="border-r p-2 w-6">
+                            <MapPinIcon className="w-4 h-4" title="Sync for active module instance" />
+                        </th>
+                        <th></th>
+                    </tr>
+                </thead>
+                <tbody>
+                    {syncableSettingKeys.map((setting) => {
+                        const globallySynced = isGlobalSyncSetting(setting);
+                        return (
+                            <tr key={setting} className="hover:bg-blue-50">
+                                <td className="border-r p-2">
+                                    <Checkbox
+                                        checked={globallySynced}
+                                        onChange={(e) => handleGlobalSyncSettingChange(setting, e.target.checked)}
+                                    />
+                                </td>
+                                <td className="border-r p-2">
+                                    <Checkbox
+                                        checked={globallySynced || activeModuleInstance.isSyncedSetting(setting)}
+                                        onChange={(e) => handleSyncSettingChange(setting, e.target.checked)}
+                                    />
+                                </td>
+                                <td className="p-2">{SyncSettingsMeta[setting].name}</td>
+                            </tr>
+                        );
+                    })}
+                </tbody>
+            </table>
+        );
+    }
+
     return (
         <Drawer title="Sync settings" icon={<LinkIcon />} visible={drawerContent === DrawerContent.SyncSettings}>
-            {activeModuleId === "" || activeModuleInstance === undefined ? (
-                <div className="text-gray-500">No module selected</div>
-            ) : (
-                <table className="w-full">
-                    <thead>
-                        <tr className="border-b">
-                            <th className="border-r p-2 w-6">
-                                <GlobeAltIcon className="w-4 h-4" title="Sync for all module instances" />
-                            </th>
-                            <th className="border-r p-2 w-6">
-                                <MapPinIcon className="w-4 h-4" title="Sync for active module instance" />
-                            </th>
-                            <th></th>
-                        </tr>
-                    </thead>
-                    <tbody>
-                        {activeModuleInstance
-                            .getModule()
-                            .getSyncableSettingKeys()
-                            .map((setting) => {
-                                const globallySynced = isGlobalSyncSetting(setting);
-                                return (
-                                    <tr key={setting} className="hover:bg-blue-50">
-                                        <td className="border-r p-2">
-                                            <Checkbox
-                                                checked={globallySynced}
-                                                onChange={(e) =>
-                                                    handleGlobalSyncSettingChange(setting, e.target.checked)
-                                                }
-                                            />
-                                        </td>
-                                        <td className="border-r p-2">
-                                            <Checkbox
-                                                checked={
-                                                    globallySynced || activeModuleInstance.isSyncedSetting(setting)
-                                                }
-                                                onChange={(e) => handleSyncSettingChange(setting, e.target.checked)}
-                                            />
-                                        </td>
-                                        <td className="p-2">{SyncSettingsMeta[setting].name}</td>
-                                    </tr>
-                                );
-                            })}
-                    </tbody>
-                </table>
-            )}
+            {makeContent()}
         </Drawer>
     );
 };
diff --git a/frontend/src/lib/components/Checkbox/checkbox.tsx b/frontend/src/lib/components/Checkbox/checkbox.tsx
index d3a639cc9..f2d1649a9 100644
--- a/frontend/src/lib/components/Checkbox/checkbox.tsx
+++ b/frontend/src/lib/components/Checkbox/checkbox.tsx
@@ -50,7 +50,7 @@ export const Checkbox: React.FC<CheckboxProps> = (props) => {
                 {props.label && (
                     <label
                         htmlFor={props.id ?? id.current}
-                        className={resolveClassNames("block", "text-sm", "text-gray-900", "cursor-pointer")}
+                        className={resolveClassNames("block", "text-gray-900", "cursor-pointer")}
                     >
                         {props.label}
                     </label>
diff --git a/frontend/src/lib/components/Dropdown/dropdown.tsx b/frontend/src/lib/components/Dropdown/dropdown.tsx
index 6cdb4dfe1..870f38617 100644
--- a/frontend/src/lib/components/Dropdown/dropdown.tsx
+++ b/frontend/src/lib/components/Dropdown/dropdown.tsx
@@ -166,14 +166,7 @@ export const Dropdown = withDefaults<DropdownProps>()(defaultProps, (props) => {
                 setOptionIndexWithFocusToCurrentSelection();
             }
         }
-    }, [
-        inputBoundingRect,
-        dropdownVisible,
-        filteredOptions,
-        selection,
-        setOptionIndexWithFocusToCurrentSelection,
-        setStartIndex,
-    ]);
+    }, [inputBoundingRect, dropdownVisible, filteredOptions, selection]);
 
     const handleOptionClick = React.useCallback(
         (value: string) => {
diff --git a/frontend/src/lib/components/RadioGroup/radioGroup.tsx b/frontend/src/lib/components/RadioGroup/radioGroup.tsx
index 1c5f78562..cc262e619 100644
--- a/frontend/src/lib/components/RadioGroup/radioGroup.tsx
+++ b/frontend/src/lib/components/RadioGroup/radioGroup.tsx
@@ -35,32 +35,32 @@ type RadioProps = {
 const Radio: React.FC<RadioProps> = (props) => {
     return (
         <BaseComponent disabled={props.disabled}>
-            <label className={resolveClassNames("relative", "cursor-pointer", "inline-flex", "align-middle", "gap-2")}>
+            <label className="relative inline-flex align-middle gap-2 items-center group">
                 <span
                     className={resolveClassNames(
                         "rounded-full",
-                        "w-5",
-                        "h-5",
+                        "w-4 max-w-4 min-w-[1rem]",
+                        "h-4 max-h-4 min-h-[1rem]",
                         "border-2",
                         "border-solid",
                         "flex",
                         "items-center",
                         "justify-center",
-                        props.checked ? "border-blue-500" : "border-gray-400"
+                        props.checked ? "border-blue-500" : "border-gray-400 group-hover:border-blue-500"
                     )}
                 >
                     <span
                         className={resolveClassNames(
                             "rounded-full",
-                            props.checked ? "w-3" : "w-0",
-                            props.checked ? "h-3" : "h-0",
+                            props.checked ? "w-2" : "w-0",
+                            props.checked ? "h-2" : "h-0",
                             "bg-blue-500",
                             "block",
                             "transition-all"
                         )}
                     />
                     <input
-                        className="opacity-0 absolute w-full h-full cursor-inherit top-0 left-0 m-0 p-0 z-1"
+                        className="opacity-0 absolute w-full h-full cursor-inherit top-0 left-0 m-0 p-0 z-1 cursor-pointer"
                         type="radio"
                         name={props.name}
                         value={props.value}
@@ -86,7 +86,7 @@ export const RadioGroup = withDefaults<RadioGroupProps>()(defaultProps, (props)
             >
                 <span>{props.name}</span>
                 <div
-                    className={resolveClassNames("flex", "radio-group", "gap-4", "m-1", {
+                    className={resolveClassNames("flex", "radio-group", "gap-1", {
                         "flex-col": props.direction === "vertical",
                     })}
                 >
diff --git a/frontend/src/lib/components/SmartNodeSelector/private-components/tag.tsx b/frontend/src/lib/components/SmartNodeSelector/private-components/tag.tsx
index 6e3bfd241..f9734183c 100644
--- a/frontend/src/lib/components/SmartNodeSelector/private-components/tag.tsx
+++ b/frontend/src/lib/components/SmartNodeSelector/private-components/tag.tsx
@@ -155,10 +155,13 @@ export class Tag extends React.Component<TagProps> {
                 position = 0;
             }
             return (
-                <div key={"TagBrowseButton_" + index} className="w-4 mr-1 relative h-full">
+                <div key={"TagBrowseButton_" + index} className="w-4 mr-1 h-full flex flex-col">
                     <button
                         key={"TagPreviousButton_" + index}
-                        className="appearance-none bg-cyan-600 border-0 cursor-pointer inline-block outline-none p-0 h-1/2 absolute w-4 disabled:opacity-30 disabled:cursor-default hover:bg-cyan-500"
+                        className={resolveClassNames(
+                            "appearance-none bg-cyan-600 border-0 cursor-pointer inline-block outline-none p-0 m-0 h-1/2 w-4 disabled:opacity-30 disabled:cursor-default",
+                            { "hover:bg-cyan-500": position !== 0 }
+                        )}
                         disabled={position === 0}
                         title="Previous option"
                         onMouseDown={(e): void => this.shiftOption(e, nodeSelection, false)}
@@ -175,8 +178,11 @@ export class Tag extends React.Component<TagProps> {
                     </button>
                     <button
                         key={"TagNextButton_" + index}
-                        className="appearance-none bg-cyan-600 border-0 cursor-pointer inline-block outline-none p-0 h-1/2 absolute top-1/2 w-4 disabled:opacity-30 disabled:cursor-default hover:bg-cyan-500"
-                        disabled={position == subgroups.length - 1}
+                        className={resolveClassNames(
+                            "appearance-none bg-cyan-600 border-0 cursor-pointer inline-block outline-none p-0 m-0 h-1/2 w-4 disabled:opacity-30 disabled:cursor-default",
+                            { "hover:bg-cyan-500": position !== subgroups.length - 1 }
+                        )}
+                        disabled={position === subgroups.length - 1}
                         title="Next option"
                         onMouseDown={(e): void => this.shiftOption(e, nodeSelection, true)}
                         onMouseUp={(e): void => {
diff --git a/frontend/src/lib/components/SmartNodeSelector/smartNodeSelector.tsx b/frontend/src/lib/components/SmartNodeSelector/smartNodeSelector.tsx
index 5badecab4..f9f967fd3 100644
--- a/frontend/src/lib/components/SmartNodeSelector/smartNodeSelector.tsx
+++ b/frontend/src/lib/components/SmartNodeSelector/smartNodeSelector.tsx
@@ -1672,7 +1672,7 @@ export class SmartNodeSelectorComponent extends React.Component<SmartNodeSelecto
                     </ul>
                     <div className="absolute right-2 top-1/2 -mt-3">
                         <button
-                            className="appearance-none bg-cyan-600 rounded-full w-6 h-6 flex items-center justify-center hover:bg-cyan-500 text-white cursor-pointer"
+                            className="appearance-none bg-cyan-600 rounded-full w-6 h-6 flex items-center justify-center hover:bg-cyan-500 text-white cursor-pointer disabled:hidden"
                             type="button"
                             title="Clear all"
                             onClick={this.clearAllTags}
diff --git a/frontend/src/lib/hooks/useElementBoundingRect.ts b/frontend/src/lib/hooks/useElementBoundingRect.ts
index fa3604654..916e13b60 100644
--- a/frontend/src/lib/hooks/useElementBoundingRect.ts
+++ b/frontend/src/lib/hooks/useElementBoundingRect.ts
@@ -13,6 +13,7 @@ export function useElementBoundingRect(ref: React.RefObject<HTMLElement>): DOMRe
 
         const resizeObserver = new ResizeObserver(handleResize);
         window.addEventListener("resize", handleResize);
+        window.addEventListener("scroll", handleResize, true);
 
         if (ref.current) {
             handleResize();
@@ -22,6 +23,7 @@ export function useElementBoundingRect(ref: React.RefObject<HTMLElement>): DOMRe
         return () => {
             resizeObserver.disconnect();
             window.removeEventListener("resize", handleResize);
+            window.removeEventListener("scroll", handleResize, true);
         };
     }, [ref]);