From 24ad841d7f27338398d58484d13ed4ffb57a5117 Mon Sep 17 00:00:00 2001 From: gabrielburnworth Date: Mon, 30 Mar 2020 17:57:32 -0700 Subject: [PATCH] add and edit tool improvements --- .../farm_designer/farm_designer_panels.scss | 14 +++++ .../tools/__tests__/add_tool_test.tsx | 54 ++++++++++++++++--- .../tools/__tests__/edit_tool_test.tsx | 18 +++++++ .../tools/__tests__/index_test.tsx | 4 +- frontend/farm_designer/tools/add_tool.tsx | 32 ++++++++--- frontend/farm_designer/tools/edit_tool.tsx | 34 ++++++++---- frontend/farm_designer/tools/index.tsx | 1 + frontend/farm_designer/tools/interfaces.ts | 2 + frontend/ui/save_button.tsx | 4 +- 9 files changed, 136 insertions(+), 27 deletions(-) diff --git a/frontend/css/farm_designer/farm_designer_panels.scss b/frontend/css/farm_designer/farm_designer_panels.scss index 997266800..a86266bde 100644 --- a/frontend/css/farm_designer/farm_designer_panels.scss +++ b/frontend/css/farm_designer/farm_designer_panels.scss @@ -650,6 +650,7 @@ margin-top: 1rem; &.red { float: left; + margin-bottom: 1rem; } } svg { @@ -659,6 +660,19 @@ height: 10rem; margin-top: 2rem; } + .edit-tool, + .add-new-tool { + margin-bottom: 3rem; + .name-error { + margin-top: 1.2rem; + margin-right: 1rem; + color: $dark_red; + float: right; + } + .save-btn { + float: right; + } + } .add-stock-tools { .filter-search { margin-bottom: 1rem; diff --git a/frontend/farm_designer/tools/__tests__/add_tool_test.tsx b/frontend/farm_designer/tools/__tests__/add_tool_test.tsx index e171a6af5..6451ce41a 100644 --- a/frontend/farm_designer/tools/__tests__/add_tool_test.tsx +++ b/frontend/farm_designer/tools/__tests__/add_tool_test.tsx @@ -1,4 +1,10 @@ -jest.mock("../../../api/crud", () => ({ initSave: jest.fn() })); +let mockSave = () => Promise.resolve(); +jest.mock("../../../api/crud", () => ({ + initSave: jest.fn(), + init: jest.fn(() => ({ payload: { uuid: "fake uuid" } })), + save: jest.fn(() => mockSave), + destroy: jest.fn(), +})); jest.mock("../../../history", () => ({ history: { push: jest.fn() } })); @@ -7,7 +13,7 @@ import { mount, shallow } from "enzyme"; import { RawAddTool as AddTool, mapStateToProps } from "../add_tool"; import { fakeState } from "../../../__test_support__/fake_state"; import { SaveBtn } from "../../../ui"; -import { initSave } from "../../../api/crud"; +import { initSave, init, destroy } from "../../../api/crud"; import { history } from "../../../history"; import { FirmwareHardware } from "farmbot"; import { AddToolProps } from "../interfaces"; @@ -32,11 +38,47 @@ describe("", () => { expect(wrapper.state().toolName).toEqual("new name"); }); - it("saves", () => { - const wrapper = shallow(); + it("disables save until name in entered", () => { + const wrapper = shallow(); + expect(wrapper.state().toolName).toEqual(""); + expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy(); + wrapper.setState({ toolName: "fake tool name" }); + expect(wrapper.find("SaveBtn").first().props().disabled).toBeFalsy(); + }); + + it("shows name collision message", () => { + const p = fakeProps(); + p.existingToolNames = ["tool"]; + const wrapper = shallow(); + wrapper.setState({ toolName: "tool" }); + expect(wrapper.find("p").first().text()).toEqual("Already added."); + expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy(); + }); + + it("saves", async () => { + mockSave = () => Promise.resolve(); + const p = fakeProps(); + p.dispatch = jest.fn(x => typeof x === "function" && x()); + const wrapper = shallow(); wrapper.setState({ toolName: "Foo" }); - wrapper.find(SaveBtn).simulate("click"); - expect(initSave).toHaveBeenCalledWith("Tool", { name: "Foo" }); + await wrapper.find(SaveBtn).simulate("click"); + expect(init).toHaveBeenCalledWith("Tool", { name: "Foo" }); + expect(wrapper.state().uuid).toEqual(undefined); + expect(history.push).toHaveBeenCalledWith("/app/designer/tools"); + }); + + it("removes unsaved tool on exit", async () => { + mockSave = () => Promise.reject(); + const p = fakeProps(); + p.dispatch = jest.fn(x => typeof x === "function" && x()); + const wrapper = shallow(); + wrapper.setState({ toolName: "Foo" }); + await wrapper.find(SaveBtn).simulate("click"); + expect(init).toHaveBeenCalledWith("Tool", { name: "Foo" }); + expect(wrapper.state().uuid).toEqual("fake uuid"); + expect(history.push).not.toHaveBeenCalled(); + wrapper.unmount(); + expect(destroy).toHaveBeenCalledWith("fake uuid"); }); it.each<[FirmwareHardware, number]>([ diff --git a/frontend/farm_designer/tools/__tests__/edit_tool_test.tsx b/frontend/farm_designer/tools/__tests__/edit_tool_test.tsx index a0b1a7a0f..faedd1aab 100644 --- a/frontend/farm_designer/tools/__tests__/edit_tool_test.tsx +++ b/frontend/farm_designer/tools/__tests__/edit_tool_test.tsx @@ -38,6 +38,7 @@ describe("", () => { dispatch: jest.fn(), mountedToolId: undefined, isActive: jest.fn(), + existingToolNames: [], }); it("renders", () => { @@ -71,6 +72,23 @@ describe("", () => { expect(wrapper.state().toolName).toEqual("new name"); }); + it("disables save until name in entered", () => { + const wrapper = shallow(); + wrapper.setState({ toolName: "" }); + expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy(); + wrapper.setState({ toolName: "fake tool name" }); + expect(wrapper.find("SaveBtn").first().props().disabled).toBeFalsy(); + }); + + it("shows name collision message", () => { + const p = fakeProps(); + p.existingToolNames = ["tool"]; + const wrapper = shallow(); + wrapper.setState({ toolName: "tool" }); + expect(wrapper.find("p").first().text()).toEqual("Name already taken."); + expect(wrapper.find("SaveBtn").first().props().disabled).toBeTruthy(); + }); + it("saves", () => { const wrapper = shallow(); wrapper.find(SaveBtn).simulate("click"); diff --git a/frontend/farm_designer/tools/__tests__/index_test.tsx b/frontend/farm_designer/tools/__tests__/index_test.tsx index cbb0c3516..13362abed 100644 --- a/frontend/farm_designer/tools/__tests__/index_test.tsx +++ b/frontend/farm_designer/tools/__tests__/index_test.tsx @@ -188,7 +188,9 @@ describe("", () => { it("displays tool as active", () => { const p = fakeProps(); - p.tools = [fakeTool()]; + const tool = fakeTool(); + tool.body.id = 1; + p.tools = [tool]; p.isActive = () => true; p.device.body.mounted_tool_id = undefined; const wrapper = mount(); diff --git a/frontend/farm_designer/tools/add_tool.tsx b/frontend/farm_designer/tools/add_tool.tsx index 86a1b5975..fadc57efd 100644 --- a/frontend/farm_designer/tools/add_tool.tsx +++ b/frontend/farm_designer/tools/add_tool.tsx @@ -7,7 +7,7 @@ import { Everything } from "../../interfaces"; import { t } from "../../i18next_wrapper"; import { SaveBtn } from "../../ui"; import { SpecialStatus } from "farmbot"; -import { initSave } from "../../api/crud"; +import { initSave, destroy, init, save } from "../../api/crud"; import { Panel } from "../panel_header"; import { history } from "../../history"; import { selectAllTools } from "../../resources/selectors"; @@ -27,7 +27,7 @@ export const mapStateToProps = (props: Everything): AddToolProps => ({ }); export class RawAddTool extends React.Component { - state: AddToolState = { toolName: "", toAdd: [] }; + state: AddToolState = { toolName: "", toAdd: [], uuid: undefined }; filterExisting = (n: string) => !this.props.existingToolNames.includes(n); @@ -41,15 +41,23 @@ export class RawAddTool extends React.Component { toAdd: this.stockToolNames().filter(this.filterExisting) }); - newTool = (name: string) => { - this.props.dispatch(initSave("Tool", { name })); - }; + newTool = (name: string) => this.props.dispatch(initSave("Tool", { name })); save = () => { - this.newTool(this.state.toolName); - history.push("/app/designer/tools"); + const initTool = init("Tool", { name: this.state.toolName }); + this.props.dispatch(initTool); + const { uuid } = initTool.payload; + this.setState({ uuid }); + this.props.dispatch(save(uuid)) + .then(() => { + this.setState({ uuid: undefined }); + history.push("/app/designer/tools"); + }).catch(() => { }); } + componentWillUnmount = () => + this.state.uuid && this.props.dispatch(destroy(this.state.uuid)); + stockToolNames = () => { switch (this.props.firmwareHardware) { case "arduino": @@ -123,6 +131,8 @@ export class RawAddTool extends React.Component { } render() { + const { toolName, uuid } = this.state; + const alreadyAdded = !uuid && !this.filterExisting(toolName); const panelName = "add-tool"; return { name="name" onChange={e => this.setState({ toolName: e.currentTarget.value })} /> - + +

+ {alreadyAdded ? t("Already added.") : ""} +

diff --git a/frontend/farm_designer/tools/edit_tool.tsx b/frontend/farm_designer/tools/edit_tool.tsx index 9264a0048..9e4108e54 100644 --- a/frontend/farm_designer/tools/edit_tool.tsx +++ b/frontend/farm_designer/tools/edit_tool.tsx @@ -9,6 +9,7 @@ import { getPathArray } from "../../history"; import { TaggedTool, SpecialStatus, TaggedToolSlotPointer } from "farmbot"; import { maybeFindToolById, getDeviceAccountSettings, selectAllToolSlotPointers, + selectAllTools, } from "../../resources/selectors"; import { SaveBtn } from "../../ui"; import { edit, destroy } from "../../api/crud"; @@ -17,6 +18,7 @@ import { Panel } from "../panel_header"; import { ToolSVG } from "../map/layers/tool_slots/tool_graphics"; import { error } from "../../toast/toast"; import { EditToolProps, EditToolState } from "./interfaces"; +import { betterCompact } from "../../util"; export const isActive = (toolSlots: TaggedToolSlotPointer[]) => (toolId: number | undefined) => @@ -29,6 +31,8 @@ export const mapStateToProps = (props: Everything): EditToolProps => ({ mountedToolId: getDeviceAccountSettings(props.resources.index) .body.mounted_tool_id, isActive: isActive(selectAllToolSlotPointers(props.resources.index)), + existingToolNames: betterCompact(selectAllTools(props.resources.index) + .map(tool => tool.body.name)), }); export class RawEditTool extends React.Component { @@ -53,18 +57,26 @@ export class RawEditTool extends React.Component { ? t("Cannot delete while mounted.") : t("Cannot delete while in a slot."); const activeOrMounted = this.props.isActive(tool.body.id) || isMounted; + const nameTaken = this.props.existingToolNames + .filter(x => x != tool.body.name).includes(this.state.toolName); return - - - this.setState({ toolName: e.currentTarget.value })} /> - { - dispatch(edit(tool, { name: toolName })); - history.push("/app/designer/tools"); - }} - status={SpecialStatus.DIRTY} /> +
+ + + this.setState({ toolName: e.currentTarget.value })} /> + { + this.props.dispatch(edit(tool, { name: toolName })); + history.push("/app/designer/tools"); + }} + disabled={!this.state.toolName || nameTaken} + status={SpecialStatus.DIRTY} /> +

+ {nameTaken ? t("Name already taken.") : ""} +

+
;