Merge pull request #570 from dolthub/eric/usability-fixes

Usability fixes

Author: eric-richardson1

web/jest.config.js

@@ -10,6 +10,7 @@ const customJestConfig = {
   rootDir: "./renderer",
   setupFilesAfterEnv: ["../jest.setup.ts"],
   testRegex: TEST_REGEX,
+  testEnvironment: "jsdom",
   transform: {
     "^.+\\.tsx?$": "babel-jest",
   },

web/main/background.ts

@@ -252,13 +252,26 @@ ipcMain.handle("toggle-left-sidebar", () => {
 
 ipcMain.handle(
   "start-dolt-server",
-  async (_, connectionName: string, port: string, init?: boolean) => {
+  async (
+    _,
+    connectionName: string,
+    port: string,
+    databaseName: string,
+    init?: boolean,
+  ) => {
     try {
-      console.log("start-dolt-server", connectionName, port, init);
+      console.log(
+        "start-dolt-server",
+        connectionName,
+        port,
+        databaseName,
+        init,
+      );
       doltServerProcess = await startServer(
         mainWindow,
         connectionName,
         port,
+        databaseName,
         init,
       );
       if (!doltServerProcess) {

web/main/doltServer.ts

@@ -13,13 +13,17 @@ export async function startServer(
   mainWindow: BrowserWindow,
   connectionName: string,
   port: string,
+  databaseName: string,
   init?: boolean,
 ): Promise<ChildProcess | null> {
   // Set the path for the database folder
   // In production, it's in the userData directory
   // In development, it's in the build directory since the development userData directory clears its contents every time the app is rerun in dev mode
-  const dbFolderPath = path.join(getDatabasesPath(), connectionName);
-
+  const dbFolderPath = path.join(
+    getDatabasesPath(),
+    connectionName,
+    databaseName,
+  );
   const doltPath = getDoltPaths();
 
   try {

web/renderer/components/CloneDatabaseForm/CloneDetails.tsx

@@ -1,4 +1,3 @@
-import { Checkbox } from "@dolthub/react-components";
 import {
   DatabaseConnectionFragment,
   DatabaseType,
@@ -13,16 +12,10 @@ import css from "./index.module.css";
 import CloneForm from "./CloneForm";
 
 type Props = {
-  cloneDolt: boolean;
-  setCloneDolt: (c: boolean) => void;
   currentConnection: DatabaseConnectionFragment;
 };
 
-export default function CloneDetails({
-  cloneDolt,
-  setCloneDolt,
-  currentConnection,
-}: Props) {
+export default function CloneDetails({ currentConnection }: Props) {
   const { mutateFn: doltClone, ...res } = useMutation({
     hook: useDoltCloneMutation,
     refetchQueries: [
@@ -92,25 +85,14 @@ export default function CloneDetails({
 
   return (
     <div className={css.form}>
-      <Checkbox
-        checked={cloneDolt}
-        onChange={() => {
-          setCloneDolt(!cloneDolt);
-        }}
-        name="clone-dolt-server"
-        label="Clone a remote Dolt database from DoltHub"
-        className={css.checkbox}
+      <CloneForm
+        onCloneDoltHubDatabase={onCloneDoltHubDatabase}
+        progress={progress}
+        loading={loading}
+        error={err}
+        setErr={setErr}
+        disabledForConnection={false}
       />
-      {cloneDolt && (
-        <CloneForm
-          onCloneDoltHubDatabase={onCloneDoltHubDatabase}
-          progress={progress}
-          loading={loading}
-          error={err}
-          setErr={setErr}
-          disabledForConnection={false}
-        />
-      )}
     </div>
   );
 }

web/renderer/components/CloneDatabaseForm/CloneForm.tsx

@@ -40,6 +40,7 @@ export default function CloneForm({
     disabledForConnection,
     owner,
     remoteDbName,
+    newDbName,
     disabledForConnectionMessage,
   );
 
@@ -53,7 +54,8 @@ export default function CloneForm({
         }}
         label="Owner Name"
         labelClassName={css.label}
-        placeholder="e.g. dolthub"
+        placeholder="e.g. dolthub (required)"
+        data-cy="owner-name-input"
         light
       />
       <FormInput
@@ -66,6 +68,7 @@ export default function CloneForm({
         label="Remote Database Name"
         labelClassName={css.label}
         placeholder="e.g. my-database (required)"
+        data-cy="remote-db-name-input"
         light
       />
       <FormInput
@@ -77,6 +80,7 @@ export default function CloneForm({
         label="New Database Name"
         labelClassName={css.label}
         placeholder="e.g. my-database (required)"
+        data-cy="new-db-name-input"
         light
       />
       <ButtonsWithError error={error} className={css.buttons}>
@@ -108,6 +112,7 @@ export default function CloneForm({
                   onClick={async e =>
                     onCloneDoltHubDatabase(e, owner, remoteDbName, newDbName)
                   }
+                  data-cy="start-clone-button"
                 >
                   Start Clone
                 </Button>
@@ -126,14 +131,25 @@ export default function CloneForm({
 function getDisabled(
   disabledForConnection: boolean,
   owner: string,
-  database?: string,
+  remoteDatabaseName: string,
+  newDatabaseName: string,
   disabledForConnectionMessage?: ReactNode,
 ): DisabledReturnType {
   if (disabledForConnection) {
     return { disabled: true, message: disabledForConnectionMessage };
   }
-  if (!database) {
-    return { disabled: true, message: <span>Database name is required.</span> };
+  if (!remoteDatabaseName) {
+    return {
+      disabled: true,
+      message: <span>Remote database name is required.</span>,
+    };
+  }
+
+  if (!newDatabaseName) {
+    return {
+      disabled: true,
+      message: <span>New database name is required.</span>,
+    };
   }
   if (!owner) {
     return { disabled: true, message: <span>Owner name is required.</span> };

web/renderer/components/CloneDatabaseForm/index.module.css

@@ -1,4 +1,4 @@
-.form .checkbox {
+.form {
   svg {
     @apply -top-1;
   }

web/renderer/components/CloneDatabaseForm/index.test.tsx

@@ -17,19 +17,14 @@ jest.mock("next/router", () => {
 });
 
 describe("tests CloneDatabaseForm", () => {
-  const defaultProps = {
-    cloneDolt: false,
-    setCloneDolt: jest.fn(),
-  };
-
   beforeEach(() => {
     jest.clearAllMocks();
   });
 
   it("renders error when no current connection", async () => {
     setup(
       <MockedProvider mocks={[mocks.currentConnectionNullMock]}>
-        <CloneDatabaseForm {...defaultProps} />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -43,7 +38,7 @@ describe("tests CloneDatabaseForm", () => {
   it("renders nothing for postgres connections", async () => {
     const { container } = setup(
       <MockedProvider mocks={[mocks.postgresConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -52,64 +47,10 @@ describe("tests CloneDatabaseForm", () => {
     expect(container).toBeEmptyDOMElement();
   });
 
-  it("renders clone form when cloneDolt is false", async () => {
-    setup(
-      <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} />
-      </MockedProvider>,
-    );
-
-    await waitForQueryLoaders();
-
-    expect(
-      screen.getByText("Clone a remote Dolt database from DoltHub"),
-    ).toBeInTheDocument();
-    expect(screen.getByRole("checkbox")).not.toBeChecked();
-    expect(screen.queryByText("Owner Name")).not.toBeInTheDocument();
-  });
-
-  it("renders form fields when cloneDolt is true", async () => {
-    setup(
-      <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
-      </MockedProvider>,
-    );
-
-    await waitForQueryLoaders();
-
-    expect(
-      screen.getByText("Clone a remote Dolt database from DoltHub"),
-    ).toBeInTheDocument();
-    expect(screen.getByRole("checkbox")).toBeChecked();
-    expect(screen.getByText("Owner Name")).toBeInTheDocument();
-    expect(screen.getByText("Remote Database Name")).toBeInTheDocument();
-    expect(screen.getByText("New Database Name")).toBeInTheDocument();
-  });
-
-  it("toggles checkbox and calls setCloneDolt", async () => {
-    const mockSetCloneDolt = jest.fn();
-    const { user } = setup(
-      <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm
-          {...defaultProps}
-          setCloneDolt={mockSetCloneDolt}
-          cloneDolt={false}
-        />
-      </MockedProvider>,
-    );
-
-    await waitForQueryLoaders();
-
-    const checkbox = screen.getByRole("checkbox");
-    await user.click(checkbox);
-
-    expect(mockSetCloneDolt).toHaveBeenCalledWith(true);
-  });
-
   it("shows disabled submit button when fields are empty", async () => {
     setup(
       <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -122,7 +63,7 @@ describe("tests CloneDatabaseForm", () => {
   it("enables submit button when all fields are filled", async () => {
     const { user } = setup(
       <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -142,7 +83,7 @@ describe("tests CloneDatabaseForm", () => {
   it("auto-fills new database name when remote database name changes", async () => {
     const { user } = setup(
       <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -162,7 +103,7 @@ describe("tests CloneDatabaseForm", () => {
       <MockedProvider
         mocks={[mocks.currentConnectionMock, mocks.doltCloneMock]}
       >
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -194,7 +135,7 @@ describe("tests CloneDatabaseForm", () => {
   it("shows tooltip when button is disabled", async () => {
     const { user } = setup(
       <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 
@@ -207,15 +148,15 @@ describe("tests CloneDatabaseForm", () => {
 
     await waitFor(() => {
       expect(
-        screen.getByText("Database name is required."),
+        screen.getByText("Remote database name is required."),
       ).toBeInTheDocument();
     });
   });
 
   it("shows owner required tooltip when only database is filled", async () => {
     const { user } = setup(
       <MockedProvider mocks={[mocks.currentConnectionMock]}>
-        <CloneDatabaseForm {...defaultProps} cloneDolt />
+        <CloneDatabaseForm />
       </MockedProvider>,
     );
 

web/renderer/components/CloneDatabaseForm/index.tsx

@@ -2,20 +2,15 @@ import { ErrorMsg, QueryHandler } from "@dolthub/react-components";
 import { useCurrentConnectionQuery } from "@gen/graphql-types";
 import CloneDetails from "./CloneDetails";
 
-type Props = {
-  cloneDolt: boolean;
-  setCloneDolt: (c: boolean) => void;
-};
-
-export default function CloneDatabaseForm(props: Props) {
+export default function CloneDatabaseForm() {
   const res = useCurrentConnectionQuery();
 
   return (
     <QueryHandler
       result={res}
       render={data =>
         data.currentConnection ? (
-          <CloneDetails {...props} currentConnection={data.currentConnection} />
+          <CloneDetails currentConnection={data.currentConnection} />
         ) : (
           <ErrorMsg errString="Could not find current connection." />
         )

web/renderer/components/CreateDatabase/index.tsx

@@ -55,6 +55,7 @@ export default function CreateDatabase(props: Props) {
       <Button.Link
         onClick={() => setIsOpen(true)}
         className={cx(css.createDB, props.buttonClassName)}
+        data-cy="add-database-button"
       >
         {props.showText ? (
           <span className={css.text}>Create Database</span>

web/renderer/components/CreateDatabaseOrSchemaModal/index.module.css

@@ -0,0 +1,3 @@
+.radios {
+  @apply mb-6;
+}