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

Unify the distance rates enabled label styles with other feature row styles #39557

Merged
merged 6 commits into from
Apr 4, 2024
Merged

Unify the distance rates enabled label styles with other feature row styles #39557

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bef52de
Make sure the style for distance rates row is same as for categories …
mountiny Apr 3, 2024
83dd287
Fix lint
mountiny Apr 3, 2024
2252271
Create a new component for the right element
mountiny Apr 3, 2024
4290391
Create a new component for the right element
mountiny Apr 3, 2024
be00a70
Merge branch 'main' into vit-39556
mountiny Apr 4, 2024
ca03fd1
Address PR feedback
mountiny Apr 4, 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
36 changes: 36 additions & 0 deletions src/components/SelectionList/ListItemRightElementWithArrow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React from 'react';
import {View} from 'react-native';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';

type ListItemRightElementWithArrowProps = {
enabled?: boolean;
};

function ListItemRightElementWithArrow({enabled}: ListItemRightElementWithArrowProps) {
const styles = useThemeStyles();
const theme = useTheme();
const {translate} = useLocalize();

return (
<View style={styles.flexRow}>
<Text style={[styles.alignSelfCenter, styles.textSupporting, styles.pl2, styles.label]}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I am no engineer but just noting that not all of these table rows have the enabled/disabled text, right? Sometimes they might have nothing, or sometimes they might have a badge (Owner, Admin, card limit, etc). Not sure how that might change this abstraction.

Copy link
Contributor

Choose a reason for hiding this comment

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

@shawnborton You're right that it can be a badge in other use cases. But this component will be only used by rows (of different components) to show the enabled status.

Maybe rename it to ListItemRightElementEnabledStatus or RightElementEnabledStatus sounds better?

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 so for this case, I have only abstracted the component used in the Categories, Tags, Taxes and Distance rates, The other usecases might have different styles or even different components inside as you mentioned.

Abstracting this more might lead to loosing the benefit in this case as we might have to pass more props and styles which would again have to be defined individually in pages/ components this is used and it could lead to different styles.

I can rename this based on @eh2077 suggestion

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, I don't feel too strongly really - mostly just noting that we have different row behaviors in various places. Thanks for hearing me out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can certainly see us further abstracting this especially for the Badge style - Members with roles, Cards with limits etc. I think we can do that as we go though

{enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')}
</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
</View>
);
}

ListItemRightElementWithArrow.displayName = 'ListItemRightElementWithArrow';

export default ListItemRightElementWithArrow;
2 changes: 0 additions & 2 deletions src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2073,8 +2073,6 @@ export default {
disableRates: ({count}: DistanceRateOperationsParams) => `Disable ${Str.pluralize('rate', 'rates', count)}`,
enableRate: 'Enable rate',
status: 'Status',
enabled: 'Enabled',
disabled: 'Disabled',
unit: 'Unit',
defaultCategory: 'Default category',
deleteDistanceRate: 'Delete distance rate',
Expand Down
2 changes: 0 additions & 2 deletions src/languages/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2101,8 +2101,6 @@ export default {
disableRates: ({count}: DistanceRateOperationsParams) => `Desactivar ${Str.pluralize('tasa', 'tasas', count)}`,
enableRate: 'Activar tasa',
status: 'Estado',
enabled: 'Activada',
disabled: 'Desactivada',
unit: 'Unidad',
defaultCategory: 'Categoría predeterminada',
deleteDistanceRate: 'Eliminar tasa de distancia',
Expand Down
30 changes: 3 additions & 27 deletions src/pages/workspace/categories/WorkspaceCategoriesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import ButtonWithDropdownMenu from '@components/ButtonWithDropdownMenu';
import type {DropdownOption} from '@components/ButtonWithDropdownMenu/types';
import ConfirmModal from '@components/ConfirmModal';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import ListItemRightElementWithArrow from '@components/SelectionList/ListItemRightElementWithArrow';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import Text from '@components/Text';
Expand Down Expand Up @@ -88,34 +88,10 @@ function WorkspaceCategoriesPage({policy, policyCategories, route}: WorkspaceCat
isDisabled,
pendingAction: value.pendingAction,
errors: value.errors ?? undefined,
rightElement: (
<View style={[styles.flexRow, isDisabled && styles.buttonOpacityDisabled]}>
<Text style={[styles.textSupporting, styles.alignSelfCenter, styles.pl2, styles.label]}>
{value.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')}
</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
</View>
),
rightElement: <ListItemRightElementWithArrow enabled={value.enabled} />,
};
}),
[
policyCategories,
selectedCategories,
styles.alignSelfCenter,
styles.buttonOpacityDisabled,
styles.flexRow,
styles.label,
styles.p1,
styles.pl2,
styles.textSupporting,
theme.icon,
translate,
],
[policyCategories, selectedCategories],
);

const toggleCategory = (category: PolicyOption) => {
Expand Down
18 changes: 3 additions & 15 deletions src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import Button from '@components/Button';
import type {DropdownOption, WorkspaceDistanceRatesBulkActionType} from '@components/ButtonWithDropdownMenu/types';
import ConfirmModal from '@components/ConfirmModal';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import ListItemRightElementWithArrow from '@components/SelectionList/ListItemRightElementWithArrow';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import Text from '@components/Text';
Expand Down Expand Up @@ -104,21 +104,9 @@ function PolicyDistanceRatesPage({policy, route}: PolicyDistanceRatesPageProps)
isDisabled: value.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
pendingAction: value.pendingAction ?? value.pendingFields?.rate ?? value.pendingFields?.enabled ?? value.pendingFields?.currency,
errors: value.errors ?? undefined,
rightElement: (
<View style={styles.flexRow}>
<Text style={[styles.alignSelfCenter, !value.enabled && styles.textSupporting]}>
{value.enabled ? translate('workspace.distanceRates.enabled') : translate('workspace.distanceRates.disabled')}
</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
</View>
),
rightElement: <ListItemRightElementWithArrow enabled={value.enabled} />,
})),
[customUnit?.attributes?.unit, customUnitRates, selectedDistanceRates, styles.alignSelfCenter, styles.flexRow, styles.p1, styles.pl2, styles.textSupporting, theme.icon, translate],
[customUnit?.attributes?.unit, customUnitRates, selectedDistanceRates, translate],
);

const addRate = () => {
Expand Down
18 changes: 3 additions & 15 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import ButtonWithDropdownMenu from '@components/ButtonWithDropdownMenu';
import type {DropdownOption} from '@components/ButtonWithDropdownMenu/types';
import ConfirmModal from '@components/ConfirmModal';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import ListItemRightElementWithArrow from '@components/SelectionList/ListItemRightElementWithArrow';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import Text from '@components/Text';
Expand Down Expand Up @@ -98,24 +98,12 @@ function WorkspaceTagsPage({policyTags, route}: WorkspaceTagsPageProps) {
errors: tag.errors ?? undefined,
enabled: tag.enabled,
isDisabled,
rightElement: (
<View style={styles.flexRow}>
<Text style={[styles.textSupporting, styles.alignSelfCenter, styles.pl2, styles.label]}>
{tag.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')}
</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
</View>
),
rightElement: <ListItemRightElementWithArrow enabled={tag.enabled} />,
};
}),
)
.flat(),
[policyTagLists, selectedTags, styles.alignSelfCenter, styles.flexRow, styles.label, styles.p1, styles.pl2, styles.textSupporting, theme.icon, translate],
[policyTagLists, selectedTags, translate],
);

const tagListKeyedByName = tagList.reduce<Record<string, PolicyForList>>((acc, tag) => {
Expand Down
16 changes: 3 additions & 13 deletions src/pages/workspace/taxes/WorkspaceTaxesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import ButtonWithDropdownMenu from '@components/ButtonWithDropdownMenu';
import type {DropdownOption, WorkspaceTaxRatesBulkActionType} from '@components/ButtonWithDropdownMenu/types';
import ConfirmModal from '@components/ConfirmModal';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import ListItemRightElementWithArrow from '@components/SelectionList/ListItemRightElementWithArrow';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import Text from '@components/Text';
Expand Down Expand Up @@ -95,20 +95,10 @@ function WorkspaceTaxesPage({
isDisabled: value.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
pendingAction: value.pendingAction ?? (Object.keys(value.pendingFields ?? {}).length > 0 ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : null),
errors: value.errors ?? ErrorUtils.getLatestErrorFieldForAnyField(value),
rightElement: (
<View style={styles.flexRow}>
<Text style={[styles.disabledText, styles.alignSelfCenter]}>{value.isDisabled ? translate('workspace.common.disabled') : translate('workspace.common.enabled')}</Text>
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
</View>
),
rightElement: <ListItemRightElementWithArrow enabled={!value.isDisabled} />,
}))
.sort((a, b) => (a.text ?? a.keyForList ?? '').localeCompare(b.text ?? b.keyForList ?? ''));
}, [policy, textForDefault, selectedTaxesIDs, styles.flexRow, styles.disabledText, styles.alignSelfCenter, styles.p1, styles.pl2, translate, theme.icon]);
}, [policy, textForDefault, selectedTaxesIDs, translate]);

const isLoading = !isOffline && taxesList === undefined;

Expand Down
Loading