SenseNet / sn-client

Monorepo for sensenet client packages 🐱‍💻
https://sensenet.com
GNU General Public License v2.0
24 stars 37 forks source link

[dms-demo] Can only create new content in the previous location that I was in #91

Closed LuisAverhoff closed 11 months ago

LuisAverhoff commented 5 years ago

Bug

Can only create new content in the previous location that I was in.

Steps to Reproduce

1.) go http://sn-dms-demo-dev.netlify.com and login 2.) Double click on the folder called sample folder. 3.) click on the document menu that is on the left hand side to open it up. 4.) click on the new button and create any new content.

Expected Results

The new content should be created in Document_Library/Sample-Folder

Actual Results

The new content is created in Document_Library

Reason

The reason this happens is because as it's adding all the new actions to the list, the current path that it gives to the AddNewDialog component is the path of the old parent(Document_Library) and not the path of the new parent(Sample-Folder). Once all actions have been placed in the actions list and is ready to be rendered does it finally load the new parent but the problem is that the AddNewDialog component is still using the old path from the old parent and will always have the wrong path after each subsequent update.

Here is the function where all of this occurs. This function is in src/components/ActionMenu/AddNewMenu.tsx

public static getDerivedStateFromProps(newProps: AddNewMenu['props'], lastState: AddNewMenu['state']) {
    if (
      newProps.currentContent &&
      newProps.currentContent.Id &&
      lastState.currentContent !== newProps.currentContent &&
      lastState.addNewOptions.length === 0
    ) {
      newProps.getActions(newProps.currentContent.Id)
    }
    const optionList: ActionModel[] = []
    const folderList: ActionModel[] = []
    if (lastState.addNewOptions.length !== newProps.actions.length) {
      newProps.actions.map((action: NewAction) => {
        const contentType = action.Url.includes('ContentType') ? getContentTypeFromUrl(action.Url) : null
        const extension = contentType === 'File' ? getExtensionFromUrl(action.Url) : null
        const displayName =
          action.DisplayName.indexOf('New') === -1 ? action.DisplayName : action.DisplayName.substring(3)
        const newDisplayName =
          action.DisplayName.indexOf('New') === -1 ? `New ${action.DisplayName.toLowerCase()}` : action.DisplayName
        action.DisplayName = newDisplayName
        // tslint:disable-next-line:no-string-literal
        action['Action'] = () => {
          newProps.closeActionMenu()
          newProps.openDialog(
            <AddNewDialog
             // parentPath will always be wrong after you double click on a folder.
              parentPath={newProps.currentContent ? newProps.currentContent.Path : ''}
              contentTypeName={contentType || ''}
              extension={extension || ''}
              title={contentType === 'File' ? displayName : contentType ? contentType : ''}
            />,
            newDisplayName,
            newProps.closeDialog
          )
        }
        if (action.DisplayName.indexOf('folder') > -1) {
          if (action.DisplayName.indexOf('smart') === -1) {
            folderList.push(action)
          }
        } else {
          optionList.push(action)
        }
      })
    }
    return {
      ...lastState,
      currentContent: newProps.currentContent,
      addNewOptions:
        lastState.addNewOptions.length !== newProps.actions.length
          ? [...optionList, ...folderList]
          : lastState.addNewOptions
    }
  }

Solution

We have to check if the parent is loading and use the property parentIdOrPath as it seems to have the most up-to-date path when transitioning between parents.

Add these two properties to mapStateToProps in the AddNewMenu.tsx

const mapStateToProps = (state: rootStateType) => {
  return {
    actions: state.dms.actionmenu.addNewTypes,
    repository: state.sensenet.session.repository,
    isParentLoading: state.dms.documentLibrary.isLoadingParent,
    ParentPath: state.dms.documentLibrary.parentIdOrPath
  }
}

then in the function getDerivedStateFromProps, all we need to do is add that additional check and cast ParentPath to a string as its type is string | number | undefined.

public static getDerivedStateFromProps(newProps: AddNewMenu['props'], lastState: AddNewMenu['state']) {
    if (
      newProps.currentContent &&
      newProps.currentContent.Id &&
      lastState.currentContent !== newProps.currentContent &&
      lastState.addNewOptions.length === 0
    ) {
      newProps.getActions(newProps.currentContent.Id)
    }
    const optionList: ActionModel[] = []
    const folderList: ActionModel[] = []

    if (lastState.addNewOptions.length !== newProps.actions.length && newProps.isParentLoading) {
      newProps.actions.map((action: NewAction) => {
        const contentType = action.Url.includes('ContentType') ? getContentTypeFromUrl(action.Url) : null
        const extension = contentType === 'File' ? getExtensionFromUrl(action.Url) : null
        const displayName =
          action.DisplayName.indexOf('New') === -1 ? action.DisplayName : action.DisplayName.substring(3)
        const newDisplayName =
          action.DisplayName.indexOf('New') === -1 ? `New ${action.DisplayName.toLowerCase()}` : action.DisplayName
        action.DisplayName = newDisplayName

        // tslint:disable-next-line:no-string-literal
        action['Action'] = () => {
          newProps.closeActionMenu()
          newProps.openDialog(
            <AddNewDialog
              parentPath={newProps.ParentPath as string}
              contentTypeName={contentType || ''}
              extension={extension || ''}
              title={contentType === 'File' ? displayName : contentType ? contentType : ''}
            />,
            newDisplayName,
            newProps.closeDialog
          )
        }
        if (action.DisplayName.indexOf('folder') > -1) {
          if (action.DisplayName.indexOf('smart') === -1) {
            folderList.push(action)
          }
        } else {
          optionList.push(action)
        }
      })
    }
    return {
      ...lastState,
      currentContent: newProps.currentContent,
      addNewOptions:
        lastState.addNewOptions.length !== newProps.actions.length
          ? [...optionList, ...folderList]
          : lastState.addNewOptions
    }
  }
zoltanbedi commented 5 years ago

Hi @LuisAverhoff.

Thanks for opening a bug for this. Would you mind creating a pr with a fix? You can reach out at gitter if you need help.

LuisAverhoff commented 5 years ago

@B3zo0 Sure, I've updated this issue with a solution that seems to be working. I'm going to test my solution more thoroughly and see if there are any edge cases that I'm missing. Once that is done, I'll make a pull request.

LuisAverhoff commented 5 years ago

@B3zo0 Well my solution works fine but the problem with it is that it's dependent upon the parent loading. If the user clicks on the document menu tab quickly enough, then none of the actions get listed. They would have to wait until the parent starts loading so that the proper conditions are met. Do you know of a better solution than this?