diff --git a/webapp/i18n/en.json b/webapp/i18n/en.json index c3932673..3baa5e77 100644 --- a/webapp/i18n/en.json +++ b/webapp/i18n/en.json @@ -145,6 +145,7 @@ "docs.share.access.canView": "Can View", "docs.share.copyLink": "Copy link", "docs.share.handle": "@{username}", + "docs.share.linkCopied": "Copied", "docs.share.noResults": "No people found", "docs.share.remove": "Remove {name}", "docs.share.search": "Add people or groups", @@ -153,8 +154,6 @@ "docs.share.visibility.disabledReason": "Public spaces are coming soon", "docs.share.visibility.private": "Private", "docs.share.visibility.privateHint": "Only invited members", - "docs.share.visibility.public": "Public", - "docs.share.visibility.publicHint": "Anyone in Mattermost", "docs.sidebar.add.browse": "Browse spaces", "docs.sidebar.add.create": "Create a space", "docs.sidebar.add.menu": "Add or browse spaces", @@ -221,6 +220,7 @@ "docs.spaceInfo.editDescription": "Edit description", "docs.spaceInfo.members": "Members", "docs.spaceInfo.menu.copyLink": "Copy link", + "docs.spaceInfo.menu.linkCopied": "Copied", "docs.spaceInfo.menu.members": "Members", "docs.spaceInfo.menu.settings": "Space settings", "docs.spaceInfo.menu.title": "Space info actions", diff --git a/webapp/src/components/share_space_modal/share_space_modal.test.tsx b/webapp/src/components/share_space_modal/share_space_modal.test.tsx index 9c89fd9e..8d24410c 100644 --- a/webapp/src/components/share_space_modal/share_space_modal.test.tsx +++ b/webapp/src/components/share_space_modal/share_space_modal.test.tsx @@ -3,6 +3,7 @@ import {act, fireEvent, screen, waitFor} from '@testing-library/react'; import React from 'react'; +import {copyToClipboard} from 'utils/clipboard'; import {makeSpace} from 'store/test_fixtures'; @@ -41,6 +42,8 @@ jest.mock('hooks/navigation', () => ({ useDocsNavigation: () => ({paths: {space: (id: string) => `/team/spaces/${id}`}}), })); +jest.mock('utils/clipboard', () => ({copyToClipboard: jest.fn(() => Promise.resolve(true))})); + // AddMembersField renders the real people picker, which pulls in mattermost-redux's // user search actions (published ESM that jest doesn't transform). Stub at the hook // boundary, as people_picker.test.tsx does. @@ -145,4 +148,13 @@ describe('ShareSpaceModal', () => { await waitFor(() => expect(mockLeave).toHaveBeenCalled()); expect(onClose).not.toHaveBeenCalled(); }); + + it('confirms the copy on the button itself', async () => { + renderModal(); + + fireEvent.click(screen.getByRole('button', {name: 'Copy link'})); + + expect(await screen.findByRole('button', {name: 'Copied'})).toBeInTheDocument(); + expect(copyToClipboard).toHaveBeenCalledWith('/team/spaces/space-1'); + }); }); diff --git a/webapp/src/components/share_space_modal/share_space_modal.tsx b/webapp/src/components/share_space_modal/share_space_modal.tsx index fc3fb857..862ab546 100644 --- a/webapp/src/components/share_space_modal/share_space_modal.tsx +++ b/webapp/src/components/share_space_modal/share_space_modal.tsx @@ -1,14 +1,15 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import {useCopyText} from 'hooks/copy_text'; import {useSpaceMemberProfiles} from 'hooks/members'; import {useDocsNavigation} from 'hooks/navigation'; import {useCanManageSpaceMembers} from 'hooks/permissions'; import {useManageSpaceMembers} from 'hooks/space_members'; import React, {useMemo} from 'react'; import {FormattedMessage, useIntl} from 'react-intl'; -import {copyToClipboard} from 'utils/clipboard'; +import CheckIcon from '@mattermost/compass-icons/components/check'; import ChevronDownIcon from '@mattermost/compass-icons/components/chevron-down'; import ContentCopyIcon from '@mattermost/compass-icons/components/content-copy'; import LockOutlineIcon from '@mattermost/compass-icons/components/lock-outline'; @@ -48,7 +49,9 @@ const ShareSpaceModal = ({space, onClose}: Props) => { disabled: busy, }; - const copyLink = () => copyToClipboard(absolutePaths.space(space.id)); + const copyLink = useCopyText(absolutePaths.space(space.id), { + announcement: formatMessage({id: 'docs.share.linkCopied', defaultMessage: 'Copied'}), + }); const title = ( { - - + {copyLink.copied ? : } + {copyLink.copied ? ( + + ) : ( + + )} ); diff --git a/webapp/src/components/space_info/space_info_menu.test.tsx b/webapp/src/components/space_info/space_info_menu.test.tsx new file mode 100644 index 00000000..15c13a52 --- /dev/null +++ b/webapp/src/components/space_info/space_info_menu.test.tsx @@ -0,0 +1,56 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {act, fireEvent, screen} from '@testing-library/react'; +import React from 'react'; +import {copyToClipboard} from 'utils/clipboard'; + +import {makeSpace} from 'store/test_fixtures'; + +import SpaceInfoMenu from './space_info_menu'; + +import {renderWithContext} from '../../../tests/react_testing_utils'; + +jest.mock('hooks/navigation', () => ({ + useDocsNavigation: () => ({paths: {space: (id: string) => `/team/spaces/${id}`}}), +})); + +jest.mock('hooks/permissions', () => ({ + useCanManageSpaceMembers: () => true, +})); + +jest.mock('utils/clipboard', () => ({copyToClipboard: jest.fn(() => Promise.resolve(true))})); + +const space = makeSpace('space-1', 'Engineering'); + +const renderMenu = () => renderWithContext( + , +); + +describe('SpaceInfoMenu', () => { + beforeEach(() => jest.clearAllMocks()); + + it('copies the space link', async () => { + renderMenu(); + + await act(async () => { + fireEvent.click(screen.getByRole('button', {name: 'Copy link'})); + }); + + expect(copyToClipboard).toHaveBeenCalledWith('/team/spaces/space-1'); + }); + + // MM-70344: the click gave no sign that anything happened. + it('confirms the copy on the item itself', async () => { + renderMenu(); + + fireEvent.click(screen.getByRole('button', {name: 'Copy link'})); + + expect(await screen.findByRole('button', {name: 'Copied'})).toBeInTheDocument(); + expect(screen.queryByRole('button', {name: 'Copy link'})).not.toBeInTheDocument(); + }); +}); diff --git a/webapp/src/components/space_info/space_info_menu.tsx b/webapp/src/components/space_info/space_info_menu.tsx index f96e4bf6..372ce25c 100644 --- a/webapp/src/components/space_info/space_info_menu.tsx +++ b/webapp/src/components/space_info/space_info_menu.tsx @@ -1,13 +1,14 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import {useCopyText} from 'hooks/copy_text'; import {useDocsNavigation} from 'hooks/navigation'; import {useCanManageSpaceMembers} from 'hooks/permissions'; import React, {useCallback} from 'react'; import {useIntl} from 'react-intl'; -import {copyToClipboard} from 'utils/clipboard'; import AccountMultipleOutlineIcon from '@mattermost/compass-icons/components/account-multiple-outline'; +import CheckIcon from '@mattermost/compass-icons/components/check'; import ChevronRightIcon from '@mattermost/compass-icons/components/chevron-right'; import CogOutlineIcon from '@mattermost/compass-icons/components/cog-outline'; import LinkVariantIcon from '@mattermost/compass-icons/components/link-variant'; @@ -84,9 +85,9 @@ const SpaceInfoMenu = ({space, memberCount, onShowMembers}: Props) => { )); }, [space]); - const copyLink = useCallback(() => { - copyToClipboard(absolutePaths.space(space.id)); - }, [absolutePaths, space.id]); + const copyLink = useCopyText(absolutePaths.space(space.id), { + announcement: formatMessage({id: 'docs.spaceInfo.menu.linkCopied', defaultMessage: 'Copied'}), + }); return ( ); diff --git a/webapp/src/hooks/copy_text.test.ts b/webapp/src/hooks/copy_text.test.ts new file mode 100644 index 00000000..c28a255f --- /dev/null +++ b/webapp/src/hooks/copy_text.test.ts @@ -0,0 +1,71 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {act, renderHook} from '@testing-library/react'; +import {copyToClipboard} from 'utils/clipboard'; + +import {clearReadout, getReadoutMessage} from 'components/readout/readout_store'; + +import {useCopyText} from './copy_text'; + +jest.mock('utils/clipboard', () => ({copyToClipboard: jest.fn()})); + +const mockCopy = copyToClipboard as jest.MockedFunction; + +describe('useCopyText', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.clearAllMocks(); + mockCopy.mockResolvedValue(true); + clearReadout(); + }); + + afterEach(() => jest.useRealTimers()); + + it('copies and confirms for a moment', async () => { + const {result} = renderHook(() => useCopyText('https://example.com/space')); + + expect(result.current.copied).toBe(false); + + await act(async () => result.current.copy()); + + expect(mockCopy).toHaveBeenCalledWith('https://example.com/space'); + expect(result.current.copied).toBe(true); + + act(() => jest.advanceTimersByTime(2000)); + + expect(result.current.copied).toBe(false); + }); + + // The clipboard write can fail on permissions or an insecure context, and + // confirming a copy that never happened is worse than staying quiet. + it('says nothing when the copy fails', async () => { + mockCopy.mockResolvedValue(false); + + const {result} = renderHook(() => useCopyText('link', {announcement: 'Copied'})); + + await act(async () => result.current.copy()); + + expect(result.current.copied).toBe(false); + expect(getReadoutMessage()).toBe(''); + }); + + it('announces the confirmation through the live region', async () => { + const {result} = renderHook(() => useCopyText('link', {announcement: 'Copied'})); + + await act(async () => result.current.copy()); + + expect(getReadoutMessage()).toBe('Copied'); + }); + + it('holds the confirmation open when copied again', async () => { + const {result} = renderHook(() => useCopyText('link')); + + await act(async () => result.current.copy()); + act(() => jest.advanceTimersByTime(1500)); + await act(async () => result.current.copy()); + act(() => jest.advanceTimersByTime(1500)); + + expect(result.current.copied).toBe(true); + }); +}); diff --git a/webapp/src/hooks/copy_text.ts b/webapp/src/hooks/copy_text.ts new file mode 100644 index 00000000..7b56134f --- /dev/null +++ b/webapp/src/hooks/copy_text.ts @@ -0,0 +1,59 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {useCallback, useEffect, useRef, useState} from 'react'; +import {copyToClipboard} from 'utils/clipboard'; + +import {announce} from 'components/readout/readout_store'; + +const COPIED_TIMEOUT = 2000; + +type CopyOptions = { + announcement?: string; + timeout?: number; +}; + +type CopyText = { + + /** True for a moment after a copy, for controls that confirm in place. */ + copied: boolean; + copy: () => void; +}; + +// Core's useCopyText, which plugins can't import. Controls own the confirmation: +// core swaps the label and icon rather than raising a toast. +export function useCopyText(text: string, {announcement, timeout = COPIED_TIMEOUT}: CopyOptions = {}): CopyText { + const [copied, setCopied] = useState(false); + const timer = useRef | null>(null); + const mounted = useRef(true); + + useEffect(() => { + mounted.current = true; + + return () => { + mounted.current = false; + if (timer.current) { + clearTimeout(timer.current); + } + }; + }, []); + + const copy = useCallback(async () => { + const done = await copyToClipboard(text); + if (!done || !mounted.current) { + return; + } + + if (announcement) { + announce(announcement); + } + + if (timer.current) { + clearTimeout(timer.current); + } + setCopied(true); + timer.current = setTimeout(() => setCopied(false), timeout); + }, [announcement, text, timeout]); + + return {copied, copy}; +} diff --git a/webapp/src/utils/clipboard.ts b/webapp/src/utils/clipboard.ts index 54df50da..9da4f50d 100644 --- a/webapp/src/utils/clipboard.ts +++ b/webapp/src/utils/clipboard.ts @@ -4,17 +4,19 @@ // Copies text to the clipboard, preferring the async Clipboard API and falling // back to a hidden textarea + execCommand on browsers/contexts without it (or // when writeText rejects — e.g. permissions or an insecure context). -// Mirrors the core webapp's utils.copyToClipboard. -export function copyToClipboard(text: string): void { +// Mirrors the core webapp's utils.copyToClipboard, resolving to whether the text +// made it, so callers can confirm a copy that actually happened. +export function copyToClipboard(text: string): Promise { if (navigator.clipboard) { - navigator.clipboard.writeText(text).catch(() => legacyCopy(text)); - return; + return navigator.clipboard.writeText(text). + then(() => true). + catch(() => legacyCopy(text)); } - legacyCopy(text); + return Promise.resolve(legacyCopy(text)); } -function legacyCopy(text: string): void { +function legacyCopy(text: string): boolean { const textArea = document.createElement('textarea'); textArea.style.position = 'fixed'; textArea.style.top = '0'; @@ -29,6 +31,14 @@ function legacyCopy(text: string): void { textArea.value = text; document.body.appendChild(textArea); textArea.select(); - document.execCommand('copy'); + + let copied = false; + try { + copied = document.execCommand('copy'); + } catch { + copied = false; + } document.body.removeChild(textArea); + + return copied; }