From 103c01d28e02bf491dd19c5866d0e89960e174c4 Mon Sep 17 00:00:00 2001 From: Zachary Eckert Date: Fri, 26 May 2023 21:03:13 +0000 Subject: [PATCH 1/5] Add onCopy prop for QuickEmbed --- .../src/QuickEmbed/QuickEmbed.spec.tsx | 30 ++++++++++++++++--- .../src/QuickEmbed/QuickEmbed.tsx | 11 ++++--- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx index 6ed6e0cc0..50395ba16 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx @@ -24,7 +24,7 @@ */ import React from 'react' -import { screen, waitFor } from '@testing-library/react' +import { fireEvent, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { renderWithTheme } from '@looker/components-test-utils' import { useThemeActions, useThemesStoreState } from '../Theme/state' @@ -50,6 +50,8 @@ describe('QuickEmbed', () => { ...overrides, }) const onClose = jest.fn() + const onCopy = jest.fn() + document.execCommand = jest.fn() beforeEach(() => { jest.spyOn(window, 'location', 'get').mockReturnValue({ @@ -69,7 +71,7 @@ describe('QuickEmbed', () => { }) it('renders', () => { - renderWithTheme() + renderWithTheme() expect( screen.getByRole('heading', { name: 'Get embed URL' }) @@ -103,7 +105,7 @@ describe('QuickEmbed', () => { href: 'https://example.com/looks/42', pathname: '/looks/42', } as Location) - renderWithTheme() + renderWithTheme() expect( screen.getByRole('heading', { name: 'Get embed URL' }) @@ -124,7 +126,7 @@ describe('QuickEmbed', () => { ;(useThemesStoreState as jest.Mock).mockReturnValue( getMockStoreState({ selectedTheme: customTheme1 }) ) - renderWithTheme() + renderWithTheme() expect( screen.getByRole('heading', { name: 'Get embed URL' }) @@ -153,4 +155,24 @@ describe('QuickEmbed', () => { ) }) }) + + it('close button function triggers on click', () => { + renderWithTheme() + const closeBtn = screen.getByRole('button', { name: 'Close' }) + + expect(closeBtn).toBeInTheDocument() + fireEvent.click(closeBtn) + + expect(onClose).toHaveBeenCalled() + }) + + it('copy button function triggers on click', () => { + renderWithTheme() + const copyBtn = screen.getByRole('button', { name: 'Copy Link' }) + + expect(copyBtn).toBeInTheDocument() + fireEvent.click(copyBtn) + + expect(onCopy).toHaveBeenCalled() + }) }) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx index aba4704e4..dd07847c8 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx @@ -44,9 +44,10 @@ import { useThemesStoreState, SelectTheme, useThemeActions } from '../Theme' interface QuickEmbedProps { onClose: () => void + onCopy: () => void } -export const QuickEmbed = ({ onClose }: QuickEmbedProps) => { +export const QuickEmbed = ({ onClose, onCopy }: QuickEmbedProps) => { const service = new EmbedUrl() const [toggleValue, setToggle] = useState(false) const [embedUrl, setEmbedUrl] = useState(service.embedUrl(false)) @@ -108,9 +109,11 @@ export const QuickEmbed = ({ onClose }: QuickEmbedProps) => { - - }>Copy Link - + + + }>Copy Link + + From a17b94cf405c0ed4af0643a1b9e111b70710c470 Mon Sep 17 00:00:00 2001 From: Zachary Eckert Date: Tue, 30 May 2023 18:13:24 +0000 Subject: [PATCH 2/5] Make onCopy prop optional --- .../src/QuickEmbed/QuickEmbed.spec.tsx | 10 ++++++++++ .../embed-components/src/QuickEmbed/QuickEmbed.tsx | 12 ++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx index 50395ba16..db3dfada3 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx @@ -166,6 +166,16 @@ describe('QuickEmbed', () => { expect(onClose).toHaveBeenCalled() }) + it('onCopy not called when not passed in', () => { + renderWithTheme() + const copyBtn = screen.getByRole('button', { name: 'Copy Link' }) + + expect(copyBtn).toBeInTheDocument() + fireEvent.click(copyBtn) + + expect(onCopy).not.toHaveBeenCalled() + }) + it('copy button function triggers on click', () => { renderWithTheme() const copyBtn = screen.getByRole('button', { name: 'Copy Link' }) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx index dd07847c8..8188c15c6 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx @@ -43,8 +43,10 @@ import { EmbedUrl } from '@looker/embed-services' import { useThemesStoreState, SelectTheme, useThemeActions } from '../Theme' interface QuickEmbedProps { + // function called when close button clicked onClose: () => void - onCopy: () => void + // function called when copy button is clicked. (copy to clipboard already handled) + onCopy?: () => void } export const QuickEmbed = ({ onClose, onCopy }: QuickEmbedProps) => { @@ -54,6 +56,12 @@ export const QuickEmbed = ({ onClose, onCopy }: QuickEmbedProps) => { const { selectedTheme } = useThemesStoreState() const { selectThemeAction } = useThemeActions() + const handleCopy = () => { + if (onCopy) { + onCopy() + } + } + const handleToggle = () => { const newToggleValue = !toggleValue if (newToggleValue) { @@ -109,7 +117,7 @@ export const QuickEmbed = ({ onClose, onCopy }: QuickEmbedProps) => { - + }>Copy Link From 198c554dfcf76b731e97c9998a2e072b6c2317c0 Mon Sep 17 00:00:00 2001 From: Zachary Eckert Date: Tue, 30 May 2023 19:43:36 +0000 Subject: [PATCH 3/5] update onCopy prop description --- packages/embed-components/src/QuickEmbed/QuickEmbed.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx index 8188c15c6..a75621a22 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx @@ -43,9 +43,10 @@ import { EmbedUrl } from '@looker/embed-services' import { useThemesStoreState, SelectTheme, useThemeActions } from '../Theme' interface QuickEmbedProps { - // function called when close button clicked + // A function triggered when close button is clicked. onClose: () => void - // function called when copy button is clicked. (copy to clipboard already handled) + /** An optional callback triggered when the copy button is clicked. + * The copy to clipboard action is already handled */ onCopy?: () => void } From c2e7710520720e5fd39067faac4af81b9aa87c3f Mon Sep 17 00:00:00 2001 From: Zachary Eckert Date: Wed, 31 May 2023 18:21:04 +0000 Subject: [PATCH 4/5] use userEvent instead of fireEvent --- .../src/QuickEmbed/QuickEmbed.spec.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx index db3dfada3..21ccc84a7 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.spec.tsx @@ -24,7 +24,7 @@ */ import React from 'react' -import { fireEvent, screen, waitFor } from '@testing-library/react' +import { screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { renderWithTheme } from '@looker/components-test-utils' import { useThemeActions, useThemesStoreState } from '../Theme/state' @@ -156,32 +156,32 @@ describe('QuickEmbed', () => { }) }) - it('close button function triggers on click', () => { + it('close button function triggers on click', async () => { renderWithTheme() const closeBtn = screen.getByRole('button', { name: 'Close' }) expect(closeBtn).toBeInTheDocument() - fireEvent.click(closeBtn) + await userEvent.click(closeBtn) expect(onClose).toHaveBeenCalled() }) - it('onCopy not called when not passed in', () => { + it('onCopy not called when not passed in', async () => { renderWithTheme() const copyBtn = screen.getByRole('button', { name: 'Copy Link' }) expect(copyBtn).toBeInTheDocument() - fireEvent.click(copyBtn) + await userEvent.click(copyBtn) expect(onCopy).not.toHaveBeenCalled() }) - it('copy button function triggers on click', () => { + it('copy button function triggers on click', async () => { renderWithTheme() const copyBtn = screen.getByRole('button', { name: 'Copy Link' }) expect(copyBtn).toBeInTheDocument() - fireEvent.click(copyBtn) + await userEvent.click(copyBtn) expect(onCopy).toHaveBeenCalled() }) From e77e745abca88983b2b30787c2ee4d4978b6a7b9 Mon Sep 17 00:00:00 2001 From: Zachary Eckert Date: Wed, 31 May 2023 19:24:35 +0000 Subject: [PATCH 5/5] update onCopy prop description --- packages/embed-components/src/QuickEmbed/QuickEmbed.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx index a75621a22..6c44a880a 100644 --- a/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx +++ b/packages/embed-components/src/QuickEmbed/QuickEmbed.tsx @@ -43,10 +43,12 @@ import { EmbedUrl } from '@looker/embed-services' import { useThemesStoreState, SelectTheme, useThemeActions } from '../Theme' interface QuickEmbedProps { - // A function triggered when close button is clicked. + /** A function triggered when close button is clicked. */ onClose: () => void - /** An optional callback triggered when the copy button is clicked. - * The copy to clipboard action is already handled */ + /** + * An optional callback triggered when the copy button is clicked. + * The copy to clipboard action is already handled + */ onCopy?: () => void }