Skip to content

feat(react): add useMessageProps hook for In-App Messaging #2340

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/react/src/components/InAppMessaging/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
export { useMessageImage } from './useMessageImage';
export {
MessageComponentStyles,
MessageOverrideStyle,
useMessageProps,
UseMessageProps,
} from './useMessageProps';
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`getMessageStyles returns the expected output in the happy path 1`] = `
Object {
"body": Object {
"color": "white",
"textAlign": "left",
},
"closeIconButton": Object {
"backgroundColor": "turquoise",
"color": "darkcyan",
},
"container": Object {
"backgroundColor": "lawngreen",
"borderRadius": 3,
},
"header": Object {
"backgroundColor": "lightpink",
"textAlign": "center",
},
"image": Object {
"backgroundColor": "royalblue",
},
"primaryButton": Object {
"backgroundColor": "seagreen",
"color": "black",
},
"secondaryButton": Object {
"backgroundColor": "sienna",
"color": "orchid",
},
}
`;

exports[`getPayloadStyle returns the expected output in the happy path 1`] = `
Object {
"body": Object {
"textAlign": "left",
},
"container": Object {
"backgroundColor": "lightgray",
"borderRadius": 2,
},
"header": Object {
"textAlign": "center",
},
"primaryButton": Object {
"backgroundColor": "salmon",
"color": "olive",
},
"secondaryButton": Object {
"backgroundColor": "sand",
"color": "peru",
},
}
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { renderHook } from '@testing-library/react-hooks';
import {
MessageButtonProps,
MessageComponentBaseProps,
} from '@aws-amplify/ui-react-core';

// import { useMessageImage } from '../../useMessageImage';
import { MessageOverrideStyle } from '../types';
import useMessageProps from '../useMessageProps';

// jest.mock('../../useMessageImage');

type TestComponentProps = MessageComponentBaseProps<MessageOverrideStyle>;

// const mockUseMessageImage = useMessageImage as jest.Mock;

const onDisplay = jest.fn();

describe('useMessageProps', () => {
beforeEach(() => {
// mockUseMessageImage.mockReturnValue({
// hasRenderableImage: false,
// isImageFetching: false,
// });
onDisplay.mockClear();
});

it('behaves as expected in the happy path', () => {
const props: TestComponentProps = {
layout: 'MIDDLE_BANNER',
onDisplay,
};

const { result } = renderHook(() => useMessageProps(props));

expect(onDisplay).toHaveBeenCalledTimes(1);
expect(result.current).toEqual({
hasButtons: false,
hasPrimaryButton: false,
hasRenderableImage: false,
hasSecondaryButton: false,
shouldRenderMessage: true,
styles: expect.any(Object) as MessageOverrideStyle,
});
});

it('behaves as expected when props includes an image', () => {
// mockUseMessageImage.mockReturnValue({
// hasRenderableImage: false,
// isImageFetching: true,
// });

const props: TestComponentProps = {
image: { src: 'https://test.png' },
layout: 'MIDDLE_BANNER',
onDisplay,
};

const { result, rerender } = renderHook(() => useMessageProps(props));

// first render
// expect(onDisplay).not.toHaveBeenCalled();
// expect(result.current).toEqual({
// hasButtons: false,
// hasPrimaryButton: false,
// hasRenderableImage: false,
// hasSecondaryButton: false,
// shouldRenderMessage: false,
// styles: null,
// });

// mockUseMessageImage.mockReturnValue({
// hasRenderableImage: true,
// isImageFetching: false,
// });

rerender();

expect(onDisplay).toHaveBeenCalledTimes(1);
expect(result.current).toEqual({
hasButtons: false,
hasPrimaryButton: false,
// hasRenderableImage: true,
hasRenderableImage: false,
hasSecondaryButton: false,
shouldRenderMessage: true,
styles: expect.any(Object) as MessageOverrideStyle,
});
});

it('returns the expected values when props includes buttons', () => {
const props: TestComponentProps = {
layout: 'MIDDLE_BANNER',

primaryButton: { title: 'primary', onAction: jest.fn() },
secondaryButton: { title: 'secondary', onAction: jest.fn() },
};

const { result } = renderHook(() => useMessageProps(props));

expect(result.current.hasButtons).toBe(true);
expect(result.current.hasPrimaryButton).toBe(true);
expect(result.current.hasSecondaryButton).toBe(true);
});

it('returns the expected values when props includes empty buttons', () => {
const props: TestComponentProps = {
layout: 'MIDDLE_BANNER',

primaryButton: {} as MessageButtonProps,
secondaryButton: {} as MessageButtonProps,
};

const { result } = renderHook(() => useMessageProps(props));

expect(result.current.hasButtons).toBe(false);
expect(result.current.hasPrimaryButton).toBe(false);
expect(result.current.hasSecondaryButton).toBe(false);
});

it('returns the expected values when props includes only a primary button', () => {
const props: TestComponentProps = {
layout: 'MIDDLE_BANNER',
primaryButton: { title: 'primary', onAction: jest.fn() },
};

const { result } = renderHook(() => useMessageProps(props));

expect(result.current.hasButtons).toBe(true);
expect(result.current.hasPrimaryButton).toBe(true);
expect(result.current.hasSecondaryButton).toBe(false);
});

it('returns the expected values when props includes only a secondary button', () => {
const props: TestComponentProps = {
layout: 'MIDDLE_BANNER',
secondaryButton: { title: 'primary', onAction: jest.fn() },
};

const { result } = renderHook(() => useMessageProps(props));

expect(result.current.hasButtons).toBe(true);
expect(result.current.hasPrimaryButton).toBe(false);
expect(result.current.hasSecondaryButton).toBe(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {
MessageComponentBaseProps,
MessageTextAlign,
} from '@aws-amplify/ui-react-core';
import { StyleParams } from '../types';

import { getPayloadStyle, getMessageStyles } from '../utils';

describe('getPayloadStyle', () => {
it('returns the expected output in the happy path', () => {
const output = getPayloadStyle({
body: { style: { textAlign: 'left' as MessageTextAlign } },
container: { style: { backgroundColor: 'lightgray', borderRadius: 2 } },
header: { style: { textAlign: 'center' as MessageTextAlign } },
primaryButton: { style: { backgroundColor: 'salmon', color: 'olive' } },
secondaryButton: { style: { backgroundColor: 'sand', color: 'peru' } },
} as MessageComponentBaseProps);

expect(output).toMatchSnapshot();
});
});

describe('getMessageStyles', () => {
const payloadStyle: StyleParams['payloadStyle'] = {
body: { textAlign: 'left' as MessageTextAlign },
container: { backgroundColor: 'lightgray', borderRadius: 2 },
header: { textAlign: 'center' as MessageTextAlign },
primaryButton: { backgroundColor: 'salmon', color: 'olive' },
secondaryButton: { backgroundColor: 'sand', color: 'peru' },
};

const overrideStyle = {
body: { color: 'white' },
closeIconButton: { backgroundColor: 'turquoise', color: 'darkcyan' },
container: { backgroundColor: 'lawngreen', borderRadius: 3 },
header: { backgroundColor: 'lightpink' },
image: { backgroundColor: 'royalblue' },
primaryButton: { backgroundColor: 'seagreen', color: 'black' },
secondaryButton: { backgroundColor: 'sienna', color: 'orchid' },
};

it('returns the expected output in the happy path', () => {
const output = getMessageStyles({
styleParams: { payloadStyle, overrideStyle },
});

expect(output).toMatchSnapshot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rather than rely on comparing the snapshot to the test code and relying on humans not to just update the snapshot, what do you think of expecting it to match an expectedStyle object we can read right here. That would make me more confident in the test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think part of why we just opted for snapshots here was it was already the case with the RN versions of these utils. Imho, we should lean on snapshot tests for a quick (and often large) tree output and trust that our contributors respect them and make updates only after they have validated the changes are intentional. Personally, I find that to be the exact same exercise as just updating some expectedStyle object but I do understand your concern. But I will add a task to re-evaluate this approach holistically and swap out snapshot tests all at once if the team is unconvinced 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair.

});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export { default as useMessageProps } from './useMessageProps';
export {
MessageComponentStyles,
MessageOverrideStyle,
UseMessageProps,
} from './types';
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { MessagePayloadStyle } from '@aws-amplify/ui-react-core';
// import { ImageDimensions } from '../useMessageImage';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we can delete this line, web useMessageImage does not return ImageDimensions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unused import


/**
* Inline style props for message components. Can be overridden by customer
*/
export interface MessageComponentStyles {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the styles provided from pinpoint, right? Should we add a comment describing why these styles are the only ones that can be overwritten?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't all coming from Pinpoint (e.g. image and close button styles aren't coming from payload). I think they represent (almost?) everything that is displayed in a message component, however, so we are actually allowing a lot of overriding via styles. Maybe the comment can be shored up (admittedly, it was copied from the React Native version of this hook)

body: React.CSSProperties;
closeIconButton: React.CSSProperties;
container: React.CSSProperties;
header: React.CSSProperties;
image: React.CSSProperties;
primaryButton: React.CSSProperties;
secondaryButton: React.CSSProperties;
}

export interface UseMessageProps {
hasButtons: boolean;
hasPrimaryButton: boolean;
hasRenderableImage: boolean;
hasSecondaryButton: boolean;
shouldRenderMessage: boolean;
styles: MessageComponentStyles | null;
}

export interface MessageStyleParams {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this extra interface necessary, or could getMessageStyles just take a single argument of type StyleParams? I guess the question is, are you expecting to receive add more properties here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A large part of doing this was simply to maintain parity with the React Native counterpart. We could scrap it and just inline the StyleParams typing in the function arguments but it is probably cleaner and more easily extensible (should we decide we do need to add more arguments especially as we add more complex components e.g. Carousel - even though I don't anticipate it yet).

/**
* style params to derive resolved style from
*/
styleParams: StyleParams;
}

export interface StyleParams {
/**
* message specific styles in the message payload
*/
payloadStyle: MessagePayloadStyle;

/**
* custom component style passed as style prop to message UI components
*/
overrideStyle: MessageOverrideStyle;
}

/**
* Override style props
*/
export type MessageOverrideStyle = Partial<MessageComponentStyles>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { useEffect, useMemo, useRef } from 'react';
import isEmpty from 'lodash/isEmpty';
import { MessageComponentBaseProps } from '@aws-amplify/ui-react-core';

// import { useMessageImage } from '../useMessageImage';
import { MessageOverrideStyle, UseMessageProps } from './types';
import { getPayloadStyle, getMessageStyles } from './utils';

/**
* Handle common message UI component prop logic including render booleans, and
* style resolving
*
* @param props message UI component props
*
* @returns message UI component render related booleans and styles
*/

export default function useMessageProps(
props: MessageComponentBaseProps<MessageOverrideStyle>
): UseMessageProps {
const { /*image*/ onDisplay, primaryButton, secondaryButton } = props;
const hasDisplayed = useRef(false);

// const { hasRenderableImage, imageDimensions, isImageFetching } =
// useMessageImage(image, layout);

// const shouldRenderMessage = !isImageFetching;
const hasRenderableImage = false;
const shouldRenderMessage = true;

useEffect(() => {
if (!hasDisplayed.current && shouldRenderMessage) {
onDisplay?.();
hasDisplayed.current = true;
}
}, [onDisplay, shouldRenderMessage]);

const hasPrimaryButton = !isEmpty(primaryButton);
const hasSecondaryButton = !isEmpty(secondaryButton);
const hasButtons = hasPrimaryButton || hasSecondaryButton;

const styles = useMemo(() => {
// prevent generating style if message rendering is delayed
if (!shouldRenderMessage) {
return null;
}

const payloadStyle = getPayloadStyle(props);
const overrideStyle = props.style;

return getMessageStyles({ styleParams: { payloadStyle, overrideStyle } });
}, [props, shouldRenderMessage]);

return {
hasButtons,
hasPrimaryButton,
hasRenderableImage,
hasSecondaryButton,
shouldRenderMessage,
styles,
};
}
Loading