-
Notifications
You must be signed in to change notification settings - Fork 116
Image button #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Image button #167
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import { v4 as uuidv4 } from 'uuid'; | ||
| import { AuthTestUtils } from '../../../support/auth-utils'; | ||
| import { EditorSelectors, waitForReactUpdate, SlashCommandSelectors, AddPageSelectors } from '../../../support/selectors'; | ||
|
|
||
| describe('Copy Image Test', () => { | ||
| const authUtils = new AuthTestUtils(); | ||
| const testEmail = `${uuidv4()}@appflowy.io`; | ||
|
|
||
| beforeEach(() => { | ||
| cy.on('uncaught:exception', () => false); | ||
|
|
||
| // Mock the image fetch | ||
| cy.intercept('GET', '**/logo.png', { | ||
| statusCode: 200, | ||
| fixture: 'appflowy.png', | ||
| headers: { | ||
| 'content-type': 'image/png', | ||
| }, | ||
| }).as('getImage'); | ||
|
|
||
| // We need to mock the clipboard write | ||
| cy.window().then((win) => { | ||
| // Check if clipboard exists | ||
| if (win.navigator.clipboard) { | ||
| cy.stub(win.navigator.clipboard, 'write').as('clipboardWrite'); | ||
| } else { | ||
| // Mock clipboard if it doesn't exist or is not writable directly | ||
| // In some browsers, we might need to redefine the property | ||
| const clipboardMock = { | ||
| write: cy.stub().as('clipboardWrite') | ||
| }; | ||
| try { | ||
| // @ts-ignore | ||
| win.navigator.clipboard = clipboardMock; | ||
| } catch (e) { | ||
| Object.defineProperty(win.navigator, 'clipboard', { | ||
| value: clipboardMock, | ||
| configurable: true, | ||
| writable: true | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| cy.visit('/login', { failOnStatusCode: false }); | ||
| authUtils.signInWithTestUrl(testEmail).then(() => { | ||
| cy.url({ timeout: 30000 }).should('include', '/app'); | ||
| waitForReactUpdate(1000); | ||
| }); | ||
| }); | ||
|
|
||
| it('should copy image to clipboard when clicking copy button', () => { | ||
| // Create a new page | ||
| AddPageSelectors.inlineAddButton().first().click(); | ||
| waitForReactUpdate(500); | ||
| cy.get('[role="menuitem"]').first().click(); // Create Doc | ||
| waitForReactUpdate(1000); | ||
|
|
||
| // Focus editor | ||
| EditorSelectors.firstEditor().should('exist').click({ force: true }); | ||
| waitForReactUpdate(1000); | ||
|
|
||
| // Ensure focus | ||
| EditorSelectors.firstEditor().focus(); | ||
| waitForReactUpdate(500); | ||
|
|
||
| // Type '/' to open slash menu | ||
| EditorSelectors.firstEditor().type('/', { force: true }); | ||
| waitForReactUpdate(1000); | ||
|
|
||
| // Check if slash panel exists | ||
| cy.get('[data-testid="slash-panel"]').should('exist').should('be.visible'); | ||
|
|
||
| // Type 'image' to filter | ||
| EditorSelectors.firstEditor().type('image', { force: true }); | ||
| waitForReactUpdate(1000); | ||
|
|
||
| // Click Image item | ||
| cy.get('[data-testid^="slash-menu-"]').contains(/^Image$/).click({ force: true }); | ||
| waitForReactUpdate(1000); | ||
|
|
||
| // Upload image directly | ||
| cy.get('input[type="file"]').attachFile('appflowy.png'); | ||
| waitForReactUpdate(2000); | ||
|
|
||
| waitForReactUpdate(2000); | ||
|
|
||
| // The image should now be rendered. | ||
| // We need to hover or click it to see the toolbar. | ||
| // The toolbar is only visible when the block is selected/focused or hovered. | ||
| // ImageToolbar.tsx uses useSlateStatic, suggesting it's part of the slate render. | ||
|
|
||
| // Find the image block. | ||
| cy.get('[data-block-type="image"]').first().should('exist').trigger('mouseover', { force: true }).click({ force: true }); | ||
| waitForReactUpdate(1000); | ||
|
|
||
| // Click the copy button | ||
| cy.get('[data-testid="copy-image-button"]').should('exist').click({ force: true }); | ||
|
|
||
| // Verify clipboard write | ||
| cy.get('@clipboardWrite').should('have.been.called'); | ||
| cy.get('@clipboardWrite').should((stub: any) => { | ||
| const clipboardItem = stub.args[0][0][0]; | ||
| expect(clipboardItem).to.be.instanceOf(ClipboardItem); | ||
| expect(clipboardItem.types).to.include('image/png'); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ import ActionButton from '@/components/editor/components/toolbar/selection-toolb | |
| import Align from '@/components/editor/components/toolbar/selection-toolbar/actions/Align'; | ||
| import { ImageBlockNode } from '@/components/editor/editor.type'; | ||
| import { useEditorContext } from '@/components/editor/EditorContext'; | ||
| import { copyTextToClipboard } from '@/utils/copy'; | ||
| import { fetchImageBlob } from '@/utils/image'; | ||
|
|
||
| function ImageToolbar({ node }: { node: ImageBlockNode }) { | ||
| const editor = useSlateStatic() as YjsEditor; | ||
|
|
@@ -27,9 +27,21 @@ function ImageToolbar({ node }: { node: ImageBlockNode }) { | |
| setOpenPreview(true); | ||
| }; | ||
|
|
||
| const onCopy = async () => { | ||
| await copyTextToClipboard(node.data.url || ''); | ||
| notify.success(t('document.plugins.image.copiedToPasteBoard')); | ||
| const onCopyImage = async () => { | ||
| const blob = await fetchImageBlob(node.data.url || ''); | ||
|
|
||
| if (blob) { | ||
| try { | ||
| await navigator.clipboard.write([ | ||
| new ClipboardItem({ | ||
| [blob.type]: blob, | ||
| }), | ||
| ]); | ||
| notify.success(t('document.plugins.image.copiedToPasteBoard')); | ||
| } catch (error) { | ||
| notify.error('Failed to copy image'); | ||
|
Comment on lines
+34
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: The error message for failed image copy is hardcoded and not localized. The success path already uses |
||
| } | ||
| } | ||
| }; | ||
|
|
||
| const onDelete = () => { | ||
|
|
@@ -45,7 +57,7 @@ function ImageToolbar({ node }: { node: ImageBlockNode }) { | |
| </ActionButton> | ||
| )} | ||
|
|
||
| <ActionButton onClick={onCopy} tooltip={t('button.copyLinkOriginal')}> | ||
| <ActionButton onClick={onCopyImage} tooltip={t('button.copy')} data-testid="copy-image-button"> | ||
| <CopyIcon /> | ||
| </ActionButton> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,4 +104,40 @@ export const checkImage = async (url: string) => { | |
|
|
||
| img.src = url; | ||
| }); | ||
| }; | ||
|
|
||
| export const fetchImageBlob = async (url: string): Promise<Blob | null> => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (complexity): Consider refactoring fetchImageBlob so URL/header resolution is separated from the fetch call and the network logic is shared in a single try/catch block to reduce duplication. You can simplify For example: export const fetchImageBlob = async (url: string): Promise<Blob | null> => {
const isStorageUrl = isAppFlowyFileStorageUrl(url);
// Resolve URL and headers up front
if (isStorageUrl) {
const token = getTokenParsed();
if (!token) return null;
const fullUrl = resolveImageUrl(url);
const headers = { Authorization: `Bearer ${token.access_token}` };
try {
const response = await fetch(fullUrl, { headers });
if (!response.ok) return null;
return await response.blob();
} catch {
return null;
}
}
try {
const response = await fetch(url);
if (!response.ok) return null;
return await response.blob();
} catch {
return null;
}
};You can go a step further and share the export const fetchImageBlob = async (url: string): Promise<Blob | null> => {
const isStorageUrl = isAppFlowyFileStorageUrl(url);
let finalUrl = url;
let headers: HeadersInit | undefined;
if (isStorageUrl) {
const token = getTokenParsed();
if (!token) return null;
finalUrl = resolveImageUrl(url);
headers = { Authorization: `Bearer ${token.access_token}` };
}
try {
const response = await fetch(finalUrl, headers ? { headers } : undefined);
if (!response.ok) return null;
return await response.blob();
} catch {
return null;
}
};This keeps:
|
||
| if (isAppFlowyFileStorageUrl(url)) { | ||
| const token = getTokenParsed(); | ||
|
|
||
| if (!token) return null; | ||
|
|
||
| const fullUrl = resolveImageUrl(url); | ||
|
|
||
| try { | ||
| const response = await fetch(fullUrl, { | ||
| headers: { | ||
| Authorization: `Bearer ${token.access_token}`, | ||
| }, | ||
| }); | ||
|
|
||
| if (response.ok) { | ||
| return await response.blob(); | ||
| } | ||
| } catch (error) { | ||
| return null; | ||
| } | ||
| } else { | ||
| try { | ||
| const response = await fetch(url); | ||
|
|
||
| if (response.ok) { | ||
| return await response.blob(); | ||
| } | ||
| } catch (error) { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: When
fetchImageBlobreturns null, the handler fails silently without user feedback.In that case, the function just returns and the button appears unresponsive to the user. Consider showing an error toast when
blobis null so users know the copy failed (and ideally why).