Merge pull request #3965 from overleaf/ae-file-tree-popup

Use custom overlay for file tree dropdown menu

GitOrigin-RevId: 261b21953f9331427d6d368716662d7eaec65477

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

@@ -1,9 +1,10 @@
 import React, { useState } from 'react'
+import { findDOMNode } from 'react-dom'
 import PropTypes from 'prop-types'
 import { useTranslation } from 'react-i18next'
 import withoutPropagation from '../../../../infrastructure/without-propagation'
 
-import { Dropdown } from 'react-bootstrap'
+import { Dropdown, Overlay } from 'react-bootstrap'
 import Icon from '../../../../shared/components/icon'
 
 import FileTreeItemMenuItems from './file-tree-item-menu-items'
@@ -12,6 +13,7 @@ function FileTreeItemMenu({ id }) {
   const { t } = useTranslation()
 
   const [dropdownOpen, setDropdownOpen] = useState(false)
+  const [dropdownTarget, setDropdownTarget] = useState()
 
   function handleToggle(wantOpen) {
     setDropdownOpen(wantOpen)
@@ -21,6 +23,13 @@ function FileTreeItemMenu({ id }) {
     handleToggle(false)
   }
 
+  const toggleRef = component => {
+    if (component) {
+      // eslint-disable-next-line react/no-find-dom-node
+      setDropdownTarget(findDOMNode(component))
+    }
+  }
+
   return (
     <Dropdown
       onClick={withoutPropagation(handleClick)}
@@ -33,12 +42,18 @@ function FileTreeItemMenu({ id }) {
         noCaret
         className="dropdown-toggle-no-background entity-menu-toggle"
         onClick={withoutPropagation()}
+        ref={toggleRef}
       >
         <Icon type="ellipsis-v" accessibilityLabel={t('menu')} />
       </Dropdown.Toggle>
-      <Dropdown.Menu>
-        <FileTreeItemMenuItems />
-      </Dropdown.Menu>
+      <Overlay
+        bsRole="menu"
+        show={dropdownOpen}
+        target={dropdownTarget}
+        container={document.body}
+      >
+        <Menu dropdownId={`dropdown-${id}`} />
+      </Overlay>
     </Dropdown>
   )
 }
@@ -47,4 +62,20 @@ FileTreeItemMenu.propTypes = {
   id: PropTypes.string.isRequired,
 }
 
+function Menu({ dropdownId, style, className }) {
+  return (
+    <div className={`dropdown open ${className}`} style={style}>
+      <ul className="dropdown-menu" role="menu" aria-labelledby={dropdownId}>
+        <FileTreeItemMenuItems />
+      </ul>
+    </div>
+  )
+}
+
+Menu.propTypes = {
+  dropdownId: PropTypes.string.isRequired,
+  style: PropTypes.object,
+  className: PropTypes.string,
+}
+
 export default FileTreeItemMenu

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

@@ -35,11 +35,6 @@ describe('<FileTreeDoc/>', function () {
     fireEvent.click(treeitem)
 
     screen.getByRole('treeitem', { selected: true })
-    screen.getByRole('menuitem', { name: 'Rename' })
-    screen.getByRole('menuitem', { name: 'Delete' })
-    screen.getByRole('menuitem', { name: 'New File' })
-    screen.getByRole('menuitem', { name: 'New Folder' })
-    screen.getByRole('menuitem', { name: 'Upload' })
   })
 
   it('renders as linked file', function () {

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

@@ -50,13 +50,6 @@ describe('<FileTreeFolder/>', function () {
     const treeitem = screen.getByRole('treeitem', { selected: false })
     fireEvent.click(treeitem)
 
-    screen.getByRole('treeitem', { selected: true })
-    screen.getByRole('menuitem', { name: 'Rename' })
-    screen.getByRole('menuitem', { name: 'Delete' })
-    screen.getByRole('menuitem', { name: 'New File' })
-    screen.getByRole('menuitem', { name: 'New Folder' })
-    screen.getByRole('menuitem', { name: 'Upload' })
-
     screen.getByRole('treeitem', { selected: true })
     expect(screen.queryByRole('tree')).to.not.exist
   })

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

@@ -5,6 +5,7 @@ import { screen, fireEvent } from '@testing-library/react'
 import renderWithContext from '../../helpers/render-with-context'
 
 import FileTreeitemInner from '../../../../../../frontend/js/features/file-tree/components/file-tree-item/file-tree-item-inner'
+import FileTreeContextMenu from '../../../../../../frontend/js/features/file-tree/components/file-tree-context-menu'
 
 describe('<FileTreeitemInner />', function () {
   const setContextMenuCoords = sinon.stub()
@@ -41,18 +42,22 @@ describe('<FileTreeitemInner />', function () {
 
     it('open / close', function () {
       const { container } = renderWithContext(
-        <FileTreeitemInner id="123abc" name="bar.tex" isSelected />
+        <>
+          <FileTreeitemInner id="123abc" name="bar.tex" isSelected />
+          <FileTreeContextMenu />
+        </>
       )
 
-      const entityElement = container.querySelector('div.entity')
-
-      screen.getByRole('menu', { visible: false })
+      expect(screen.queryByRole('menu')).to.be.null
 
+      // open the context menu
+      const entityElement = container.querySelector('div.entity')
       fireEvent.contextMenu(entityElement)
       screen.getByRole('menu', { visible: true })
 
-      fireEvent.contextMenu(entityElement)
-      screen.getByRole('menu', { visible: false })
+      // close the context menu
+      fireEvent.click(entityElement)
+      expect(screen.queryByRole('menu')).to.be.null
     })
   })
 
@@ -83,7 +88,8 @@ describe('<FileTreeitemInner />', function () {
           },
         }
       )
-
+      const toggleButton = screen.getByRole('button', { name: 'Menu' })
+      fireEvent.click(toggleButton)
       const renameButton = screen.getByRole('menuitem', { name: 'Rename' })
       fireEvent.click(renameButton)
       expect(screen.queryByRole('button', { name: 'bar.tex' })).to.not.exist

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

@@ -1,5 +1,6 @@
 import React from 'react'
 import sinon from 'sinon'
+import { expect } from 'chai'
 import { screen, fireEvent } from '@testing-library/react'
 import renderWithContext from '../../helpers/render-with-context'
 
@@ -20,7 +21,9 @@ describe('<FileTreeitemMenu />', function () {
       />
     )
 
-    screen.getByRole('button', { name: 'Menu' })
+    const toggleButton = screen.getByRole('button', { name: 'Menu' })
+    fireEvent.click(toggleButton)
+
     screen.getByRole('menu')
   })
 
@@ -32,9 +35,9 @@ describe('<FileTreeitemMenu />', function () {
       />
     )
 
-    const toggleButton = screen.getByRole('button', { name: 'Menu' })
+    expect(screen.queryByRole('menu')).to.be.null
 
-    screen.getByRole('menu', { visible: false })
+    const toggleButton = screen.getByRole('button', { name: 'Menu' })
 
     fireEvent.click(toggleButton)
     screen.getByRole('menu', { visible: true })

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

@@ -89,6 +89,8 @@ describe('<FileTreeRoot/>', function () {
     // selected) This is needed to make sure the test fail.
     const treeitemFile = screen.getByRole('treeitem', { name: 'main.tex' })
     fireEvent.click(treeitemFile, { ctrlKey: true })
+    const toggleButton = screen.getByRole('button', { name: 'Menu' })
+    fireEvent.click(toggleButton)
     const deleteButton = screen.getByRole('menuitem', { name: 'Delete' })
     fireEvent.click(deleteButton)
     await waitFor(() => screen.getByRole('button', { name: 'Cancel' }))

services/web/test/frontend/features/file-tree/flows/context-menu.test.js

@@ -36,11 +36,11 @@ describe('FileTree Context Menu Flow', function () {
     )
     const treeitem = screen.getByRole('button', { name: 'main.tex' })
 
-    expect(screen.getAllByRole('menu').length).to.equal(1) // toolbar
+    expect(screen.queryByRole('menu')).to.be.null
 
     fireEvent.contextMenu(treeitem)
 
-    expect(screen.getAllByRole('menu').length).to.equal(2) // toolbar + menu
+    screen.getByRole('menu')
   })
 
   it("doesn't open in read only mode", async function () {

services/web/test/frontend/features/file-tree/flows/delete-entity.test.js

@@ -53,6 +53,9 @@ describe('FileTree Delete Entity Flow', function () {
       const treeitem = screen.getByRole('treeitem', { name: 'main.tex' })
       fireEvent.click(treeitem)
 
+      const toggleButton = screen.getByRole('button', { name: 'Menu' })
+      fireEvent.click(toggleButton)
+
       const deleteButton = screen.getByRole('menuitem', { name: 'Delete' })
       fireEvent.click(deleteButton)
     })
@@ -186,6 +189,8 @@ describe('FileTree Delete Entity Flow', function () {
       // as a proxy to check that the child entity has been unselect we start
       // a delete and ensure the modal is displayed (the cancel button can be
       // selected) This is needed to make sure the test fail.
+      const toggleButton = screen.getByRole('button', { name: 'Menu' })
+      fireEvent.click(toggleButton)
       const deleteButton = screen.getByRole('menuitem', { name: 'Delete' })
       fireEvent.click(deleteButton)
       await waitFor(() => screen.getByRole('button', { name: 'Cancel' }))
@@ -193,7 +198,7 @@ describe('FileTree Delete Entity Flow', function () {
   })
 
   describe('multiple entities', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       const rootFolder = [
         {
           _id: 'root-folder-id',
@@ -202,6 +207,7 @@ describe('FileTree Delete Entity Flow', function () {
           fileRefs: [{ _id: '789ghi', name: 'my.bib' }],
         },
       ]
+
       render(
         <FileTreeRoot
           rootFolder={rootFolder}
@@ -218,14 +224,23 @@ describe('FileTree Delete Entity Flow', function () {
         />
       )
 
+      // select two files
       const treeitemDoc = screen.getByRole('treeitem', { name: 'main.tex' })
       fireEvent.click(treeitemDoc)
       const treeitemFile = screen.getByRole('treeitem', { name: 'my.bib' })
       fireEvent.click(treeitemFile, { ctrlKey: true })
 
-      const deleteButton = screen.getAllByRole('menuitem', {
-        name: 'Delete',
-      })[0]
+      // open the context menu
+      const treeitemButton = screen.getByRole('button', { name: 'my.bib' })
+      fireEvent.contextMenu(treeitemButton)
+
+      // make sure the menu has opened, with only a "Delete" item (as multiple files are selected)
+      screen.getByRole('menu')
+      const menuItems = await screen.findAllByRole('menuitem')
+      expect(menuItems.length).to.equal(1)
+
+      // select the Delete menu item
+      const deleteButton = screen.getByRole('menuitem', { name: 'Delete' })
       fireEvent.click(deleteButton)
     })
 

services/web/test/frontend/features/file-tree/flows/rename-entity.test.js

@@ -185,6 +185,9 @@ describe('FileTree Rename Entity Flow', function () {
     const treeitem = screen.getByRole('treeitem', { name: treeitemName })
     fireEvent.click(treeitem)
 
+    const toggleButton = screen.getByRole('button', { name: 'Menu' })
+    fireEvent.click(toggleButton)
+
     const renameButton = screen.getByRole('menuitem', { name: 'Rename' })
     fireEvent.click(renameButton)