Fix Dialogue form #308

Author: joepio

CHANGELOG.md

@@ -12,6 +12,7 @@ This changelog covers all three packages, as they are (for now) updated as a who
 - Add new file preview UI for images, audio, text and PDF files.
 - Add new file preview types to the folder grid view.
 - Add `store.preloadClassesAndProperties` and remove `urls.properties.getAll` and `urls.classes.getAll`. This enables using `atomic-data-browser` without relying on `atomicdata.dev` being available.
+- Fix Dialogue form #308
 
 ## v0.35.0
 

…r/src/components/forms/NewForm/index.tsx …mponents/forms/NewForm/NewFormDialog.tsxdata-browser/src/components/forms/NewForm/index.tsx renamed to data-browser/src/components/forms/NewForm/NewFormDialog.tsx data-browser/src/components/forms/NewForm/index.tsx renamed to data-browser/src/components/forms/NewForm/NewFormDialog.tsx

@@ -1,61 +1,24 @@
 import { properties, useResource, useStore, useTitle } from '@tomic/react';
-import React, { useCallback, useState } from 'react';
-import { useQueryString } from '../../../helpers/navigation';
+import React, { useState, useCallback } from 'react';
 import { useEffectOnce } from '../../../hooks/useEffectOnce';
 import { Button } from '../../Button';
-import { DialogActions, DialogContent, DialogTitle } from '../../Dialog';
+import { DialogTitle, DialogContent, DialogActions } from '../../Dialog';
 import { ErrorLook } from '../../ErrorLook';
 import { useSaveResource } from '../hooks/useSaveResource';
 import { InlineErrMessage } from '../InputStyles';
 import { ResourceForm, ResourceFormVariant } from '../ResourceForm';
+import { NewFormProps } from './NewFormPage';
 import { NewFormTitle, NewFormTitleVariant } from './NewFormTitle';
 import { SubjectField } from './SubjectField';
 import { useNewForm } from './useNewForm';
 
-export interface NewFormProps {
-  classSubject: string;
-}
-
 export interface NewFormDialogProps extends NewFormProps {
   closeDialog: () => void;
   initialTitle: string;
   onSave: (subject: string) => void;
   parent: string;
 }
 
-/** Fullpage Form for instantiating a new Resource from some Class */
-export const NewFormFullPage = ({
-  classSubject,
-}: NewFormProps): JSX.Element => {
-  const klass = useResource(classSubject);
-  const [subject, setSubject] = useQueryString('newSubject');
-  const [parentSubject] = useQueryString('parent');
-
-  const { subjectErr, subjectValue, setSubjectValue, resource } = useNewForm(
-    klass,
-    subject,
-    setSubject,
-    parentSubject,
-  );
-
-  return (
-    <>
-      <NewFormTitle classSubject={classSubject} />
-      <SubjectField
-        error={subjectErr}
-        value={subjectValue}
-        onChange={setSubjectValue}
-      />
-      {/* Key is required for re-rendering when subject changes */}
-      <ResourceForm
-        resource={resource}
-        classSubject={classSubject}
-        key={`${classSubject}+${subject}`}
-      />
-    </>
-  );
-};
-
 /** Form for instantiating a new Resource from some Class in a Modal / Dialog view */
 export const NewFormDialog = ({
   classSubject,
@@ -72,12 +35,12 @@ export const NewFormDialog = ({
 
   const [subject, setSubject] = useState(store.createSubject());
 
-  const { subjectErr, subjectValue, setSubjectValue, resource } = useNewForm(
+  const { subjectErr, subjectValue, setSubjectValue, resource } = useNewForm({
     klass,
-    subject,
     setSubject,
+    initialSubject: subject,
     parent,
-  );
+  });
 
   const onResourceSave = useCallback(() => {
     onSave(resource.getSubject());

data-browser/src/components/forms/NewForm/NewFormPage.tsx

@@ -0,0 +1,44 @@
+import { useResource } from '@tomic/react';
+import { useQueryString } from '../../../helpers/navigation';
+import { ResourceForm } from '../ResourceForm';
+import { NewFormTitle } from './NewFormTitle';
+import { SubjectField } from './SubjectField';
+import { useNewForm } from './useNewForm';
+import React from 'react';
+
+export interface NewFormProps {
+  classSubject: string;
+}
+
+/** Fullpage Form for instantiating a new Resource from some Class */
+export const NewFormFullPage = ({
+  classSubject,
+}: NewFormProps): JSX.Element => {
+  const klass = useResource(classSubject);
+  const [subject, setSubject] = useQueryString('newSubject');
+  const [parentSubject] = useQueryString('parent');
+
+  const { subjectErr, subjectValue, setSubjectValue, resource } = useNewForm({
+    klass,
+    setSubject,
+    initialSubject: subject,
+    parent: parentSubject,
+  });
+
+  return (
+    <>
+      <NewFormTitle classSubject={classSubject} />
+      <SubjectField
+        error={subjectErr}
+        value={subjectValue}
+        onChange={setSubjectValue}
+      />
+      {/* Key is required for re-rendering when subject changes */}
+      <ResourceForm
+        resource={resource}
+        classSubject={classSubject}
+        key={`${classSubject}+${subject}`}
+      />
+    </>
+  );
+};

data-browser/src/components/forms/NewForm/useNewForm.ts

@@ -6,66 +6,63 @@ import {
   useResource,
   useArray,
 } from '@tomic/react';
-import { useState, useEffect, useDeferredValue } from 'react';
+import { useState, useEffect } from 'react';
 
 const resourseOpts = { newResource: true };
 
+type UseNewForm = {
+  klass: Resource;
+  setSubject: (v: string) => void;
+  initialSubject?: string;
+  parent?: string;
+};
+
 /** Shared logic for NewForm components. */
-export const useNewForm = (
-  klass: Resource,
-  subject: string | undefined,
-  setSubject: (v: string) => void,
-  parent?: string,
-) => {
+export const useNewForm = (args: UseNewForm) => {
+  const { klass, setSubject, initialSubject, parent } = args;
+
   const store = useStore();
-  // TODO: Don't push to history, but replace, because currenlty back is broken
   const [klassShortname] = useString(klass, properties.shortname);
-  const [subjectValue, setSubjectValue] = useState<string>(() => {
-    if (subject === undefined) {
+  const [subjectValue, setSubjectValueInternal] = useState<string>(() => {
+    if (initialSubject === undefined) {
       return store.createSubject(klassShortname);
     }
 
-    return subject;
+    return initialSubject;
   });
 
   const [subjectErr, setSubjectErr] = useState<Error | undefined>(undefined);
   const resource = useResource(subjectValue, resourseOpts);
-  const [_, setParent] = useString(resource, properties.parent);
+  const [parentVal, setParent] = useString(resource, properties.parent);
+  const [isAVal, setIsA] = useArray(resource, properties.isA);
 
+  // When the resource is created or updated, make sure that the parent and class are present
   useEffect(() => {
-    setParent(parent);
-  }, [parent]);
-
-  // Set the class for new resources
-  const [currentClass] = useArray(resource, properties.isA);
-
-  if (currentClass.length === 0) {
-    resource.set(properties.isA, [klass.getSubject()], store);
-  }
+    if (parentVal !== parent) {
+      setParent(parent);
+    }
 
-  const defferedSubjectValue = useDeferredValue(subjectValue);
-  /** Changes the URL of a subject. Updates the store */
-  // Should be debounced as it is quite expensive, but getting that to work turned out to be really hard
-  useEffect(() => {
-    if (!defferedSubjectValue) {
-      return;
+    if (isAVal.length === 0) {
+      setIsA([klass.getSubject()]);
     }
+  }, [resource, parent]);
 
-    const oldSubject = resource.getSubject();
+  async function setSubjectValue(newSubject: string) {
+    setSubjectValueInternal(newSubject);
+    setSubjectErr(undefined);
+    setSubject(newSubject);
 
-    if (oldSubject === defferedSubjectValue) {
+    if (resource.get(properties.parent) !== parent) {
+      // This prevents that we move an empty temporary resource
       return;
     }
 
-    setSubjectErr(undefined);
-    // Expensive!
-    store
-      .renameSubject(oldSubject, defferedSubjectValue)
-      .then(() => {
-        setSubject(defferedSubjectValue);
-      })
-      .catch(e => setSubjectErr(e));
-  }, [defferedSubjectValue, resource]);
+    try {
+      await store.renameSubject(resource, newSubject);
+    } catch (e) {
+      setSubjectErr(e);
+    }
+  }
 
   return {
     subjectErr,

data-browser/src/components/forms/ResourceSelector.tsx

@@ -17,9 +17,9 @@ import { ErrMessage, InputWrapper } from './InputStyles';
 import { DropdownInput } from './DropdownInput';
 import { Dialog, useDialog } from '../Dialog';
 import { DialogTreeContext } from '../Dialog/dialogContext';
-import { NewFormDialog } from './NewForm';
 import { useSettings } from '../../helpers/AppSettings';
 import styled from 'styled-components';
+import { NewFormDialog } from './NewForm/NewFormDialog';
 
 interface ResourceSelectorProps {
   /**

data-browser/src/routes/NewRoute.tsx

@@ -13,12 +13,12 @@ import { ResourceSelector } from '../components/forms/ResourceSelector';
 import { Button } from '../components/Button';
 import { useSettings } from '../helpers/AppSettings';
 import { Row } from '../components/Row';
-import { NewFormFullPage } from '../components/forms/NewForm/index';
 import { ResourceInline } from '../views/ResourceInline';
 import styled from 'styled-components';
 import { FileDropzoneInput } from '../components/forms/FileDropzone/FileDropzoneInput';
 import toast from 'react-hot-toast';
 import { getIconForClass } from '../views/FolderPage/iconMap';
+import { NewFormFullPage } from '../components/forms/NewForm/NewFormPage';
 
 /** Start page for instantiating a new Resource from some Class */
 function New(): JSX.Element {

data-browser/tests/e2e.spec.ts

@@ -556,6 +556,13 @@ test.describe('data-browser', async () => {
     await newDrive(page);
     // Create new class from new resource menu
     await newResource('class', page);
+
+    await fillInput('shortname', page);
+    await fillInput('description', page);
+    await page.click('[data-test="save"]');
+    // go back browser history
+    await page.goBack();
+
     await page
       .locator('[title="Add an item to this list"] >> nth=0')
       .first()
@@ -580,6 +587,7 @@ test.describe('data-browser', async () => {
       .fill('This is a test prop');
     await page.locator('dialog footer >> text=Save').click();
 
+    await page.locator('text=Resource Saved');
     expect(
       await page.locator(
         '[data-test="input-recommends"] >> nth=0 >> "test-prop"',
@@ -710,3 +718,18 @@ async function editTitle(title: string, page: Page) {
 async function clickSidebarItem(text: string, page: Page) {
   await page.click(`[data-test="sidebar"] >> text="${text}"`);
 }
+
+async function fillInput(
+  propertyShortname: string,
+  page: Page,
+  value?: string,
+) {
+  let locator = `[data-test="input-${propertyShortname}"]`;
+
+  if (propertyShortname === 'description') {
+    locator = 'textarea[name="yamdeContent"]';
+  }
+
+  await page.click(locator);
+  await page.fill(locator, value || `test-${propertyShortname}`);
+}

lib/src/store.ts

@@ -494,22 +494,19 @@ export class Store {
    * errors if so.
    */
   public async renameSubject(
-    oldSubject: string,
+    resource: Resource,
     newSubject: string,
   ): Promise<void> {
     Client.tryValidSubject(newSubject);
-    const old = this.resources.get(oldSubject);
-
-    if (old === undefined) {
-      throw Error(`Old subject does not exist in store: ${oldSubject}`);
-    }
+    const oldSubject = resource.getSubject();
 
     if (await this.checkSubjectTaken(newSubject)) {
       throw Error(`New subject name is already taken: ${newSubject}`);
     }
 
-    old.setSubject(newSubject);
-    this.resources.set(newSubject, old);
+    resource.setSubject(newSubject);
+    this.addResources(resource);
+    this.resources.set(newSubject, resource);
     this.removeResource(oldSubject);
   }