Browse Source

Merge pull request #34627 from overleaf/ii-share-modal-improvements

[web] New share modal improvements

GitOrigin-RevId: d8b47a6294f8d7e4833130fd2bb1720bfe2357d8
ilkin-overleaf 1 month ago
parent
commit
f536b39b03

+ 14 - 2
services/web/frontend/js/features/share-project-modal/components/add-collaborators-select.tsx

@@ -122,8 +122,6 @@ function AddCollaboratorsSelect({
         continue
       }
 
-      hasInvited = true
-
       let data
 
       try {
@@ -196,17 +194,31 @@ function AddCollaboratorsSelect({
           setInFlight(false)
         }
       } else if (data.invite) {
+        hasInvited = true
         updateProject({
           invites: invites?.concat(data.invite) || [data.invite],
         })
       } else if (data.users) {
+        hasInvited = true
         updateProject({
           members: members?.concat(data.users) || data.users,
         })
       } else if (data.user) {
+        hasInvited = true
         updateProject({
           members: members?.concat(data.user) || [data.user],
         })
+      } else if (!('invite' in data)) {
+        // a successful resend returns an empty body (no `invite` field)
+        hasInvited = true
+      } else {
+        hasError = true
+        setError('generic_something_went_wrong')
+        if (isSharingUpdatesEnabled) {
+          setIsSubmitting(false)
+        } else {
+          setInFlight(false)
+        }
       }
 
       // wait for a short time, so canAddCollaborators has time to update with new collaborator information

+ 11 - 3
services/web/frontend/js/features/share-project-modal/components/give-feedback-link.tsx

@@ -1,14 +1,22 @@
 import { useTranslation } from 'react-i18next'
 import OLButton from '@/shared/components/ol/ol-button'
 import getMeta from '@/utils/meta'
+import { useSplitTest } from '@/shared/context/split-test-context'
 
 export default function GiveFeedbackLink() {
   const { t } = useTranslation()
   const isProfessionalGroupPlan = getMeta('ol-user')?.isProfessionalGroupPlan
+  const { info } = useSplitTest('sharing-updates')
 
-  const link = isProfessionalGroupPlan
-    ? 'https://forms.gle/rz1JDMuNajWG4ZY49'
-    : 'https://forms.gle/WLEjzG4Ayp8zFscM9'
+  let link: string
+  if (info?.phase === 'labs') {
+    link =
+      'https://docs.google.com/forms/d/e/1FAIpQLSeOsPzSw8lWLY310ZvR7BCK08v3Puc4JWFdV6K3m9QbsL2OSw/viewform'
+  } else if (isProfessionalGroupPlan) {
+    link = 'https://forms.gle/rz1JDMuNajWG4ZY49'
+  } else {
+    link = 'https://forms.gle/WLEjzG4Ayp8zFscM9'
+  }
 
   return (
     <OLButton

+ 14 - 12
services/web/frontend/js/features/share-project-modal/components/select-collaborators.tsx

@@ -16,6 +16,7 @@ import OLTag from '@/shared/components/ol/ol-tag'
 import AddCollaboratorsSelect from '@/features/share-project-modal/components/add-collaborators-select'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { isValidEmail } from '@/shared/utils/email'
+import { useShareProjectContext } from '@/features/share-project-modal/components/share-project-modal'
 
 export type ContactItem = {
   email: string
@@ -57,6 +58,7 @@ export default function SelectCollaborators({
 }) {
   const isSharingUpdatesEnabled = useFeatureFlag('sharing-updates')
   const { t } = useTranslation()
+  const { setSuccessActionMessage } = useShareProjectContext()
   const {
     getSelectedItemProps,
     getDropdownProps,
@@ -118,15 +120,6 @@ export default function SelectCollaborators({
     return true
   }, [inputValue, selectedItems])
 
-  function stateReducer(_: unknown, actionAndChanges: any) {
-    const { type, changes } = actionAndChanges
-    // force selected item to be null so that adding, removing, then re-adding the same collaborator is recognised as a selection change
-    if (type === useCombobox.stateChangeTypes.InputChange) {
-      return { ...changes, selectedItem: null }
-    }
-    return changes
-  }
-
   const {
     isOpen,
     getLabelProps,
@@ -137,10 +130,12 @@ export default function SelectCollaborators({
     reset,
   } = useCombobox({
     inputValue,
+    // Pinning `selectedItem` to null means every selection is treated as a change
+    // by downshift, so re-selecting the same option reliably fires `onStateChange`.
+    selectedItem: null,
     defaultHighlightedIndex: 0,
     items: filteredOptions,
     itemToString: item => (item && item.name) || '',
-    stateReducer,
     onStateChange: ({ type, selectedItem }) => {
       switch (type) {
         // add a selected item on Enter (keypress), click or blur
@@ -239,6 +234,10 @@ export default function SelectCollaborators({
     setInputErrors(prev => prev.filter(e => selectedEmails.includes(e.email)))
   }, [selectedEmails])
 
+  useEffect(() => {
+    setInviteSent(false)
+  }, [selectedItems])
+
   return (
     <div className="tags-input tags-new">
       {isSharingUpdatesEnabled ? (
@@ -327,7 +326,7 @@ export default function SelectCollaborators({
                           break
 
                         case ',':
-                          // comma: try to create a new item using inputValue
+                        case ' ':
                           event.preventDefault()
                           addNewItem(inputValue)
                           break
@@ -395,7 +394,10 @@ export default function SelectCollaborators({
                 multipleSelectionProps={multipleSelectionProps}
                 currentMemberEmails={currentMemberEmails}
                 inputValue={inputValue}
-                onInviteSuccess={() => setInviteSent(true)}
+                onInviteSuccess={() => {
+                  setInviteSent(true)
+                  setSuccessActionMessage(undefined)
+                }}
                 hasErrors={inputErrors.length > 0}
               />
             </div>

+ 1 - 1
services/web/frontend/js/features/share-project-modal/components/share-project-modal-content.tsx

@@ -146,7 +146,7 @@ function ShareProjectModalContentInner({
               {successActionMessage && (
                 <div className="ms-auto px-3 align-self-center">
                   <div
-                    className="d-flex gap-3 align-items-center"
+                    className="d-flex gap-2 align-items-center"
                     role="status"
                     aria-live="polite"
                   >

+ 222 - 0
services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.tsx

@@ -998,6 +998,46 @@ describe('<ShareProjectModal/>', function () {
     })
   })
 
+  it('re-selects the same suggestion after removing it', async function () {
+    renderWithEditorContext(
+      <ShareProjectModal {...modalProps} />,
+      createContextProps()
+    )
+
+    const [inputElement] = await screen.findAllByLabelText('Add email address')
+
+    await waitFor(() => {
+      expect(fetchMock.callHistory.called('express:/user/contacts')).to.be.true
+    })
+
+    await userEvent.type(inputElement, 'pto')
+    await userEvent.click(
+      screen.getByRole('option', {
+        name: `Claudius Ptolemy <ptolemy@example.com>`,
+      })
+    )
+
+    const removeButton = await screen.findByRole('button', { name: /Remove/ })
+
+    await userEvent.click(removeButton)
+    await waitFor(() => {
+      expect(screen.queryByRole('button', { name: /remove/i })).to.be.null
+    })
+
+    await userEvent.click(inputElement)
+    await userEvent.click(
+      await screen.findByRole('option', {
+        name: `Claudius Ptolemy <ptolemy@example.com>`,
+      })
+    )
+
+    await waitFor(() => {
+      expect(screen.getAllByRole('button', { name: /remove/i })).to.have.length(
+        1
+      )
+    })
+  })
+
   describe('sharing-updates feature flag enabled', function () {
     beforeEach(function () {
       window.metaAttributesCache.set('ol-splitTestVariants', {
@@ -1255,6 +1295,8 @@ describe('<ShareProjectModal/>', function () {
 
     afterEach(function () {
       window.metaAttributesCache.set('ol-splitTestVariants', {})
+      window.metaAttributesCache.set('ol-splitTestInfo', {})
+      delete (navigator as any).clipboard
     })
 
     it('disables the invite button when no email is entered', async function () {
@@ -1368,6 +1410,164 @@ describe('<ShareProjectModal/>', function () {
       await screen.findByText('Invitation(s) sent.')
     })
 
+    it('shows a generic error and no success message when an invite fails (e.g. collaborator limit reached)', async function () {
+      fetchMock.post('express:/project/:projectId/invite', {
+        status: 200,
+        body: {
+          invite: null,
+        },
+      })
+
+      renderWithEditorContext(
+        <ShareProjectModal {...modalProps} />,
+        createContextProps({ publicAccessLevel: 'tokenBased' })
+      )
+
+      const inputElement = await screen.findByTestId('collaborator-email-input')
+      fireEvent.change(inputElement, { target: { value: 'new@example.com' } })
+      fireEvent.blur(inputElement)
+
+      const inviteButton = (await screen.findByRole('button', {
+        name: /invite/i,
+      })) as HTMLButtonElement
+      await waitFor(() => expect(inviteButton.disabled).to.be.false)
+      await userEvent.click(inviteButton)
+
+      await screen.findByText('Sorry, something went wrong')
+      expect(screen.queryByText('Invitation(s) sent.')).to.be.null
+    })
+
+    it('clears "successActionMessage" when invitations are sent', async function () {
+      Object.defineProperty(navigator, 'clipboard', {
+        value: { writeText: sinon.stub().resolves() },
+        configurable: true,
+        writable: true,
+      })
+
+      const sharingLinkToken = 'abc123token'
+      fetchMock.get('express:/project/:projectId/sharing-link', {
+        _id: 'invite-id',
+        token: sharingLinkToken,
+        privileges: 'readAndWrite',
+      })
+      fetchMock.post('express:/project/:projectId/invite', {
+        status: 200,
+        body: {
+          invite: {
+            _id: 'new-invite',
+            email: 'new@example.com',
+            privileges: 'readAndWrite',
+          },
+        },
+      })
+
+      renderWithEditorContext(
+        <ShareProjectModal {...modalProps} />,
+        createContextProps()
+      )
+
+      const copyButton: HTMLButtonElement = await screen.findByRole('button', {
+        name: /copy sharing link/i,
+      })
+      expect(copyButton.disabled).to.be.false
+
+      await userEvent.click(copyButton)
+      await screen.findByText(/link copied/i)
+
+      const inputElement = screen.getByTestId('collaborator-email-input')
+      fireEvent.change(inputElement, { target: { value: 'new@example.com' } })
+      fireEvent.blur(inputElement)
+
+      const inviteButton = (await screen.findByRole('button', {
+        name: /invite/i,
+      })) as HTMLButtonElement
+      await waitFor(() => expect(inviteButton.disabled).to.be.false)
+      await userEvent.click(inviteButton)
+
+      await screen.findByText('Invitation(s) sent.')
+      expect(screen.queryByText(/link copied/i)).to.be.null
+    })
+
+    it('clears "invitations sent" message when input is changed', async function () {
+      fetchMock.post('express:/project/:projectId/invite', {
+        status: 200,
+        body: {
+          invite: {
+            _id: 'new-invite',
+            email: 'new@example.com',
+            privileges: 'readAndWrite',
+          },
+        },
+      })
+
+      renderWithEditorContext(
+        <ShareProjectModal {...modalProps} />,
+        createContextProps()
+      )
+
+      const inputElement = await screen.findByTestId('collaborator-email-input')
+      fireEvent.change(inputElement, { target: { value: 'new@example.com' } })
+      fireEvent.blur(inputElement)
+
+      const inviteButton = (await screen.findByRole('button', {
+        name: /invite/i,
+      })) as HTMLButtonElement
+      await waitFor(() => expect(inviteButton.disabled).to.be.false)
+      await userEvent.click(inviteButton)
+
+      await screen.findByText('Invitation(s) sent.')
+
+      fireEvent.change(inputElement, { target: { value: 'a' } })
+
+      await waitFor(
+        () => expect(screen.queryByText('Invitation(s) sent.')).to.be.null
+      )
+    })
+
+    it('clears "invitations sent" message when a selected item is removed', async function () {
+      fetchMock.post('express:/project/:projectId/invite', {
+        status: 200,
+        body: {
+          invite: {
+            _id: 'new-invite',
+            email: 'new@example.com',
+            privileges: 'readAndWrite',
+          },
+        },
+      })
+
+      renderWithEditorContext(
+        <ShareProjectModal {...modalProps} />,
+        createContextProps()
+      )
+
+      const inputElement = await screen.findByTestId('collaborator-email-input')
+      fireEvent.change(inputElement, { target: { value: 'new@example.com' } })
+      fireEvent.blur(inputElement)
+
+      const inviteButton = (await screen.findByRole('button', {
+        name: /invite/i,
+      })) as HTMLButtonElement
+      await waitFor(() => expect(inviteButton.disabled).to.be.false)
+      await userEvent.click(inviteButton)
+
+      await screen.findByText('Invitation(s) sent.')
+
+      fireEvent.change(inputElement, {
+        target: { value: 'another@example.com' },
+      })
+      fireEvent.blur(inputElement)
+
+      const removeButton = await screen.findByRole('button', {
+        name: /remove/i,
+      })
+      await userEvent.click(removeButton)
+
+      await waitFor(
+        () => expect(screen.queryByText('Invitation(s) sent.')).to.be.null
+      )
+    })
+
     it('shows the "Give feedback" link for the project owner', async function () {
       renderWithEditorContext(
         <ShareProjectModal {...modalProps} />,
@@ -1425,6 +1625,28 @@ describe('<ShareProjectModal/>', function () {
           'https://forms.gle/WLEjzG4Ayp8zFscM9'
         )
       })
+
+      it('links to the Labs feedback URL when the "sharing-updates" feature is in the Labs phase', async function () {
+        window.metaAttributesCache.set('ol-splitTestInfo', {
+          'sharing-updates': { phase: 'labs' },
+        })
+
+        renderWithEditorContext(<ShareProjectModal {...modalProps} />, {
+          ...createContextProps(),
+          user: {
+            id: USER_ID,
+            email: USER_EMAIL,
+            isProfessionalGroupPlan: false,
+          },
+        })
+
+        const feedbackLink = await screen.findByRole('link', {
+          name: 'Give feedback',
+        })
+        expect(feedbackLink.getAttribute('href')).to.equal(
+          'https://docs.google.com/forms/d/e/1FAIpQLSeOsPzSw8lWLY310ZvR7BCK08v3Puc4JWFdV6K3m9QbsL2OSw/viewform'
+        )
+      })
     })
   })
 })