Skip to content

Commit

Permalink
chore: remove usage of deprecated appBridgeBlock functions (#680)
Browse files Browse the repository at this point in the history
* chore: remove usage of deprecated appBridgeBlock functions

* test: improve LegacyTemplateDummy import

* test: improve LegacyTemplateDummy import

* fix: add unsubscribe to assetChooser and templateChooser hook
  • Loading branch information
ragi96 authored Jan 16, 2024
1 parent fabe170 commit 3fa74ba
Show file tree
Hide file tree
Showing 16 changed files with 157 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changeset/lemon-starfishes-visit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@frontify/app-bridge": patch
---

chore: replace usage of deprecated appBridgeBlock functions
29 changes: 25 additions & 4 deletions packages/app-bridge/src/react/useAssetChooser.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { afterEach, describe, it } from 'vitest';
import { cleanup, render } from '@testing-library/react';

import type { Asset } from '../types/Asset';
import { AssetDummy } from '../tests/AssetDummy';
import { AppBridgeBlock } from '../AppBridgeBlock';
import { useAssetChooser } from './useAssetChooser';
import { withAppBridgeBlockStubs } from '../tests/withAppBridgeBlockStubs';
Expand Down Expand Up @@ -33,7 +34,7 @@ const AssetChooserDummy = ({
);
};

describe('useReadyForPrint hook', () => {
describe('useAssetChooser hook', () => {
afterEach(() => {
cleanup();
});
Expand All @@ -43,14 +44,34 @@ describe('useReadyForPrint hook', () => {
const { getByTestId } = render(<BlockWithStubs />);
const openAssetChooserButton = getByTestId(OPEN_ASSET_CHOOSER_BUTTON_ID) as HTMLButtonElement;
openAssetChooserButton.click();
sinon.assert.calledOnce(appBridge.openAssetChooser);
sinon.assert.calledWith(appBridge.dispatch, sinon.match.has('name', 'openAssetChooser'));
});

it('should close the asset chooser', () => {
const [BlockWithStubs, appBridge] = withAppBridgeBlockStubs(AssetChooserDummy);
const { getByTestId } = render(<BlockWithStubs />);
const openAssetChooserButton = getByTestId(CLOSE_ASSET_CHOOSER_BUTTON_ID) as HTMLButtonElement;
const closeAssetChooserButton = getByTestId(CLOSE_ASSET_CHOOSER_BUTTON_ID) as HTMLButtonElement;
closeAssetChooserButton.click();
sinon.assert.calledWith(appBridge.dispatch, sinon.match.has('name', 'closeAssetChooser'));
});

it('should call the onAssetChosen callback when an asset is chosen', () => {
const [BlockWithStubs] = withAppBridgeBlockStubs(AssetChooserDummy);
const onAssetChosen = sinon.spy();
const { getByTestId } = render(<BlockWithStubs onAssetChosen={onAssetChosen} />);
const openAssetChooserButton = getByTestId(OPEN_ASSET_CHOOSER_BUTTON_ID) as HTMLButtonElement;
openAssetChooserButton.click();
sinon.assert.calledOnce(appBridge.closeAssetChooser);
sinon.assert.calledWith(onAssetChosen, [AssetDummy.with(123)]);
});

it('should unsubscribe if asset chooser gets opened and closed', () => {
const unsubscribeSpy = sinon.spy();
const [BlockWithStubs] = withAppBridgeBlockStubs(AssetChooserDummy, { unsubscribe: unsubscribeSpy });
const { getByTestId } = render(<BlockWithStubs />);
const openTemplateChooserButton = getByTestId(OPEN_ASSET_CHOOSER_BUTTON_ID) as HTMLButtonElement;
openTemplateChooserButton.click();
const closeTemplateChooserButton = getByTestId(CLOSE_ASSET_CHOOSER_BUTTON_ID) as HTMLButtonElement;
closeTemplateChooserButton.click();
sinon.assert.calledOnce(unsubscribeSpy);
});
});
16 changes: 14 additions & 2 deletions packages/app-bridge/src/react/useAssetChooser.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,28 @@
/* (c) Copyright Frontify Ltd., all rights reserved. */

import { closeAssetChooser, openAssetChooser } from '../registries/commands/AssetChooser';
import type { AppBridgeBlock } from '../AppBridgeBlock';
import type { Asset, AssetChooserOptions } from '../types';
import { EventUnsubscribeFunction } from '../AppBridge';

type UseAssetChooserType = {
openAssetChooser: (callback: (selectedAsset: Asset[]) => void, options: AssetChooserOptions) => void;
closeAssetChooser: () => void;
};

export const useAssetChooser = (appBridge: AppBridgeBlock): UseAssetChooserType => {
let unsubscribe: EventUnsubscribeFunction;

return {
openAssetChooser: appBridge.openAssetChooser.bind(appBridge),
closeAssetChooser: appBridge.closeAssetChooser.bind(appBridge),
openAssetChooser: (callback, options) => {
appBridge.dispatch(openAssetChooser(options));
unsubscribe = appBridge.subscribe('assetsChosen', (selectedAssets) => {
callback(selectedAssets.assets);
});
},
closeAssetChooser: () => {
unsubscribe?.();
appBridge.dispatch(closeAssetChooser());
},
};
};
10 changes: 7 additions & 3 deletions packages/app-bridge/src/react/useAssetViewer.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ describe('useAssetViewer', () => {
const { result, appBridgeStub, asset } = await loadUseAssetViewer();
result.current.open(asset);

const call = appBridgeStub.openAssetViewer.getCall(0);
waitFor(() => {
expect(call.calledOnceWithExactly(asset.token)).toBe(true);
await waitFor(() => {
expect(
appBridgeStub.dispatch.calledOnceWithExactly({
name: 'openAssetViewer',
payload: { token: asset.token },
}),
).toBe(true);
});
});
});
7 changes: 4 additions & 3 deletions packages/app-bridge/src/react/useAssetViewer.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
/* (c) Copyright Frontify Ltd., all rights reserved. */

import { AppBridgeBlock } from '../AppBridgeBlock';
import { Asset } from '../types';
import { openAssetViewer } from '../registries/commands/AssetViewer';
import { type AppBridgeBlock } from '../AppBridgeBlock';
import { type Asset } from '../types';

export const useAssetViewer = (appBridge: AppBridgeBlock) => {
const open = async ({ token }: Asset) => {
appBridge.openAssetViewer(token);
appBridge.dispatch(openAssetViewer({ token }));
};

return {
Expand Down
2 changes: 1 addition & 1 deletion packages/app-bridge/src/react/useBlockAssets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { Asset } from '../types';
import { compareObjects } from '../utilities';

export const useBlockAssets = (appBridge: AppBridgeBlock) => {
const blockId = appBridge.getBlockId();
const blockId = appBridge.context('blockId').get();

const [blockAssets, setBlockAssets] = useState<Record<string, Asset[]>>({});

Expand Down
2 changes: 1 addition & 1 deletion packages/app-bridge/src/react/useBlockSettings.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const Block = ({ appBridge }: { appBridge: AppBridgeBlock }): ReactElement => {
const [blockSettings, updateBlockSettings] = useBlockSettings(appBridge);

return (
<div data-test-id={`block-${appBridge.getBlockId()}`}>
<div data-test-id={`block-${appBridge.context('blockId').get()}`}>
<div data-test-id={BLOCK_SETTINGS_DIV_ID}>{JSON.stringify(blockSettings)}</div>
<button onClick={() => updateBlockSettings(NEW_SETTINGS)} data-test-id={SET_BLOCK_SETTING_BUTTON}>
Update block setting
Expand Down
2 changes: 1 addition & 1 deletion packages/app-bridge/src/react/useBlockSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type BlockSettingsUpdateEvent<T = Record<string, unknown>> = {
export const useBlockSettings = <T = Record<string, unknown>>(
appBridge: AppBridgeBlock,
): [T, (newSettings: Partial<T>) => Promise<void>] => {
const blockId = appBridge.getBlockId();
const blockId = appBridge.context('blockId').get();
const [blockSettings, setBlockSettings] = useState<T>(structuredClone(window.blockSettings[blockId]) as T);

// Add listener for block settings updates
Expand Down
2 changes: 1 addition & 1 deletion packages/app-bridge/src/react/useBlockTemplates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { Template } from '../types';
import { compareObjects } from '../utilities';

export const useBlockTemplates = (appBridge: AppBridgeBlock) => {
const blockId = appBridge.getBlockId();
const blockId = appBridge.context('blockId').get();

const [blockTemplates, setBlockTemplates] = useState<Record<string, Template[]>>({});
const [error, setError] = useState<string | null>(null);
Expand Down
2 changes: 1 addition & 1 deletion packages/app-bridge/src/react/useColorPalettes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export type UseColorPalettesReturnType = {
};

export const useColorPalettes = (appBridge: AppBridgeBlock, colorPaletteIds?: number[]): UseColorPalettesReturnType => {
const blockId = appBridge.getBlockId();
const blockId = appBridge.context('blockId').get();

const [colorPalettes, setColorPalettes] = useState<ColorPalette[]>([]);

Expand Down
2 changes: 1 addition & 1 deletion packages/app-bridge/src/react/useColors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export type UseColorsReturnType = {
};

export const useColors = (appBridge: AppBridgeBlock, colorPaletteId: number): UseColorsReturnType => {
const blockId = appBridge.getBlockId();
const blockId = appBridge.context('blockId').get();

const [colorsByPaletteId, setColorsByPaletteId] = useState<Color[]>([]);

Expand Down
13 changes: 10 additions & 3 deletions packages/app-bridge/src/react/useReadyForPrint.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* (c) Copyright Frontify Ltd., all rights reserved. */

import React from 'react';
import { afterEach, describe, expect, test } from 'vitest';
import { afterEach, describe, expect, test, vi } from 'vitest';
import { cleanup, fireEvent, render, screen } from '@testing-library/react';

import { useReadyForPrint } from './useReadyForPrint';
Expand All @@ -14,12 +14,12 @@ const IS_READY_FOR_PRINT = 'is ready for print';
const IS_NOT_READY_FOR_PRINT = 'is not ready for print';
const BLOCK_ID = 345;

const ReadyForPrintDummy = () => {
const ReadyForPrintDummy = ({ dataBlockId = BLOCK_ID } = {}) => {
const appBridge = getAppBridgeBlockStub({ blockId: BLOCK_ID });
const { isReadyForPrint, setIsReadyForPrint } = useReadyForPrint(appBridge);

return (
<div data-test-id={IS_READY_CONTAINER} className="mod block" data-block={BLOCK_ID}>
<div data-test-id={IS_READY_CONTAINER} className="mod block" data-block={dataBlockId}>
<div>
{(isReadyForPrint && IS_READY_FOR_PRINT) || IS_NOT_READY_FOR_PRINT}
<button data-test-id={SET_TO_FALSE_BUTTON} onClick={() => setIsReadyForPrint(false)} />
Expand All @@ -32,6 +32,7 @@ const ReadyForPrintDummy = () => {
describe('useReadyForPrint hook', () => {
afterEach(() => {
cleanup();
vi.restoreAllMocks();
});

test('Should initially set the data-ready attribute to false', () => {
Expand Down Expand Up @@ -60,4 +61,10 @@ describe('useReadyForPrint hook', () => {
expect(container.getAttribute('data-ready')).toBe('false');
expect(screen.getByText(IS_NOT_READY_FOR_PRINT)).toBeDefined;
});

test('Should throw an error if no blockId is provided', () => {
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
render(<ReadyForPrintDummy dataBlockId={123} />);
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Could not find block wrapper:'), BLOCK_ID);
});
});
4 changes: 2 additions & 2 deletions packages/app-bridge/src/react/useReadyForPrint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ export const useReadyForPrint = (
const [ready, setReady] = useState<boolean>(false);

useEffect(() => {
const blockWrapper = document.querySelector(`.block[data-block="${appBridge.getBlockId()}"]`);
const blockWrapper = document.querySelector(`.block[data-block="${appBridge.context('blockId').get()}"]`);

if (!blockWrapper) {
console.error('Could not find block wrapper:', appBridge.getBlockId());
console.error('Could not find block wrapper:', appBridge.context('blockId').get());
return;
}

Expand Down
27 changes: 24 additions & 3 deletions packages/app-bridge/src/react/useTemplateChooser.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { TemplateLegacy } from '../types/TemplateLegacy';
import { AppBridgeBlock } from '../AppBridgeBlock';
import { useTemplateChooser } from './useTemplateChooser';
import { withAppBridgeBlockStubs } from '../tests/withAppBridgeBlockStubs';
import { TemplateLegacyDummy } from '../tests/TemplateLegacyDummy';

const OPEN_TEMPLATE_CHOOSER_BUTTON_ID = 'open-template-chooser';
const CLOSE_TEMPLATE_CHOOSER_BUTTON_ID = 'close-template-chooser';
Expand Down Expand Up @@ -43,14 +44,34 @@ describe('useReadyForPrint hook', () => {
const { getByTestId } = render(<BlockWithStubs />);
const openTemplateChooserButton = getByTestId(OPEN_TEMPLATE_CHOOSER_BUTTON_ID) as HTMLButtonElement;
openTemplateChooserButton.click();
sinon.assert.calledOnce(appBridge.openTemplateChooser);
sinon.assert.calledWith(appBridge.dispatch, sinon.match.has('name', 'openTemplateChooser'));
});

it('should close the template chooser', () => {
const [BlockWithStubs, appBridge] = withAppBridgeBlockStubs(TemplateChooserDummy);
const { getByTestId } = render(<BlockWithStubs />);
const openTemplateChooserButton = getByTestId(CLOSE_TEMPLATE_CHOOSER_BUTTON_ID) as HTMLButtonElement;
const closeTemplateChooserButton = getByTestId(CLOSE_TEMPLATE_CHOOSER_BUTTON_ID) as HTMLButtonElement;
closeTemplateChooserButton.click();
sinon.assert.calledWith(appBridge.dispatch, sinon.match.has('name', 'closeTemplateChooser'));
});

it('should call the onTemplateChosen callback when an template is chosen', () => {
const [BlockWithStubs] = withAppBridgeBlockStubs(TemplateChooserDummy);
const onTemplateChosen = sinon.spy();
const { getByTestId } = render(<BlockWithStubs onTemplateChosen={onTemplateChosen} />);
const openTemplateChooserButton = getByTestId(OPEN_TEMPLATE_CHOOSER_BUTTON_ID) as HTMLButtonElement;
openTemplateChooserButton.click();
sinon.assert.calledWith(onTemplateChosen, TemplateLegacyDummy.with(234));
});

it('should unsubscribe if template chooser gets opened and closed', () => {
const unsubscribeSpy = sinon.spy();
const [BlockWithStubs] = withAppBridgeBlockStubs(TemplateChooserDummy, { unsubscribe: unsubscribeSpy });
const { getByTestId } = render(<BlockWithStubs />);
const openTemplateChooserButton = getByTestId(OPEN_TEMPLATE_CHOOSER_BUTTON_ID) as HTMLButtonElement;
openTemplateChooserButton.click();
sinon.assert.calledOnce(appBridge.closeTemplateChooser);
const closeTemplateChooserButton = getByTestId(CLOSE_TEMPLATE_CHOOSER_BUTTON_ID) as HTMLButtonElement;
closeTemplateChooserButton.click();
sinon.assert.calledOnce(unsubscribeSpy);
});
});
15 changes: 13 additions & 2 deletions packages/app-bridge/src/react/useTemplateChooser.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
/* (c) Copyright Frontify Ltd., all rights reserved. */

import { closeTemplateChooser, openTemplateChooser } from '../registries/commands/TemplateChooser';
import type { AppBridgeBlock } from '../AppBridgeBlock';
import type { TemplateLegacy } from '../types/TemplateLegacy';
import { EventUnsubscribeFunction } from '../AppBridge';

type UseTemplateChooserType = {
openTemplateChooser: (callback: (selectedTemplate: TemplateLegacy) => void) => void;
closeTemplateChooser: () => void;
};

export const useTemplateChooser = (appBridge: AppBridgeBlock): UseTemplateChooserType => {
let unsubscribe: EventUnsubscribeFunction;
return {
openTemplateChooser: appBridge.openTemplateChooser.bind(appBridge),
closeTemplateChooser: appBridge.closeTemplateChooser.bind(appBridge),
openTemplateChooser: (callback) => {
appBridge.dispatch(openTemplateChooser());
unsubscribe = appBridge.subscribe('templateChosen', (selectedTemplate) => {
callback(selectedTemplate.template);
});
},
closeTemplateChooser: () => {
appBridge.dispatch(closeTemplateChooser());
unsubscribe?.();
},
};
};
Loading

0 comments on commit 3fa74ba

Please sign in to comment.