Skip to content
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

feat(in-app-messaging): add BannerMessage UI component #9124

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright 2017-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
* and limitations under the License.
*/

import React from 'react';
import { Image, Text, View } from 'react-native';

import icons from '../../../icons';
import { ICON_BUTTON_HIT_SLOP, ICON_BUTTON_SIZE } from '../constants';
calebpollman marked this conversation as resolved.
Show resolved Hide resolved
import { useInAppMessageButtonStyle, useInAppMessageImage } from '../../hooks';
import { Button, IconButton } from '../../ui';

import { MessageWrapper } from '../MessageWrapper';
import { styles } from './styles';
import { BannerMessageProps } from './types';

export default function BannerMessage({
body,
container,
header,
image,
layout,
onClose,
position,
primaryButton,
secondaryButton,
style,
}: BannerMessageProps) {
const { imageStyle, shouldDelayMessageRendering, shouldRenderImage } = useInAppMessageImage(image, layout);
const { primaryButtonStyle, secondaryButtonStyle } = useInAppMessageButtonStyle({
baseButtonStyle: styles,
messageButtonStyle: { primaryButton: primaryButton?.style, secondaryButton: secondaryButton?.style },
calebpollman marked this conversation as resolved.
Show resolved Hide resolved
overrideButtonStyle: style,
});

return shouldDelayMessageRendering ? null : (
<MessageWrapper>
<View style={[styles.positionContainer, styles[position]]}>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: This is not blocking but I wonder if styles should be defaultStyle or something? I keep finding myself having to remind myself what the difference between styles and style is. As a reader of foreign code, I would have no idea why they're different things.

Copy link
Member Author

@calebpollman calebpollman Oct 28, 2021

Choose a reason for hiding this comment

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

imo default styling as styles is a pretty standard convention in React Native. That said i do think all the styling applied in these components is not the easiest to parse 😟

Copy link
Member

Choose a reason for hiding this comment

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

default styling as styles is a pretty standard convention in React Native

This is a good point too since static StyleSheets are typically assigned to a styles variable. It was just a thought

<View style={[styles.container, container?.style, style?.container]}>
<View style={styles.contentContainer}>
{shouldRenderImage && (
<View style={styles.imageContainer}>
<Image source={{ uri: image.src }} style={imageStyle} />
calebpollman marked this conversation as resolved.
Show resolved Hide resolved
</View>
)}
<View style={styles.textContainer}>
{header?.content && <Text style={[styles.header, header.style, style?.header]}>{header.content}</Text>}
{body?.content && <Text style={[styles.message, body.style, style?.message]}>{body.content}</Text>}
</View>
<IconButton
color={style?.closeIconColor}
hitSlop={ICON_BUTTON_HIT_SLOP}
onPress={onClose}
size={ICON_BUTTON_SIZE}
source={icons.close}
style={[styles.iconButton, style?.closeIconButton]}
/>
</View>
{primaryButton && (
<View style={styles.buttonsContainer}>
{secondaryButton && (
Copy link
Contributor

Choose a reason for hiding this comment

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

What happen if there is only secondaryButton?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally, the contract we have with pinpoint should prevent that from happening, but it seems reasonable to add handling for that scenario

<Button
onPress={secondaryButton.onPress}
style={secondaryButtonStyle.container}
textStyle={secondaryButtonStyle.text}
>
{secondaryButton.title}
</Button>
)}
<Button
onPress={primaryButton.onPress}
style={primaryButtonStyle.container}
textStyle={primaryButtonStyle.text}
>
{primaryButton.title}
</Button>
</View>
)}
</View>
</View>
</MessageWrapper>
);
}
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export { default } from './BannerMessage';
export { BannerMessageProps } from './types';
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Copyright 2017-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
* and limitations under the License.
*/

import { StyleSheet } from 'react-native';

import { getLineHeight } from '../utils';
import {
BANNER_ELEVATION,
BANNER_SHADOW_HEIGHT,
BANNER_SHADOW_OPACITY,
BANNER_SHADOW_RADIUS,
BANNER_SHADOW_WIDTH,
BASE_SPACING,
BASE_FONT_SIZE,
BASE_FONT_WEIGHT,
BLACK,
BUTTON_BORDER_RADIUS,
EXTRA_LARGE_SPACING,
LARGE_SPACING,
LARGE_FONT_SIZE,
LIGHT_GREY,
SMALL_SPACING,
WHITE,
} from '../constants';
import { BannerMessageStyle } from './types';

export const styles: BannerMessageStyle = StyleSheet.create({
// position style
positionContainer: {
flex: 1,
backgroundColor: 'transparent',
},
bottom: {
justifyContent: 'flex-end',
},
middle: {
justifyContent: 'center',
},
top: {
justifyContent: 'flex-start',
},

// shared style
buttonContainer: {
backgroundColor: LIGHT_GREY,
borderRadius: BUTTON_BORDER_RADIUS,
flex: 1,
margin: BASE_SPACING,
padding: LARGE_SPACING,
},
buttonsContainer: {
flexDirection: 'row',
justifyContent: 'center',
paddingHorizontal: SMALL_SPACING,
},
buttonText: {
fontSize: BASE_FONT_SIZE,
fontWeight: BASE_FONT_WEIGHT,
lineHeight: getLineHeight(BASE_FONT_SIZE),
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You could extract this to ../constants as well so that it doesn't need to be recalculated if it needs to be used elsewhere.

textAlign: 'center',
},
container: {
backgroundColor: WHITE,
elevation: BANNER_ELEVATION,
margin: EXTRA_LARGE_SPACING,
shadowColor: BLACK,
shadowOffset: {
width: BANNER_SHADOW_WIDTH,
height: BANNER_SHADOW_HEIGHT,
},
shadowOpacity: BANNER_SHADOW_OPACITY,
shadowRadius: BANNER_SHADOW_RADIUS,
},
contentContainer: {
flexDirection: 'row',
padding: LARGE_SPACING,
},
header: {
fontSize: LARGE_FONT_SIZE,
fontWeight: BASE_FONT_WEIGHT,
lineHeight: getLineHeight(LARGE_FONT_SIZE),
},
iconButton: {
alignSelf: 'flex-start',
marginLeft: 'auto',
},
imageContainer: {
justifyContent: 'center',
},
message: {
fontSize: BASE_FONT_SIZE,
fontWeight: BASE_FONT_WEIGHT,
lineHeight: getLineHeight(BASE_FONT_SIZE),
},
textContainer: {
marginHorizontal: SMALL_SPACING,
paddingLeft: BASE_SPACING,
flex: 1,
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,29 @@
* and limitations under the License.
*/

import { TextStyle, ViewStyle } from 'react-native';
import { InAppMessageComponentPosition, InAppMessageComponentBaseProps } from '../types';

export interface BannerMessageProps extends InAppMessageComponentBaseProps {
position: InAppMessageComponentPosition;
}

export interface BannerMessageStyle {
// position specific style
bottom: ViewStyle;
middle: ViewStyle;
top: ViewStyle;

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the grouping approach you did here :)

// component style
buttonContainer: ViewStyle;
buttonsContainer: ViewStyle;
buttonText: TextStyle;
container: ViewStyle;
contentContainer: ViewStyle;
header: TextStyle;
iconButton: ViewStyle;
imageContainer: ViewStyle;
message: TextStyle;
positionContainer: ViewStyle;
textContainer: ViewStyle;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright 2017-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
* and limitations under the License.
*/

import React from 'react';
import { SafeAreaView } from 'react-native';

import { styles } from './styles';
import { MessageWrapperProps } from './types';

export function MessageWrapper({ children, style }: MessageWrapperProps) {
return <SafeAreaView style={[styles.safeArea, style]}>{children}</SafeAreaView>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './MessageWrapper';
export * from './types';
calebpollman marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2017-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
* and limitations under the License.
*/

import { StyleSheet } from 'react-native';
import { MessageWrapperStyle } from './types';

export const styles: MessageWrapperStyle = StyleSheet.create({
safeArea: {
...StyleSheet.absoluteFillObject,
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright 2017-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
* and limitations under the License.
*/

import { ReactNode } from 'react';
import { StyleProp, ViewStyle } from 'react-native';

export interface MessageWrapperProps {
children: ReactNode;
style?: StyleProp<ViewStyle>;
}

export interface MessageWrapperStyle {
safeArea: ViewStyle;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright 2017-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
* the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
* and limitations under the License.
*/

/**
* Style constants either match or approximate the values used in the Pinpoint console preview.
* Some values, such as spacing, are slightly different to allow for a more mobile friendly UX
*/

// color
export const BLACK = '#000';
export const LIGHT_GREY = '#e8e8e8';
export const WHITE = '#fff';
Comment on lines +19 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'm not yet familiar with the React Native codebase's patterns, but in the future, it could be good to prefix the color consts with COLOR_ - i.e. COLOR_BLACK, COLOR_WHITE, COLOR_GREY_LIGHT (similar to BANNER_) in order to group the declarations closer together.


// spacing
export const SMALL_SPACING = 4;
export const BASE_SPACING = 8;
export const LARGE_SPACING = 12;
export const EXTRA_LARGE_SPACING = 16;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Consider switching this around - i.e. SPACING_X so developers can easily access spacing values rather than typing LARGE and then having to choose spacing vs fontsize etc. This could even be a SPACING object that contains different size properties.

Copy link
Member Author

@calebpollman calebpollman Oct 28, 2021

Choose a reason for hiding this comment

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

Agree with naming, but prefer to keep these explicit exports. Basically feel like putting them in an object implies they should be long lived instead of temporary


// button
export const BUTTON_BORDER_RADIUS = 4;

// font
export const BASE_FONT_SIZE = 16;
export const LARGE_FONT_SIZE = 18;
export const BASE_FONT_WEIGHT = '400';

export const LINE_HEIGHT_MULTIPLIER = 1.5;

// icon
export const ICON_BUTTON_SIZE = 20;
export const ICON_BUTTON_HIT_SLOP = 10;

// component specific constants

// BannerMessage
// iOS shadow values
export const BANNER_SHADOW_HEIGHT = 2;
export const BANNER_SHADOW_WIDTH = 2;
export const BANNER_SHADOW_OPACITY = 0.1;
export const BANNER_SHADOW_RADIUS = 2;

// android shadow values
export const BANNER_ELEVATION = 3;

// TODO: delete these once they are no longer needed
// pinpoint max chars 150
export const MAX_MESSAGE =
Copy link
Contributor

Choose a reason for hiding this comment

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

Why MAX_MESSAGE instead of MAX_MESSAGE_LENGTH?

Copy link
Member Author

Choose a reason for hiding this comment

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

No real reason, they are just for my testing

'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. Nullam dictum felis eu pede mollis pretium. Integer tincidunt. Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, viverra quis, feugiat a, tellus. Phasellus viverra nulla ut metus varius laoreet. Quisque rutrum. Aenean imperdiet. Etiam ultricies nisi vel augue. Curabitur ullamcorper ultricies nisi. Nam eget dui. Etiam rhoncus. Maecenas tempus, tellus eget condimentum rhoncus, sem quam semper libero, sit amet adipiscing sem neque sed ipsum. Nam quam nunc, blandit vel, luctus pulvinar,';

// pinpoint max chars 64
export const MAX_HEADER =
'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. Nullam dictum';

// pinpoint max chars 30
export const MAX_BUTTON =
'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis,';
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { BannerMessageProps } from './BannerMessage';
export { default as BannerMessage, BannerMessageProps } from './BannerMessage';
export { CarouselMessageProps } from './CarouselMessage';
export { FullScreenMessageProps } from './FullScreenMessage';
export { default as InAppMessageDisplay } from './InAppMessageDisplay';
Expand Down
Loading