Merge pull request #3640 from overleaf/ta-file-tree-input-draggable

[ReactFileTree] Disable Draggable when Renaming Entity

GitOrigin-RevId: 7241815d43791685453431aa95b8258ec17d3f81

services/web/frontend/js/features/file-tree/components/file-tree-item/file-tree-item-inner.js

@@ -16,7 +16,7 @@ function FileTreeItemInner({ id, name, isSelected, icons }) {
 
   const hasMenu = hasWritePermissions && isSelected
 
-  const { isDragging, dragRef } = useDraggable(id)
+  const { isDragging, dragRef, isDraggable, setIsDraggable } = useDraggable(id)
 
   const itemRef = createRef()
 
@@ -54,6 +54,7 @@ function FileTreeItemInner({ id, name, isSelected, icons }) {
       role="presentation"
       ref={dragRef}
       onContextMenu={handleContextMenu}
+      draggable={isDraggable}
     >
       <div
         className="entity-name entity-name-react"
@@ -61,7 +62,11 @@ function FileTreeItemInner({ id, name, isSelected, icons }) {
         ref={itemRef}
       >
         {icons}
-        <FileTreeItemName name={name} isSelected={isSelected} />
+        <FileTreeItemName
+          name={name}
+          isSelected={isSelected}
+          setIsDraggable={setIsDraggable}
+        />
         {hasMenu ? <FileTreeItemMenu id={id} /> : null}
       </div>
     </div>

services/web/frontend/js/features/file-tree/components/file-tree-item/file-tree-item-name.js

@@ -1,11 +1,11 @@
-import React, { useState } from 'react'
+import React, { useState, useEffect } from 'react'
 import PropTypes from 'prop-types'
 
 import { useRefWithAutoFocus } from '../../../../infrastructure/auto-focus'
 
 import { useFileTreeActionable } from '../../contexts/file-tree-actionable'
 
-function FileTreeItemName({ name, isSelected }) {
+function FileTreeItemName({ name, isSelected, setIsDraggable }) {
   const {
     isRenaming,
     startRenaming,
@@ -16,6 +16,10 @@ function FileTreeItemName({ name, isSelected }) {
 
   const isRenamingEntity = isRenaming && isSelected && !error
 
+  useEffect(() => {
+    setIsDraggable(!isRenamingEntity)
+  }, [setIsDraggable, isRenamingEntity])
+
   if (isRenamingEntity) {
     return (
       <InputName
@@ -36,7 +40,8 @@ function FileTreeItemName({ name, isSelected }) {
 
 FileTreeItemName.propTypes = {
   name: PropTypes.string.isRequired,
-  isSelected: PropTypes.bool.isRequired
+  isSelected: PropTypes.bool.isRequired,
+  setIsDraggable: PropTypes.func.isRequired
 }
 
 function DisplayName({ name, isSelected, startRenaming }) {

services/web/frontend/js/features/file-tree/contexts/file-tree-draggable.js

@@ -1,4 +1,4 @@
-import React, { useRef, useEffect } from 'react'
+import React, { useRef, useEffect, useState } from 'react'
 import PropTypes from 'prop-types'
 import { useTranslation } from 'react-i18next'
 
@@ -78,6 +78,8 @@ export function useDraggable(draggedEntityId) {
   const { fileTreeData } = useFileTreeMutable()
   const { selectedEntityIds } = useFileTreeSelectable()
 
+  const [isDraggable, setIsDraggable] = useState(true)
+
   const item = { type: DRAGGABLE_TYPE }
   const [{ isDragging }, dragRef, preview] = useDrag({
     item, // required, but overwritten by the return value of `begin`
@@ -104,7 +106,9 @@ export function useDraggable(draggedEntityId) {
 
   return {
     dragRef,
-    isDragging
+    isDragging,
+    isDraggable,
+    setIsDraggable
   }
 }
 

services/web/test/frontend/features/file-tree/components/file-tree-item/file-tree-item-name.test.js

@@ -7,23 +7,38 @@ import renderWithContext from '../../helpers/render-with-context'
 import FileTreeItemName from '../../../../../../frontend/js/features/file-tree/components/file-tree-item/file-tree-item-name'
 
 describe('<FileTreeItemName />', function() {
+  const setIsDraggable = sinon.stub()
+
   beforeEach(function() {
     global.requestAnimationFrame = sinon.stub()
   })
 
   afterEach(function() {
     delete global.requestAnimationFrame
+    setIsDraggable.reset()
   })
 
   it('renders name as button', function() {
-    renderWithContext(<FileTreeItemName name="foo.tex" isSelected />)
+    renderWithContext(
+      <FileTreeItemName
+        name="foo.tex"
+        isSelected
+        setIsDraggable={setIsDraggable}
+      />
+    )
 
     screen.getByRole('button', { name: 'foo.tex' })
     expect(screen.queryByRole('textbox')).to.not.exist
   })
 
   it("doesn't start renaming on unselected component", function() {
-    renderWithContext(<FileTreeItemName name="foo.tex" isSelected={false} />)
+    renderWithContext(
+      <FileTreeItemName
+        name="foo.tex"
+        isSelected={false}
+        setIsDraggable={setIsDraggable}
+      />
+    )
 
     const button = screen.queryByRole('button')
     fireEvent.click(button)
@@ -33,7 +48,13 @@ describe('<FileTreeItemName />', function() {
   })
 
   it('start renaming on double-click', function() {
-    renderWithContext(<FileTreeItemName name="foo.tex" isSelected />)
+    renderWithContext(
+      <FileTreeItemName
+        name="foo.tex"
+        isSelected
+        setIsDraggable={setIsDraggable}
+      />
+    )
 
     const button = screen.queryByRole('button')
     fireEvent.click(button)
@@ -42,12 +63,20 @@ describe('<FileTreeItemName />', function() {
     screen.getByRole('textbox')
     expect(screen.queryByRole('button')).to.not.exist
     expect(global.requestAnimationFrame).to.be.calledOnce
+    expect(setIsDraggable).to.be.calledWith(false)
   })
 
   it('cannot start renaming in read-only', function() {
-    renderWithContext(<FileTreeItemName name="foo.tex" isSelected />, {
+    renderWithContext(
-      contextProps: { hasWritePermissions: false }
+      <FileTreeItemName
-    })
+        name="foo.tex"
+        isSelected
+        setIsDraggable={setIsDraggable}
+      />,
+      {
+        contextProps: { hasWritePermissions: false }
+      }
+    )
 
     const button = screen.queryByRole('button')
     fireEvent.click(button)
@@ -59,7 +88,13 @@ describe('<FileTreeItemName />', function() {
 
   describe('stop renaming', function() {
     beforeEach(function() {
-      renderWithContext(<FileTreeItemName name="foo.tex" isSelected />)
+      renderWithContext(
+        <FileTreeItemName
+          name="foo.tex"
+          isSelected
+          setIsDraggable={setIsDraggable}
+        />
+      )
 
       const button = screen.getByRole('button')
       fireEvent.click(button)
@@ -75,6 +110,7 @@ describe('<FileTreeItemName />', function() {
       fireEvent.keyDown(input, { key: 'Escape' })
 
       screen.getByRole('button', { name: 'foo.tex' })
+      expect(setIsDraggable).to.be.calledWith(true)
     })
   })
 })