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.

Sign up for GitHub

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

Jump to bottom

[TS Migration] Style fixes in Text.tsx #30497

Merged
merged 2 commits into from
Oct 27, 2023
Merged

[TS Migration] Style fixes in Text.tsx #30497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81a05f8
Style fixes in Text.tsx
BartoszGrajdek Oct 27, 2023
9c0efa5
Fix tests
BartoszGrajdek Oct 27, 2023
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
24 changes: 4 additions & 20 deletions src/components/Text.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,62 +1,46 @@
import React, {ForwardedRef} from 'react';
// eslint-disable-next-line no-restricted-imports
import {Text as RNText} from 'react-native';
import {Text as RNText, TextProps as RNTextProps, StyleSheet} from 'react-native';
import type {TextStyle} from 'react-native';
import fontFamily from '../styles/fontFamily';
import themeColors from '../styles/themes/default';
import variables from '../styles/variables';

type TextProps = {
type TextProps = RNTextProps & {
/** The color of the text */
color?: string;

/** The size of the text */
fontSize?: number;

/** The alignment of the text */
textAlign?: 'left' | 'right' | 'auto' | 'center' | 'justify';

/** Any children to display */
children: React.ReactNode;

/** The family of the font to use */
family?: keyof typeof fontFamily;

/** Any additional styles to apply */
style?: TextStyle | TextStyle[];
};

function Text(
{color = themeColors.text, fontSize = variables.fontSizeNormal, textAlign = 'left', children = null, family = 'EXP_NEUE', style = {}, ...props}: TextProps,
ref: ForwardedRef<RNText>,
) {
// If the style prop is an array of styles, we need to mix them all together
const mergedStyles = !Array.isArray(style)
? style
: style.reduce(
(finalStyles, s) => ({
...finalStyles,
...s,
}),
{},
);
const componentStyle: TextStyle = {
color,
fontSize,
textAlign,
fontFamily: fontFamily[family],
...mergedStyles,
...StyleSheet.flatten(style),
};

if (!componentStyle.lineHeight && componentStyle.fontSize === variables.fontSizeNormal) {
componentStyle.lineHeight = variables.fontSizeNormalHeight;
}

return (
<RNText
allowFontScaling={false}
ref={ref}
style={[componentStyle]}
style={componentStyle}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
>
Expand Down
25 changes: 7 additions & 18 deletions tests/ui/UnreadIndicatorsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ describe('Unread Indicators', () => {
// And that the text is bold
const displayNameHintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNameText = screen.queryByLabelText(displayNameHintText);
expect(lodashGet(displayNameText, ['props', 'style', 0, 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(displayNameText, ['props', 'style', 'fontWeight'])).toBe(fontWeightBold);

return navigateToSidebarOption(0);
})
Expand All @@ -233,20 +233,17 @@ describe('Unread Indicators', () => {
const reportCommentsHintText = Localize.translateLocal('accessibilityHints.chatMessage');
const reportComments = screen.queryAllByLabelText(reportCommentsHintText);
expect(reportComments).toHaveLength(9);

// Since the last read timestamp is the timestamp of action 3 we should have an unread indicator above the next "unread" action which will
// have actionID of 4
const newMessageLineIndicatorHintText = Localize.translateLocal('accessibilityHints.newMessageLineIndicator');
const unreadIndicator = screen.queryAllByLabelText(newMessageLineIndicatorHintText);
expect(unreadIndicator).toHaveLength(1);
const reportActionID = lodashGet(unreadIndicator, [0, 'props', 'data-action-id']);
expect(reportActionID).toBe('4');

// Scroll up and verify that the "New messages" badge appears
scrollUpToRevealNewMessagesBadge();
return waitFor(() => expect(isNewMessagesBadgeVisible()).toBe(true));
}));

it('Clear the new line indicator and bold when we navigate away from a chat that is now read', () =>
signInAndGetAppWithUnreadChat()
// Navigate to the unread chat from the sidebar
Expand All @@ -255,14 +252,12 @@ describe('Unread Indicators', () => {
.then(() => navigateToSidebarOption(0))
.then(() => {
expect(areYouOnChatListScreen()).toBe(false);

// Then navigate back to the sidebar
return navigateToSidebar();
})
.then(() => {
// Verify the LHN is now open
expect(areYouOnChatListScreen()).toBe(true);

// Tap on the chat again
return navigateToSidebarOption(0);
})
Expand All @@ -271,7 +266,6 @@ describe('Unread Indicators', () => {
const newMessageLineIndicatorHintText = Localize.translateLocal('accessibilityHints.newMessageLineIndicator');
const unreadIndicator = screen.queryAllByLabelText(newMessageLineIndicatorHintText);
expect(unreadIndicator).toHaveLength(0);

// Tap on the chat again
return navigateToSidebarOption(0);
})
Expand All @@ -282,7 +276,6 @@ describe('Unread Indicators', () => {
expect(unreadIndicator).toHaveLength(0);
expect(areYouOnChatListScreen()).toBe(false);
}));

it('Shows a browser notification and bold text when a new message arrives for a chat that is read', () =>
signInAndGetAppWithUnreadChat()
.then(() => {
Expand All @@ -291,7 +284,6 @@ describe('Unread Indicators', () => {
const NEW_REPORT_ID = '2';
const NEW_REPORT_CREATED_DATE = subSeconds(new Date(), 5);
const NEW_REPORT_FIST_MESSAGE_CREATED_DATE = addSeconds(NEW_REPORT_CREATED_DATE, 1);

const createdReportActionID = NumberUtils.rand64();
const commentReportActionID = NumberUtils.rand64();
const channel = Pusher.getChannel(`${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}${USER_A_ACCOUNT_ID}${CONFIG.PUSHER.SUFFIX}`);
Expand Down Expand Up @@ -354,17 +346,16 @@ describe('Unread Indicators', () => {
const optionRowsHintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
const optionRows = screen.queryAllByAccessibilityHint(optionRowsHintText);
expect(optionRows).toHaveLength(2);

// Verify the text for both chats are bold indicating that nothing has not yet been read
const displayNameHintTexts = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNameTexts = screen.queryAllByLabelText(displayNameHintTexts);
expect(displayNameTexts).toHaveLength(2);
const firstReportOption = displayNameTexts[0];
expect(lodashGet(firstReportOption, ['props', 'style', 0, 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(firstReportOption, ['props', 'style', 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(firstReportOption, ['props', 'children'])).toBe('C User');

const secondReportOption = displayNameTexts[1];
expect(lodashGet(secondReportOption, ['props', 'style', 0, 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(secondReportOption, ['props', 'style', 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(secondReportOption, ['props', 'children'])).toBe('B User');

// Tap the new report option and navigate back to the sidebar again via the back button
Expand All @@ -376,9 +367,9 @@ describe('Unread Indicators', () => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNameTexts = screen.queryAllByLabelText(hintText);
expect(displayNameTexts).toHaveLength(2);
expect(lodashGet(displayNameTexts[0], ['props', 'style', 0, 'fontWeight'])).toBe(undefined);
expect(lodashGet(displayNameTexts[0], ['props', 'style', 'fontWeight'])).toBe(undefined);
expect(lodashGet(displayNameTexts[0], ['props', 'children'])).toBe('C User');
expect(lodashGet(displayNameTexts[1], ['props', 'style', 0, 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(displayNameTexts[1], ['props', 'style', 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(displayNameTexts[1], ['props', 'children'])).toBe('B User');
}));

Expand All @@ -399,20 +390,18 @@ describe('Unread Indicators', () => {
expect(unreadIndicator).toHaveLength(1);
const reportActionID = lodashGet(unreadIndicator, [0, 'props', 'data-action-id']);
expect(reportActionID).toBe('3');

// Scroll up and verify the new messages badge appears
scrollUpToRevealNewMessagesBadge();
return waitFor(() => expect(isNewMessagesBadgeVisible()).toBe(true));
})

// Navigate to the sidebar
.then(navigateToSidebar)
.then(() => {
// Verify the report is marked as unread in the sidebar
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNameTexts = screen.queryAllByLabelText(hintText);
expect(displayNameTexts).toHaveLength(1);
expect(lodashGet(displayNameTexts[0], ['props', 'style', 0, 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(displayNameTexts[0], ['props', 'style', 'fontWeight'])).toBe(fontWeightBold);
expect(lodashGet(displayNameTexts[0], ['props', 'children'])).toBe('B User');

// Navigate to the report again and back to the sidebar
Expand All @@ -424,7 +413,7 @@ describe('Unread Indicators', () => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNameTexts = screen.queryAllByLabelText(hintText);
expect(displayNameTexts).toHaveLength(1);
expect(lodashGet(displayNameTexts[0], ['props', 'style', 0, 'fontWeight'])).toBe(undefined);
expect(lodashGet(displayNameTexts[0], ['props', 'style', 'fontWeight'])).toBe(undefined);
expect(lodashGet(displayNameTexts[0], ['props', 'children'])).toBe('B User');

// Navigate to the report again and verify the new line indicator is missing
Expand Down
Loading