test

mrc-ide · Aug 3, 2024 · 9bfb077 · 9bfb077
1 parent 3825997
commit 9bfb077
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 10 deletions.
diff --git a/app/static/src/app/components/graphConfig/GraphConfig.vue b/app/static/src/app/components/graphConfig/GraphConfig.vue
@@ -26,8 +26,8 @@
                 </span>
             </template>
             <div v-if="!selectedVariables.length" class="drop-zone-instruction p-2 me-4">
-                Drag variables here to select them for this graph.
-                Press the Ctrl key on drag to make a copy of a variable.
+                Drag variables here to select them for this graph. Press the Ctrl key on drag to make a copy of a
+                variable.
             </div>
         </div>
     </div>

diff --git a/app/static/src/app/components/mixins/selectVariables.ts b/app/static/src/app/components/mixins/selectVariables.ts
@@ -19,8 +19,8 @@ export default (
     const thisSrcGraphConfig = hasHiddenVariables ? "hidden" : graphIndex!.toString();
 
     const startDrag = (evt: DragEvent, variable: string) => {
-        const { dataTransfer } = evt;
-        const copy = !hasHiddenVariables && evt.ctrlKey;
+        const { dataTransfer, ctrlKey } = evt;
+        const copy = !hasHiddenVariables && ctrlKey;
         dataTransfer!.dropEffect = "move";
         dataTransfer!.effectAllowed = "move";
         dataTransfer!.setData("variable", variable);
@@ -64,7 +64,17 @@ export default (
             }
 
             if (srcGraphConfig !== "hidden" && !copy) {
-                removeVariable(parseInt(srcGraphConfig, 10), variable);
+                // Remove variable from all graphs where it occurs if this is HiddenVariables, otherwise from source
+                // graph only
+                if (hasHiddenVariables) {
+                    store.state.graphs.config.forEach((config, index) => {
+                        if (config.selectedVariables.includes(variable)) {
+                            removeVariable(index, variable);
+                        }
+                    });
+                } else {
+                    removeVariable(parseInt(srcGraphConfig, 10), variable);
+                }
             }
         }
     };

diff --git a/app/static/tests/e2e/code.etest.ts b/app/static/tests/e2e/code.etest.ts
@@ -312,7 +312,7 @@ test.describe("Code Tab tests", () => {
         await expectGraphVariables(page, 0, ["I", "R"]);
     });
 
-    test("can add a graph, drag a variable onto it and delete it", async ({ page }) => {
+    test("can add a graph, drag variables onto it and delete it", async ({ page }) => {
         // Add graph
         await page.click("#add-graph-btn");
 
@@ -332,6 +332,14 @@ test.describe("Code Tab tests", () => {
         await expectGraphVariables(page, 1, ["S"]);
         await expect(page.locator(".hidden-variables-panel .variable")).toHaveCount(0);
 
+        // Drag a variable with Ctrl key to make copy
+        const iVariable = await page.locator(":nth-match(.graph-config-panel .variable, 1)");
+        await page.keyboard.down("Control");
+        await iVariable.dragTo(page.locator(":nth-match(.graph-config-panel .drop-zone, 2)"));
+        await page.keyboard.up("Control");
+        await expectGraphVariables(page, 0, ["R"]);
+        await expectGraphVariables(page, 1, ["S", "I"]);
+
         // Delete second graph
         await page.click(":nth-match(.graph-config-panel .delete-graph, 2)");
         await expect(page.locator(".graph-config-panel")).toHaveCount(1);

diff --git a/app/static/tests/unit/components/graphConfig/graphConfig.test.ts b/app/static/tests/unit/components/graphConfig/graphConfig.test.ts
@@ -93,14 +93,25 @@ describe("GraphConfig", () => {
         const wrapper = getWrapper();
         const s = wrapper.findAll(".graph-config-panel .badge").at(0)!;
         const setData = jest.fn();
-        await s.trigger("dragstart", { dataTransfer: { setData } });
+        await s.trigger("dragstart", { dataTransfer: { setData }, ctrlKey: false });
         expect(setData.mock.calls[0][0]).toBe("variable");
         expect(setData.mock.calls[0][1]).toStrictEqual("S");
         expect(setData.mock.calls[1][0]).toStrictEqual("srcGraphConfig");
         expect(setData.mock.calls[1][1]).toStrictEqual("0");
+        expect(setData.mock.calls[2][0]).toBe("copyVar");
+        expect(setData.mock.calls[2][1]).toBe("false");
         expect(wrapper.emitted("setDragging")![0]).toStrictEqual([true]);
     });
 
+    it("start drag sets values copyVar to true in event when Ctrl key pressed", async () => {
+        const wrapper = getWrapper();
+        const s = wrapper.findAll(".graph-config-panel .badge").at(0)!;
+        const setData = jest.fn();
+        await s.trigger("dragstart", { dataTransfer: { setData }, ctrlKey: true });
+        expect(setData.mock.calls[2][0]).toBe("copyVar");
+        expect(setData.mock.calls[2][1]).toBe("true");
+    });
+
     it("ending drag emits setDragging", async () => {
         const wrapper = getWrapper();
         const s = wrapper.findAll(".graph-config-panel .badge").at(0)!;
@@ -114,6 +125,7 @@ describe("GraphConfig", () => {
             getData: (s: string) => {
                 if (s === "variable") return "I";
                 if (s === "srcGraphConfig") return "1";
+                if (s === "copyVar") return "false";
                 return null;
             }
         };
@@ -133,6 +145,26 @@ describe("GraphConfig", () => {
             getData: (s: string) => {
                 if (s === "variable") return "I";
                 if (s === "srcGraphConfig") return "hidden";
+                if (s === "copyVar") return "false";
+                return null;
+            }
+        };
+        const dropPanel = wrapper.find(".graph-config-panel");
+        await dropPanel.trigger("drop", { dataTransfer });
+        expect(mockUpdateSelectedVariables.mock.calls.length).toBe(1);
+        expect(mockUpdateSelectedVariables.mock.calls[0][1]).toStrictEqual({
+            graphIndex: 0,
+            selectedVariables: ["S", "R", "I"]
+        });
+    });
+
+    it("onDrop does not remove variable if copyVar is true", async () => {
+        const wrapper = getWrapper();
+        const dataTransfer = {
+            getData: (s: string) => {
+                if (s === "variable") return "I";
+                if (s === "srcGraphConfig") return "1";
+                if (s === "copyVar") return "true";
                 return null;
             }
         };
@@ -167,7 +199,8 @@ describe("GraphConfig", () => {
             ]
         } as any);
         expect(wrapper.find(".drop-zone-instruction").text()).toBe(
-            "Drag variables here to select them for this graph."
+            "Drag variables here to select them for this graph. " +
+                "Press the Ctrl key on drag to make a copy of a variable."
         );
     });
 

diff --git a/app/static/tests/unit/components/graphConfig/hiddenVariables.test.ts b/app/static/tests/unit/components/graphConfig/hiddenVariables.test.ts
@@ -33,6 +33,9 @@ describe("HiddenVariables", () => {
                         config: [
                             {
                                 selectedVariables: ["S", "J"]
+                            },
+                            {
+                                selectedVariables: ["S"]
                             }
                         ]
                     },
@@ -88,6 +91,8 @@ describe("HiddenVariables", () => {
         expect(setData.mock.calls[0][1]).toStrictEqual("I");
         expect(setData.mock.calls[1][0]).toStrictEqual("srcGraphConfig");
         expect(setData.mock.calls[1][1]).toStrictEqual("hidden");
+        expect(setData.mock.calls[2][0]).toStrictEqual("copyVar");
+        expect(setData.mock.calls[2][1]).toStrictEqual("false");
         expect(wrapper.emitted("setDragging")![0]).toStrictEqual([true]);
     });
 
@@ -98,19 +103,21 @@ describe("HiddenVariables", () => {
         expect(wrapper.emitted("setDragging")![0]).toStrictEqual([false]);
     });
 
-    it("onDrop removes variable from source", async () => {
+    it("onDrop removes variable from all configs with variable", async () => {
         const wrapper = getWrapper();
         const dataTransfer = {
             getData: (s: string) => {
                 if (s === "variable") return "S";
                 if (s === "srcGraphConfig") return "0";
+                if (s === "copyVar") return "false";
                 return null;
             }
         };
         const dropPanel = wrapper.find(".hidden-variables-panel");
         await dropPanel.trigger("drop", { dataTransfer });
-        expect(mockUpdateSelectedVariables.mock.calls.length).toBe(1);
+        expect(mockUpdateSelectedVariables.mock.calls.length).toBe(2);
         expect(mockUpdateSelectedVariables.mock.calls[0][1]).toStrictEqual({ graphIndex: 0, selectedVariables: ["J"] });
+        expect(mockUpdateSelectedVariables.mock.calls[1][1]).toStrictEqual({ graphIndex: 1, selectedVariables: [] });
     });
 
     it("shows drop zone when dragging", async () => {