[layout] Enhance ticket #160

- fix boolean value by using the defaultValue instead of the checked attribute (see facebook/react#3005) - notify an error if some params are wrong
gephi · Jun 18, 2024 · 4a4e5fe · 4a4e5fe
1 parent 9c41081
commit 4a4e5fe
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 21 deletions.
diff --git a/src/components/forms/TypedInputs.tsx b/src/components/forms/TypedInputs.tsx
@@ -1,9 +1,9 @@
 import cx from "classnames";
-import { clamp, isNil } from "lodash";
+import { clamp } from "lodash";
 import Slider from "rc-slider";
 import { MarkObj } from "rc-slider/lib/Marks";
 import { SliderProps } from "rc-slider/lib/Slider";
-import React, { FC, InputHTMLAttributes, ReactNode, useEffect, useMemo, useState } from "react";
+import React, { FC, InputHTMLAttributes, ReactNode, useMemo } from "react";
 import Select from "react-select";
 
 import MessageTooltip from "../MessageTooltip";
@@ -129,11 +129,6 @@ export const BooleanInput: FC<
   { value: boolean | null; onChange: (v: boolean) => void } & BaseTypedInputProps &
     Omit<InputHTMLAttributes<HTMLInputElement>, "value" | "onChange" | "id">
 > = ({ id, label, description, value, onChange, className, ...attrs }) => {
-  const [isChecked, setIsChecked] = useState<boolean>(isNil(value) ? false : value);
-  useEffect(() => {
-    setIsChecked(isNil(value) ? false : value);
-  }, [value]);
-
   return (
     <>
       <div className="form-check mt-1">
@@ -142,7 +137,7 @@ export const BooleanInput: FC<
           type="checkbox"
           className={cx("form-check-input", className)}
           id={id}
-          checked={isChecked}
+          defaultChecked={!!value}
           onChange={(e) => onChange(e.target.checked)}
         />
         <label htmlFor={id} className="form-check-label small ms-1">

diff --git a/src/core/metrics/index.ts b/src/core/metrics/index.ts
@@ -3,7 +3,8 @@ import { dataGraphToFullGraph, inferFieldType } from "../graph/utils";
 import { Metric, MetricReport } from "./types";
 
 /**
- * computeMetric: compute a metric a mutate the graphdataset state directly for better performance
+ * Compute a metric and mutate the graphdataset state directly for better performance.
+ *
  * @param metric metric object to apply
  * @param params metric params from metric form
  * @param attributeNames attribute⋅s where the result will be stored

diff --git a/src/views/graphPage/LayoutsPanel.tsx b/src/views/graphPage/LayoutsPanel.tsx
@@ -19,6 +19,7 @@ import { getFilteredDataGraph } from "../../core/graph/utils";
 import { LAYOUTS } from "../../core/layouts/collection";
 import { Layout, LayoutScriptParameter } from "../../core/layouts/types";
 import { useModal } from "../../core/modals";
+import { useNotifications } from "../../core/notifications";
 import { sessionAtom } from "../../core/session";
 import { useAtom } from "../../core/utils/atoms";
 import { FunctionEditorModal } from "./modals/FunctionEditorModal";
@@ -48,7 +49,6 @@ export const LayoutForm: FC<{
     () => session.layoutsParameters[layout.id] || {},
     [layout.id, session.layoutsParameters],
   );
-
   // default layout parameters
   const layoutDefaultParameters = useMemo(
     () =>
@@ -102,7 +102,7 @@ export const LayoutForm: FC<{
    * Reset parameters for the current layout
    */
   const setParameters = useCallback(
-    (newParameters?: { [layout: string]: Record<string, unknown> }) => {
+    (newParameters?: Record<string, unknown>) => {
       setSession((prev) => ({
         ...prev,
         layoutsParameters: {
@@ -130,7 +130,7 @@ export const LayoutForm: FC<{
         if (layout.type === "sync")
           setSuccessMessage(t("layouts.exec.success", { layout: t(`layouts.${layout.id}.title`) as string }) as string);
       } catch (e) {
-        // nothing todo
+        console.error(e);
       }
     }
   }, [isRunning, layout.id, layout.type, layoutParameters, onStart, onStop, setSuccessMessage, t]);
@@ -149,6 +149,7 @@ export const LayoutForm: FC<{
         {layout.description && <p className="text-muted small">{t(`layouts.${layout.id}.description`)}</p>}
 
         {layout.parameters.map((param) => {
+          const value = layoutParameters[param.id];
           const id = `layouts-${layout.id}-params-${param.id})}`;
           return (
             <div className="my-1" key={id}>
@@ -161,7 +162,7 @@ export const LayoutForm: FC<{
                       ? (t(`layouts.${layout.id}.parameters.${param.id}.description`) as string)
                       : undefined
                   }
-                  value={layoutParameters[param.id] as number}
+                  value={value as number}
                   disabled={isRunning}
                   onChange={(v) => onChangeParameters(param.id, v)}
                   required={param.required || false}
@@ -179,7 +180,7 @@ export const LayoutForm: FC<{
                       ? (t(`layouts.${layout.id}.parameters.${param.id}.description`) as string)
                       : undefined
                   }
-                  value={!!layoutParameters[param.id] as boolean}
+                  value={!!value as boolean}
                   disabled={isRunning}
                   onChange={(v) => onChangeParameters(param.id, v)}
                   required={param.required || false}
@@ -196,7 +197,7 @@ export const LayoutForm: FC<{
                       : undefined
                   }
                   placeholder={t("common.none") as string}
-                  value={layoutParameters[param.id] as string}
+                  value={value as string}
                   disabled={isRunning}
                   onChange={(v) => onChangeParameters(param.id, v)}
                   options={((param.itemType === "nodes" ? nodeFields : edgeFields) as FieldModel[])
@@ -210,17 +211,17 @@ export const LayoutForm: FC<{
               {param.type === "script" && (
                 <div className="position-relative">
                   <>
-                    {layoutParameters[param.id] && (
+                    {value && (
                       <>
                         <div className="code-thumb mt-1" style={{ height: "auto", maxHeight: "auto" }}>
                           <Highlight className="javascript">
-                            {(layoutParameters[param.id] as LayoutScriptParameter["defaultValue"]).toString()}
+                            {(value as LayoutScriptParameter["defaultValue"]).toString()}
                           </Highlight>
                         </div>
                         <div className="filler-fade-out position-absolute bottom-0"></div>
                       </>
                     )}
-                    <div className={cx(layoutParameters[param.id] ? "bottom-0 position-absolute w-100" : "")}>
+                    <div className={cx(value ? "bottom-0 position-absolute w-100" : "")}>
                       <button
                         type="button"
                         className="btn btn-dark mx-auto d-block m-3"
@@ -232,7 +233,7 @@ export const LayoutForm: FC<{
                               withSaveAndRun: true,
                               functionJsDoc: param.functionJsDoc,
                               defaultFunction: param.defaultValue,
-                              value: layoutParameters[param.id] as LayoutScriptParameter["defaultValue"],
+                              value: value as LayoutScriptParameter["defaultValue"],
                               checkFunction: param.functionCheck,
                             },
                             beforeSubmit: ({ run, script }) => {
@@ -318,6 +319,7 @@ export const LayoutForm: FC<{
 
 export const LayoutsPanel: FC = () => {
   const { t } = useTranslation();
+  const { notify } = useNotifications();
   const { startLayout, stopLayout } = useLayoutActions();
   const { type } = useLayoutState();
 
@@ -361,8 +363,12 @@ export const LayoutsPanel: FC = () => {
           <LayoutForm
             key={option.layout.id}
             layout={option.layout}
-            onStart={(params) => {
-              startLayout(option.layout.id, params);
+            onStart={async (params) => {
+              try {
+                await startLayout(option.layout.id, params);
+              } catch (e) {
+                notify({ type: "error", message: (e as Error).message });
+              }
             }}
             onStop={() => {
               stopLayout();