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

Refactor the error text style + add new hint prop to MenuItem component #41670

Merged
merged 11 commits into from
May 13, 2024
+280 −297
Merged

Refactor the error text style + add new hint prop to MenuItem component #41670

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7fd48ee
Menu item changes
narefyev91 Apr 30, 2024
f8f536c
updates for styles
narefyev91 Apr 30, 2024
232163f
Merge branch 'refs/heads/main' into menu-item-changes
narefyev91 May 6, 2024
bd6595f
Refactoring all places to use errorText instead of error for menu items
narefyev91 May 6, 2024
fc6455c
update after design review
narefyev91 May 7, 2024
c96c61a
remove redundant error prop
narefyev91 May 8, 2024
505a1a6
Merge branch 'refs/heads/main' into menu-item-changes
narefyev91 May 8, 2024
22556b5
updates after merge conflicts
narefyev91 May 8, 2024
99ae6f0
updates after design review
narefyev91 May 8, 2024
3abce2c
Merge branch 'refs/heads/main' into menu-item-changes
narefyev91 May 9, 2024
730f272
updates after c+ review
narefyev91 May 9, 2024
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
11 changes: 4 additions & 7 deletions src/components/AmountPicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import React, {forwardRef, useState} from 'react';
import type {ForwardedRef} from 'react';
import {View} from 'react-native';
import FormHelpMessage from '@components/FormHelpMessage';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import callOrReturn from '@src/types/utils/callOrReturn';
import AmountSelectorModal from './AmountSelectorModal';
import type {AmountPickerProps} from './types';

function AmountPicker({value, description, title, errorText = '', onInputChange, furtherDetails, rightLabel, ...rest}: AmountPickerProps, forwardedRef: ForwardedRef<View>) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const [isPickerVisible, setIsPickerVisible] = useState(false);

Expand Down Expand Up @@ -43,11 +41,10 @@ function AmountPicker({value, description, title, errorText = '', onInputChange,
description={description}
onPress={showPickerModal}
furtherDetails={furtherDetails}
rightLabel={rightLabel}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
rightLabel={!errorText ? rightLabel : undefined}
errorText={errorText}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
<AmountSelectorModal
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
Expand Down
35 changes: 16 additions & 19 deletions src/components/CountrySelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {useIsFocused} from '@react-navigation/native';
import React, {forwardRef, useEffect, useRef} from 'react';
import type {ForwardedRef} from 'react';
import {View} from 'react-native';
import type {View} from 'react-native';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import type {MaybePhraseKey} from '@libs/Localize';
import Navigation from '@libs/Navigation/Navigation';
import CONST from '@src/CONST';
import type {Country} from '@src/CONST';
import ROUTES from '@src/ROUTES';
import FormHelpMessage from './FormHelpMessage';
import MenuItemWithTopDescription from './MenuItemWithTopDescription';

type CountrySelectorProps = {
Expand Down Expand Up @@ -53,23 +53,20 @@ function CountrySelector({errorText = '', value: countryCode, onInputChange = ()
}, [countryCode]);

return (
<View>
<MenuItemWithTopDescription
shouldShowRightIcon
title={title}
ref={ref}
descriptionTextStyle={countryTitleDescStyle}
description={translate('common.country')}
onPress={() => {
const activeRoute = Navigation.getActiveRouteWithoutParams();
didOpenContrySelector.current = true;
Navigation.navigate(ROUTES.SETTINGS_ADDRESS_COUNTRY.getRoute(countryCode ?? '', activeRoute));
}}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
</View>
<MenuItemWithTopDescription
shouldShowRightIcon
title={title}
ref={ref}
descriptionTextStyle={countryTitleDescStyle}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
description={translate('common.country')}
errorText={errorText}
onPress={() => {
const activeRoute = Navigation.getActiveRouteWithoutParams();
didOpenContrySelector.current = true;
Navigation.navigate(ROUTES.SETTINGS_ADDRESS_COUNTRY.getRoute(countryCode ?? '', activeRoute));
}}
/>
);
}

Expand Down
394 changes: 213 additions & 181 deletions src/components/MenuItem.tsx
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need both error and errorText? This is redundant, can we remove one of these props?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i will remove error props - it's not in correct position for menu item

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/components/MoneyRequestConfirmationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ function MoneyRequestConfirmationList({
titleStyle={styles.moneyRequestConfirmationAmount}
disabled={didConfirm}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isAmountMissing(transaction ?? null) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={shouldDisplayFieldError && TransactionUtils.isAmountMissing(transaction ?? null) ? translate('common.error.enterAmount') : ''}
errorText={shouldDisplayFieldError && TransactionUtils.isAmountMissing(transaction ?? null) ? translate('common.error.enterAmount') : ''}
/>
),
shouldShow: shouldShowSmartScanFields,
Expand Down Expand Up @@ -826,8 +826,8 @@ function MoneyRequestConfirmationList({
disabled={didConfirm}
interactive={!isReadOnly}
brickRoadIndicator={shouldDisplayMerchantError ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={shouldDisplayMerchantError ? translate('common.error.fieldRequired') : ''}
rightLabel={isMerchantRequired ? translate('common.required') : ''}
errorText={shouldDisplayMerchantError ? translate('common.error.fieldRequired') : ''}
rightLabel={isMerchantRequired && !shouldDisplayMerchantError ? translate('common.required') : ''}
/>
),
shouldShow: shouldShowMerchant,
Expand All @@ -849,7 +849,7 @@ function MoneyRequestConfirmationList({
disabled={didConfirm}
interactive={!isReadOnly}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}
errorText={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}
/>
),
shouldShow: shouldShowDate,
Expand Down
14 changes: 7 additions & 7 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ function MoneyRequestView({
shouldShowRightIcon={canEditAmount}
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_AMOUNT.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))}
brickRoadIndicator={getErrorForField('amount') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('amount')}
errorText={getErrorForField('amount')}
/>
</OfflineWithFeedback>
<OfflineWithFeedback pendingAction={getPendingFieldAction('comment')}>
Expand All @@ -420,7 +420,7 @@ function MoneyRequestView({
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))}
wrapperStyle={[styles.pv2, styles.taskDescriptionMenuItem]}
brickRoadIndicator={getErrorForField('comment') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('comment')}
errorText={getErrorForField('comment')}
numberOfLinesTitle={0}
/>
</OfflineWithFeedback>
Expand All @@ -438,7 +438,7 @@ function MoneyRequestView({
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_MERCHANT.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))
}
brickRoadIndicator={getErrorForField('merchant') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('merchant')}
errorText={getErrorForField('merchant')}
/>
</OfflineWithFeedback>
)}
Expand All @@ -451,7 +451,7 @@ function MoneyRequestView({
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DATE.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))}
brickRoadIndicator={getErrorForField('date') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('date')}
errorText={getErrorForField('date')}
/>
</OfflineWithFeedback>
{shouldShowCategory && (
Expand All @@ -466,7 +466,7 @@ function MoneyRequestView({
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_CATEGORY.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))
}
brickRoadIndicator={getErrorForField('category') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('category')}
errorText={getErrorForField('category')}
/>
</OfflineWithFeedback>
)}
Expand Down Expand Up @@ -495,7 +495,7 @@ function MoneyRequestView({
? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR
: undefined
}
error={getErrorForField('tag', {tagListIndex: index, tagListName: name})}
errorText={getErrorForField('tag', {tagListIndex: index, tagListName: name})}
/>
</OfflineWithFeedback>
))}
Expand All @@ -521,7 +521,7 @@ function MoneyRequestView({
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_TAX_RATE.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))
}
brickRoadIndicator={getErrorForField('tax') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('tax')}
errorText={getErrorForField('tax')}
/>
</OfflineWithFeedback>
)}
Expand Down
41 changes: 19 additions & 22 deletions src/components/StateSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import {useIsFocused} from '@react-navigation/native';
import {CONST as COMMON_CONST} from 'expensify-common/lib/CONST';
import React, {useEffect, useRef} from 'react';
import type {ForwardedRef} from 'react';
import {View} from 'react-native';
import type {View} from 'react-native';
import useGeographicalStateFromRoute from '@hooks/useGeographicalStateFromRoute';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import type {MaybePhraseKey} from '@libs/Localize';
import Navigation from '@libs/Navigation/Navigation';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import FormHelpMessage from './FormHelpMessage';
import type {MenuItemProps} from './MenuItem';
import MenuItemWithTopDescription from './MenuItemWithTopDescription';

Expand Down Expand Up @@ -79,26 +79,23 @@ function StateSelector(
const descStyle = title.length === 0 ? styles.textNormal : null;

return (
<View>
<MenuItemWithTopDescription
descriptionTextStyle={descStyle}
ref={ref}
shouldShowRightIcon
title={title}
// Label can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
description={label || translate('common.state')}
onPress={() => {
const activeRoute = Navigation.getActiveRoute();
didOpenStateSelector.current = true;
Navigation.navigate(stateSelectorRoute.getRoute(stateCode, activeRoute, label));
}}
wrapperStyle={wrapperStyle}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
</View>
<MenuItemWithTopDescription
descriptionTextStyle={descStyle}
ref={ref}
shouldShowRightIcon
title={title}
// Label can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
description={label || translate('common.state')}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
errorText={errorText}
onPress={() => {
const activeRoute = Navigation.getActiveRoute();
didOpenStateSelector.current = true;
Navigation.navigate(stateSelectorRoute.getRoute(stateCode, activeRoute, label));
}}
wrapperStyle={wrapperStyle}
/>
);
}

Expand Down
9 changes: 4 additions & 5 deletions src/components/TextPicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React, {forwardRef, useState} from 'react';
import type {ForwardedRef} from 'react';
import {View} from 'react-native';
import FormHelpMessage from '@components/FormHelpMessage';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import TextSelectorModal from './TextSelectorModal';
import type {TextPickerProps} from './types';

Expand Down Expand Up @@ -41,12 +41,11 @@ function TextPicker({value, description, placeholder = '', errorText = '', onInp
description={description}
onPress={showPickerModal}
furtherDetails={furtherDetails}
rightLabel={rightLabel}
rightLabel={!errorText ? rightLabel : undefined}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
errorText={errorText}
style={[styles.moneyRequestMenuItem]}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
<TextSelectorModal
value={value}
isVisible={isPickerVisible}
Expand Down
9 changes: 3 additions & 6 deletions src/components/ValuePicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import React, {forwardRef, useState} from 'react';
import type {ForwardedRef} from 'react';
import {View} from 'react-native';
import FormHelpMessage from '@components/FormHelpMessage';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import type {ValuePickerItem, ValuePickerProps} from './types';
import ValueSelectorModal from './ValueSelectorModal';

function ValuePicker({value, label, items, placeholder = '', errorText = '', onInputChange, furtherDetails, shouldShowTooltips = true}: ValuePickerProps, forwardedRef: ForwardedRef<View>) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const [isPickerVisible, setIsPickerVisible] = useState(false);

Expand Down Expand Up @@ -44,10 +42,9 @@ function ValuePicker({value, label, items, placeholder = '', errorText = '', onI
description={label}
onPress={showPickerModal}
furtherDetails={furtherDetails}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
errorText={errorText}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
<ValueSelectorModal
isVisible={isPickerVisible}
label={label}
Expand Down
7 changes: 3 additions & 4 deletions ...ages/ReimbursementAccount/BusinessInfo/substeps/TypeBusiness/BusinessTypePicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React, {useState} from 'react';
import type {StyleProp, ViewStyle} from 'react-native';
import {View} from 'react-native';
import FormHelpMessage from '@components/FormHelpMessage';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import BusinessTypeSelectorModal from './BusinessTypeSelectorModal';
import type {BusinessTypeItemType, IncorporationType} from './types';

Expand Down Expand Up @@ -65,11 +65,10 @@ function BusinessTypePicker({errorText = '', value = '', wrapperStyle, onInputCh
description={label}
descriptionTextStyle={descStyle}
onPress={showPickerModal}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
errorText={errorText}
wrapperStyle={wrapperStyle}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
<BusinessTypeSelectorModal
isVisible={isPickerVisible}
currentBusinessType={value}
Expand Down
5 changes: 1 addition & 4 deletions src/pages/workspace/WorkspaceProfilePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import * as Illustrations from '@components/Icon/Illustrations';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import Section from '@components/Section';
import Text from '@components/Text';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useLocalize from '@hooks/useLocalize';
import usePermissions from '@hooks/usePermissions';
Expand Down Expand Up @@ -215,10 +214,8 @@ function WorkspaceProfilePage({policy, currencyList = {}, route}: WorkSpaceProfi
onPress={onPressCurrency}
shouldGreyOutWhenDisabled={false}
shouldUseDefaultCursorWhenDisabled
hintText={hasVBA ? translate('workspace.editor.currencyInputDisabledText') : translate('workspace.editor.currencyInputHelpText')}
/>
<Text style={[styles.textLabel, styles.colorMuted, styles.mt1, styles.mh5, styles.sectionMenuItemTopDescription]}>
{hasVBA ? translate('workspace.editor.currencyInputDisabledText') : translate('workspace.editor.currencyInputHelpText')}
</Text>
</View>
</OfflineWithFeedback>
{canUseSpotnanaTravel && (
Expand Down
Loading
Loading