-
Notifications
You must be signed in to change notification settings - Fork 338
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
Changes from all commits
a2fae85
97d0c00
b4bcef3
bbd8c95
8512099
0446497
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 |
---|---|---|
@@ -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(); | ||
}); | ||
}); |
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'; | ||
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. Think we can delete this line, web 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. nit: unused import |
||
|
||
/** | ||
* Inline style props for message components. Can be overridden by customer | ||
*/ | ||
export interface MessageComponentStyles { | ||
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. 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? 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. 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 { | ||
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. Is this extra interface necessary, or could 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. A large part of doing this was simply to maintain parity with the React Native counterpart. We could scrap it and just inline the |
||
/** | ||
* 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, | ||
}; | ||
} |
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.
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.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.
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 👍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.
That's fair.