cgra-dynamicmeasures - Improvements by github.com/no23reason

gooddata · Jan 10, 2022 · 17f133b · 17f133b
1 parent ec5bc67
commit 17f133b
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 109 deletions.
diff --git a/cgra-dynamicmeasures/README.md b/cgra-dynamicmeasures/README.md
@@ -8,16 +8,6 @@ This app adds new functionality to insights on a dashboard. Insights with multip
 * Measures bucket must be the first bucket
 * Insight must have 2 or more measures
 
-## 🦠 Bugs
-
-* When you double-click any checkbox REALLY fast, sometimes the insight renders two or even three times stack on each other. See the bug-01 screenshot below.
-
-![bug-01](public/bug-01.png)
-
-* When you double-click any checkbox REALLY fast, the insight will re-render and won't respect the state of the checkbox; in other words the measure will be displayed in the insight even when the checkbox is unchecked, and vice versa. See the bug-02 screenshot below.
-
-![bug-02](public/bug-02.png)
-
 ## Ideas for improvements
 
 * Activate the functionality based on insight title prefix

diff --git a/cgra-dynamicmeasures/src/components/MetricSwitcher.js b/cgra-dynamicmeasures/src/components/MetricSwitcher.js
@@ -0,0 +1,70 @@
+import React, { useMemo, useState } from "react";
+import { DefaultDashboardInsight } from "@gooddata/sdk-ui-dashboard";
+import { insightSetBuckets } from "@gooddata/sdk-model";
+
+import styles from "./MetricSwitcher.module.scss";
+
+const MetricSwitcher = props => {
+    const widgetId = props.widget.identifier;
+    const originalBuckets = props.insight.insight.buckets;
+    const originalMeasures = originalBuckets[0].items;
+    const [measures, setMeasures] = useState(originalMeasures);
+
+    // memoize the altered insight properly to improve performance and prevent weird race conditions
+    // this makes sure the underlying pluggable visualization is updated only when actually needed
+    const newInsight = useMemo(() => {
+        const newMeasuresBucket = {
+            ...originalBuckets[0],
+            items: measures,
+        };
+
+        return insightSetBuckets(props.insight, [
+            newMeasuresBucket,
+            ...originalBuckets.filter(b => b.localIdentifier !== "measures"),
+        ]);
+    }, [measures, props.insight, originalBuckets]);
+
+    return (
+        <div className={styles.CustomInsight}>
+            <div className={styles.MetricSwitcher}>
+                {originalMeasures.map(defaultMeasure => {
+                    const localIdentifier = defaultMeasure.measure.localIdentifier;
+                    const isChecked = !!measures.find(m => m.measure.localIdentifier === localIdentifier);
+
+                    return (
+                        <label
+                            htmlFor={`${widgetId}-${localIdentifier}`}
+                            // use key to prevent react warnings
+                            key={`${widgetId}-${localIdentifier}`}
+                        >
+                            <input
+                                id={`${widgetId}-${localIdentifier}`}
+                                value={localIdentifier}
+                                type="checkbox"
+                                defaultChecked={isChecked}
+                                disabled={measures.length === 1 && isChecked}
+                                onChange={e => {
+                                    if (!e.currentTarget.checked) {
+                                        setMeasures(
+                                            measures.filter(
+                                                m => m.measure.localIdentifier !== localIdentifier,
+                                            ),
+                                        );
+                                    } else {
+                                        setMeasures([...measures, defaultMeasure]);
+                                    }
+                                }}
+                            />
+                            {defaultMeasure.measure.title}
+                        </label>
+                    );
+                })}
+            </div>
+            <div className={styles.Insight}>
+                <DefaultDashboardInsight {...props} insight={newInsight} />
+            </div>
+        </div>
+    );
+};
+
+export default MetricSwitcher;
diff --git a/cgra-dynamicmeasures/src/components/MetricSwitcher.module.scss b/cgra-dynamicmeasures/src/components/MetricSwitcher.module.scss
@@ -0,0 +1,25 @@
+.CustomInsight {
+    // border: 1px solid red;
+    height: 100%;
+    display: flex;
+
+    .MetricSwitcher {
+        padding: 20px;
+        // border: 1px solid green;
+        width: 400px;
+
+        label {
+            display: block;
+            margin-bottom: 10px;
+
+            input {
+                margin-right: 10px;
+            }
+        }
+    }
+
+    .Insight {
+        // border: 1px solid blue;
+        flex: 1 0 auto;
+    }
+}
diff --git a/cgra-dynamicmeasures/src/routes/Home.js b/cgra-dynamicmeasures/src/routes/Home.js
@@ -1,85 +1,27 @@
-import React, { useState } from "react";
-import Snowfall from 'react-snowfall'
-import { Dashboard, DefaultDashboardInsight } from "@gooddata/sdk-ui-dashboard";
-import { idRef } from "@gooddata/sdk-model";
+import React from "react";
+import Snowfall from "react-snowfall";
+import { Dashboard } from "@gooddata/sdk-ui-dashboard";
+import { idRef, insightBuckets } from "@gooddata/sdk-model";
 
-import styles from "./Home.module.scss";
+import MetricSwitcher from "../components/MetricSwitcher";
 
 // const DASHBOARD_ID = "aaNEDetXTWPh"; // single insight
 const DASHBOARD_ID = "aagCCFA94QP5"; // multiple insights for testing
 
 const Home = () => {
-    const CustomInsight = (insight) => {
-        const widgetId = insight.widget.identifier;
-        const originalBuckets = insight.insight.insight.buckets;
-        const originalMeasures = originalBuckets[0].items;
-        const [measures, setMeasures] = useState(originalMeasures);
-
-        if (originalBuckets[0].localIdentifier !== 'measures'
-            || originalBuckets[0].items.length <= 1) {
-            return <DefaultDashboardInsight {...insight} />;
-        }
-
-        const newMeasuresBucket = {
-            ...originalBuckets[0],
-            items: measures
-        };
-        const newInsight = {
-            ...insight,
-            insight: {
-                ...insight.insight,
-                insight: {
-                    ...insight.insight.insight,
-                    buckets: [
-                        newMeasuresBucket,
-                        ...[...originalBuckets].splice(1)
-                    ]
-                }
-            }
-        };
-
-        return (
-            <div className={styles.CustomInsight}>
-                <div className={styles.MetricSwitcher}>
-                    {originalMeasures.map(defaultMeasure => {
-                        const localIdentifier = defaultMeasure.measure.localIdentifier;
-                        const isChecked = !!measures.find(m => m.measure.localIdentifier === localIdentifier);
-
-                        return (
-                            <label htmlFor={`${widgetId}-${localIdentifier}`}>
-                                <input
-                                    id={`${widgetId}-${localIdentifier}`}
-                                    value={localIdentifier}
-                                    type="checkbox"
-                                    defaultChecked={isChecked}
-                                    disabled={measures.length === 1 && isChecked}
-                                    onChange={e => {
-                                        if (!e.currentTarget.checked) {
-                                            setMeasures(measures.filter(m =>
-                                                m.measure.localIdentifier !== localIdentifier))
-                                        } else {
-                                            setMeasures([...measures, defaultMeasure]);
-                                        }
-                                    }}
-                                />
-                                {defaultMeasure.measure.title}
-                            </label>
-                        );
-                    })}
-                </div>
-                <div className={styles.Insight}>
-                    <DefaultDashboardInsight {...newInsight} />
-                </div>
-            </div>
-        );
-    };
-
     return (
         <>
-            <div className={styles.Dashboard}>
+            <div>
                 <Dashboard
                     dashboard={idRef(DASHBOARD_ID)}
-                    InsightComponentProvider={() => CustomInsight}
+                    InsightComponentProvider={insight => {
+                        const buckets = insightBuckets(insight);
+                        if (buckets[0].localIdentifier !== "measures" || buckets[0].items.length <= 1) {
+                            // fall back to whatever the default is
+                            return undefined;
+                        }
+                        return MetricSwitcher;
+                    }}
                 />
             </div>
             <Snowfall />

diff --git a/cgra-dynamicmeasures/src/routes/Home.module.scss b/cgra-dynamicmeasures/src/routes/Home.module.scss