Skip to content

Commit

Permalink
Merge pull request #214 from mrc-ide/mrc-5443
Browse files Browse the repository at this point in the history 
mrc-5443 Copy variable on drag if Ctrl key is pressed
  • Loading branch information
@EmmaLRussell
EmmaLRussell authored Aug 16, 2024
2 parents ce43d29 + fcbdd63 commit e9bb88c
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 17 deletions.
3 changes: 2 additions & 1 deletion app/static/src/app/components/graphConfig/GraphConfig.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +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.
Drag variables here to select them for this graph. Press the Ctrl or ⌘ key on drag to make a copy of a
variable.
</div>
</div>
</div>
Expand Down
22 changes: 18 additions & 4 deletions app/static/src/app/components/mixins/selectVariables.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ export default (
): SelectVariablesMixin => {
const thisSrcGraphConfig = hasHiddenVariables ? "hidden" : graphIndex!.toString();

const startDrag = (evt: DragEvent, variable: string) => {
const { dataTransfer } = evt;
const startDrag = (event: DragEvent, variable: string) => {
const { dataTransfer, ctrlKey, metaKey } = event;
const copy = !hasHiddenVariables && (ctrlKey || metaKey);
dataTransfer!.dropEffect = "move";
dataTransfer!.effectAllowed = "move";
dataTransfer!.setData("variable", variable);
dataTransfer!.setData("srcGraphConfig", thisSrcGraphConfig);
dataTransfer!.setData("copyVar", copy.toString());

emit("setDragging", true);
};
Expand Down Expand Up @@ -53,14 +55,26 @@ export default (
const variable = dataTransfer!.getData("variable");
const srcGraphConfig = dataTransfer!.getData("srcGraphConfig");
if (srcGraphConfig !== thisSrcGraphConfig) {
const copy = !hasHiddenVariables && dataTransfer!.getData("copyVar") === "true";

// add to this graph if necessary - do this before remove so, if a linked variable, it is not unlinked
if (!hasHiddenVariables && !selectedVariables.value.includes(variable)) {
const newVars = [...selectedVariables.value, variable];
updateSelectedVariables(graphIndex!, newVars);
}

if (srcGraphConfig !== "hidden") {
removeVariable(parseInt(srcGraphConfig, 10), variable);
if (srcGraphConfig !== "hidden" && !copy) {
// 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);
}
}
}
};
Expand Down
16 changes: 14 additions & 2 deletions app/static/tests/e2e/code.etest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -326,12 +326,24 @@ test.describe("Code Tab tests", () => {

// Drag variable to second graph
const sVariable = await page.locator(":nth-match(.graph-config-panel .variable, 1)");
await sVariable.dragTo(page.locator(":nth-match(.graph-config-panel .drop-zone, 2)"));
const dragPositions = {
sourcePosition: { x: 5, y: 5 },
targetPosition: { x: 5, y: 5 }
};
await sVariable.dragTo(page.locator(":nth-match(.graph-config-panel .drop-zone, 2)"), dragPositions);

await expectGraphVariables(page, 0, ["I", "R"]);
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)"), dragPositions);
await page.keyboard.up("Control");
await expectGraphVariables(page, 0, ["I", "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);
Expand Down
52 changes: 44 additions & 8 deletions app/static/tests/unit/components/graphConfig/graphConfig.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,26 @@ 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 } });
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(wrapper.emitted("setDragging")![0]).toStrictEqual([true]);
await s.trigger("dragstart", { dataTransfer: { setData }, ctrlKey: false, metaKey: false });
expect(setData).toHaveBeenNthCalledWith(1, "variable", "S");
expect(setData).toHaveBeenNthCalledWith(2, "srcGraphConfig", "0");
expect(setData).toHaveBeenNthCalledWith(3, "copyVar", "false");
});

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, metaKey: false });
expect(setData).toHaveBeenNthCalledWith(3, "copyVar", "true");
});

it("start drag sets values copyVar to true in event when meta 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: false, metaKey: true });
expect(setData).toHaveBeenNthCalledWith(3, "copyVar", "true");
});

it("ending drag emits setDragging", async () => {
Expand All @@ -114,6 +128,7 @@ describe("GraphConfig", () => {
getData: (s: string) => {
if (s === "variable") return "I";
if (s === "srcGraphConfig") return "1";
if (s === "copyVar") return "false";
return null;
}
};
Expand All @@ -127,12 +142,32 @@ describe("GraphConfig", () => {
expect(mockUpdateSelectedVariables.mock.calls[1][1]).toStrictEqual({ graphIndex: 1, selectedVariables: ["J"] });
});

it("onDrop does not attempt to remove variable if source was hidden variables", async () => {
it("onDrop does not attempt to remove variable if source is hidden, even with Ctrl key", async () => {
const wrapper = getWrapper();
const dataTransfer = {
getData: (s: string) => {
if (s === "variable") return "I";
if (s === "srcGraphConfig") return "hidden";
if (s === "copyVar") return "true";
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;
}
};
Expand Down Expand Up @@ -167,7 +202,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 or ⌘ key on drag to make a copy of a variable."
);
});

Expand Down
11 changes: 9 additions & 2 deletions app/static/tests/unit/components/graphConfig/hiddenVariables.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ describe("HiddenVariables", () => {
config: [
{
selectedVariables: ["S", "J"]
},
{
selectedVariables: ["S"]
}
]
},
Expand Down Expand Up @@ -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]);
});

Expand All @@ -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 () => {
Expand Down

0 comments on commit e9bb88c

Please sign in to comment.