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

Add toggle for enable/disable instead of label #54451

Merged
merged 4 commits into from
Jan 9, 2025
Merged

Add toggle for enable/disable instead of label #54451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f1ef671
Add toggle for enable/disable instead of label
abzokhattab Dec 22, 2024
6dd35a2
main
abzokhattab Dec 31, 2024
771069b
Merge remote-tracking branch 'origin/main' into add-toggle-for-enable…
abzokhattab Jan 9, 2025
a5d13c8
fixing eslint errs
abzokhattab Jan 9, 2025
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
1 change: 1 addition & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,7 @@ const CONST = {
EMPTY_ARRAY,
EMPTY_OBJECT,
DEFAULT_NUMBER_ID: 0,
EMPTY_STRING: '',
USE_EXPENSIFY_URL,
EXPENSIFY_URL,
GOOGLE_MEET_URL_ANDROID: 'https://meet.google.com',
Expand Down
22 changes: 18 additions & 4 deletions src/pages/workspace/categories/WorkspaceCategoriesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import LottieAnimations from '@components/LottieAnimations';
import ScreenWrapper from '@components/ScreenWrapper';
import ListItemRightCaretWithLabel from '@components/SelectionList/ListItemRightCaretWithLabel';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import SelectionListWithModal from '@components/SelectionListWithModal';
import CustomListHeader from '@components/SelectionListWithModal/CustomListHeader';
import TableListItemSkeleton from '@components/Skeletons/TableRowSkeleton';
import Switch from '@components/Switch';
import Text from '@components/Text';
import TextLink from '@components/TextLink';
import useAutoTurnSelectionModeOffWhenHasNoActiveOption from '@hooks/useAutoTurnSelectionModeOffWhenHasNoActiveOption';
Expand All @@ -42,8 +42,8 @@ import type {FullScreenNavigatorParamList} from '@libs/Navigation/types';
import * as PolicyUtils from '@libs/PolicyUtils';
import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper';
import * as Modal from '@userActions/Modal';
import {deleteWorkspaceCategories, setWorkspaceCategoryEnabled} from '@userActions/Policy/Category';
import * as Category from '@userActions/Policy/Category';
import {deleteWorkspaceCategories, setWorkspaceCategoryEnabled} from '@userActions/Policy/Category';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -105,6 +105,13 @@ function WorkspaceCategoriesPage({route}: WorkspaceCategoriesPageProps) {
setSelectedCategories({});
}, [isFocused]);

const updateWorkspaceRequiresCategory = useCallback(
(value: boolean, categoryName: string) => {
Category.setWorkspaceCategoryEnabled(policyId, {[categoryName]: {name: categoryName, enabled: value}});
},
[policyId],
);

const categoryList = useMemo<PolicyOption[]>(() => {
const categories = lodashSortBy(Object.values(policyCategories ?? {}), 'name', localeCompare) as PolicyCategory[];
return categories.reduce<PolicyOption[]>((acc, value) => {
Expand All @@ -121,12 +128,19 @@ function WorkspaceCategoriesPage({route}: WorkspaceCategoriesPageProps) {
isDisabled,
pendingAction: value.pendingAction,
errors: value.errors ?? undefined,
rightElement: <ListItemRightCaretWithLabel labelText={value.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')} />,
rightElement: (
<Switch
isOn={value.enabled}
disabled={isDisabled}
accessibilityLabel={translate('workspace.categories.enableCategory')}
onToggle={(newValue: boolean) => updateWorkspaceRequiresCategory(newValue, value.name)}
/>
),
});

return acc;
}, []);
}, [policyCategories, isOffline, selectedCategories, canSelectMultiple, translate]);
}, [policyCategories, isOffline, selectedCategories, canSelectMultiple, translate, updateWorkspaceRequiresCategory]);

useAutoTurnSelectionModeOffWhenHasNoActiveOption(categoryList);

Expand Down
43 changes: 35 additions & 8 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 HeaderWithBackButton from '@components/HeaderWithBackButton';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import ListItemRightCaretWithLabel from '@components/SelectionList/ListItemRightCaretWithLabel';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import SelectionListWithModal from '@components/SelectionListWithModal';
import CustomListHeader from '@components/SelectionListWithModal/CustomListHeader';
import Switch from '@components/Switch';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import useMobileSelectionMode from '@hooks/useMobileSelectionMode';
Expand Down Expand Up @@ -74,11 +74,11 @@ function PolicyDistanceRatesPage({
const dismissError = useCallback(
(item: RateForList) => {
if (customUnitRates[item.value].errors) {
DistanceRate.clearDeleteDistanceRateError(policyID, customUnit?.customUnitID ?? '', item.value);
DistanceRate.clearDeleteDistanceRateError(policyID, customUnit?.customUnitID ?? CONST.EMPTY_STRING, item.value);
return;
}

DistanceRate.clearCreateDistanceRateItemAndError(policyID, customUnit?.customUnitID ?? '', item.value);
DistanceRate.clearCreateDistanceRateItemAndError(policyID, customUnit?.customUnitID ?? CONST.EMPTY_STRING, item.value);
},
[customUnit?.customUnitID, customUnitRates, policyID],
);
Expand All @@ -98,16 +98,36 @@ function PolicyDistanceRatesPage({
setSelectedDistanceRates([]);
}, [isFocused]);

const updateDistanceRateEnabled = useCallback(
(value: boolean, rateID: string) => {
if (!customUnit) {
return;
}
const rate = customUnit?.rates?.[rateID];
// Rates can be disabled or deleted as long as in the remaining rates there is always at least one enabled rate and there are no pending delete action
const canDisableOrDeleteRate = Object.values(customUnit?.rates ?? {}).some(
(distanceRate: Rate) => distanceRate?.enabled && rateID !== distanceRate?.customUnitRateID && distanceRate?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
);

if (!rate?.enabled || canDisableOrDeleteRate) {
DistanceRate.setPolicyDistanceRatesEnabled(policyID, customUnit, [{...rate, enabled: value}]);
} else {
setIsWarningModalVisible(true);
}
},
[customUnit, policyID],
);

const distanceRatesList = useMemo<RateForList[]>(
() =>
Object.values(customUnitRates)
.sort((rateA, rateB) => (rateA?.rate ?? 0) - (rateB?.rate ?? 0))
.map((value) => ({
value: value.customUnitRateID ?? '',
value: value.customUnitRateID ?? CONST.EMPTY_STRING,
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 need to start enforcing this @akinwale?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, this approach is wrong.
Please check https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-ids:

String IDs should not have a default value

This was the whole purpose of the eslint change – to correctly handle such cases.
Since this PR, apparently, caused multiple regressions, I suggest that a follow-up PR should handle these missing IDs properly.

An example PR for proper handling is #54297

Copy link
Contributor

Choose a reason for hiding this comment

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

@abzokhattab @akinwale yes please, lets fix this. Please read over the documentation linked and feel free to ask in slack (also search past convos in slack as this was discussed multiple times)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I misunderstood that we can use the default value from a fixed constant. Should I raise a PR to fix the blocker issues along with this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

@abzokhattab Please raise a single PR to fix all the issues and tag the fixed issues in the OP. I am available to review so ping me when it's ready.

text: `${CurrencyUtils.convertAmountToDisplayString(value.rate, value.currency ?? CONST.CURRENCY.USD)} / ${translate(
`common.${customUnit?.attributes?.unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES}`,
)}`,
keyForList: value.customUnitRateID ?? '',
keyForList: value.customUnitRateID ?? CONST.EMPTY_STRING,
isSelected: selectedDistanceRates.find((rate) => rate.customUnitRateID === value.customUnitRateID) !== undefined && canSelectMultiple,
isDisabled: value.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
pendingAction:
Expand All @@ -119,9 +139,16 @@ function PolicyDistanceRatesPage({
value.pendingFields?.taxClaimablePercentage ??
(policy?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD ? policy?.pendingAction : undefined),
errors: value.errors ?? undefined,
rightElement: <ListItemRightCaretWithLabel labelText={value.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')} />,
rightElement: (
<Switch
isOn={!!value?.enabled}
accessibilityLabel={translate('workspace.distanceRates.trackTax')}
onToggle={(newValue: boolean) => updateDistanceRateEnabled(newValue, value.customUnitRateID)}
disabled={value.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}
/>
),
})),
[customUnit?.attributes?.unit, customUnitRates, selectedDistanceRates, translate, policy?.pendingAction, canSelectMultiple],
[customUnitRates, translate, customUnit, selectedDistanceRates, canSelectMultiple, policy?.pendingAction, updateDistanceRateEnabled],
);

const addRate = () => {
Expand Down Expand Up @@ -170,7 +197,7 @@ function PolicyDistanceRatesPage({
DistanceRate.deletePolicyDistanceRates(
policyID,
customUnit,
selectedDistanceRates.map((rate) => rate.customUnitRateID ?? ''),
selectedDistanceRates.map((rate) => rate.customUnitRateID ?? CONST.EMPTY_STRING),
);
setSelectedDistanceRates([]);
setIsDeleteModalVisible(false);
Expand Down
26 changes: 22 additions & 4 deletions src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useMemo, useState} from 'react';
import React, {useCallback, useMemo, useState} from 'react';
import {View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import Button from '@components/Button';
Expand All @@ -10,12 +10,12 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton';
import * as Expensicons from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import ListItemRightCaretWithLabel from '@components/SelectionList/ListItemRightCaretWithLabel';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import SelectionListWithModal from '@components/SelectionListWithModal';
import CustomListHeader from '@components/SelectionListWithModal/CustomListHeader';
import TableListItemSkeleton from '@components/Skeletons/TableRowSkeleton';
import Switch from '@components/Switch';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import useMobileSelectionMode from '@hooks/useMobileSelectionMode';
Expand Down Expand Up @@ -90,6 +90,18 @@ function ReportFieldsListValuesPage({
return [reportFieldValues, reportFieldDisabledValues];
}, [formDraft?.disabledListValues, formDraft?.listValues, policy?.fieldList, reportFieldID]);

const updateReportFieldListValueEnabled = useCallback(
(value: boolean, valueIndex: number) => {
if (reportFieldID) {
ReportField.updateReportFieldListValueEnabled(policyID, reportFieldID, [Number(valueIndex)], value);
return;
}

ReportField.setReportFieldsListValueEnabled([valueIndex], value);
},
[policyID, reportFieldID],
);

const listValuesSections = useMemo(() => {
const data = listValues
.map<ValueListItem>((value, index) => ({
Expand All @@ -99,11 +111,17 @@ function ReportFieldsListValuesPage({
keyForList: value,
isSelected: selectedValues[value] && canSelectMultiple,
enabled: !disabledListValues.at(index) ?? true,
rightElement: <ListItemRightCaretWithLabel labelText={disabledListValues.at(index) ? translate('workspace.common.disabled') : translate('workspace.common.enabled')} />,
rightElement: (
<Switch
isOn={!disabledListValues.at(index) ?? true}
accessibilityLabel={translate('workspace.distanceRates.trackTax')}
onToggle={(newValue: boolean) => updateReportFieldListValueEnabled(newValue, index)}
/>
),
}))
.sort((a, b) => localeCompare(a.value, b.value));
return [{data, isDisabled: false}];
}, [canSelectMultiple, disabledListValues, listValues, selectedValues, translate]);
}, [canSelectMultiple, disabledListValues, listValues, selectedValues, translate, updateReportFieldListValueEnabled]);

const shouldShowEmptyState = Object.values(listValues ?? {}).length <= 0;
const selectedValuesArray = Object.keys(selectedValues).filter((key) => selectedValues[key]);
Expand Down
69 changes: 51 additions & 18 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ import * as Illustrations from '@components/Icon/Illustrations';
import LottieAnimations from '@components/LottieAnimations';
import type {PopoverMenuItem} from '@components/PopoverMenu';
import ScreenWrapper from '@components/ScreenWrapper';
import ListItemRightCaretWithLabel from '@components/SelectionList/ListItemRightCaretWithLabel';
import TableListItem from '@components/SelectionList/TableListItem';
import SelectionListWithModal from '@components/SelectionListWithModal';
import CustomListHeader from '@components/SelectionListWithModal/CustomListHeader';
import TableListItemSkeleton from '@components/Skeletons/TableRowSkeleton';
import Switch from '@components/Switch';
import Text from '@components/Text';
import TextLink from '@components/TextLink';
import useEnvironment from '@hooks/useEnvironment';
Expand Down Expand Up @@ -103,23 +103,49 @@ function WorkspaceTagsPage({route}: WorkspaceTagsPageProps) {
: undefined;
};

const updateWorkspaceTagEnabled = useCallback(
(value: boolean, tagName: string) => {
Tag.setWorkspaceTagEnabled(policyID, {[tagName]: {name: tagName, enabled: value}}, 0);
},
[policyID],
);

const updateWorkspaceRequiresTag = useCallback(
(value: boolean, orderWeight: number) => {
Tag.setPolicyTagsRequired(policyID, value, orderWeight);
},
[policyID],
);
const tagList = useMemo<TagListItem[]>(() => {
if (isMultiLevelTags) {
return policyTagLists.map((policyTagList) => ({
value: policyTagList.name,
orderWeight: policyTagList.orderWeight,
text: PolicyUtils.getCleanedTagName(policyTagList.name),
keyForList: String(policyTagList.orderWeight),
isSelected: selectedTags[policyTagList.name] && canSelectMultiple,
pendingAction: getPendingAction(policyTagList),
enabled: true,
required: policyTagList.required,
rightElement: (
<ListItemRightCaretWithLabel
labelText={policyTagList.required && !!Object.values(policyTagList?.tags ?? {}).some((tag) => tag.enabled) ? translate('common.required') : undefined}
/>
),
}));
return policyTagLists.map((policyTagList) => {
const areTagsEnabled = !!Object.values(policyTagList?.tags ?? {}).some((tag) => tag.enabled);
const isSwitchDisabled = !policyTagList.required && !areTagsEnabled;
const isSwitchEnabled = policyTagList.required && areTagsEnabled;

if (policyTagList.required && !areTagsEnabled) {
updateWorkspaceRequiresTag(false, policyTagList.orderWeight);
}

return {
value: policyTagList.name,
orderWeight: policyTagList.orderWeight,
text: PolicyUtils.getCleanedTagName(policyTagList.name),
keyForList: String(policyTagList.orderWeight),
isSelected: selectedTags[policyTagList.name] && canSelectMultiple,
pendingAction: getPendingAction(policyTagList),
enabled: true,
required: policyTagList.required,
rightElement: (
<Switch
isOn={isSwitchEnabled}
accessibilityLabel={translate('workspace.tags.requiresTag')}
onToggle={(newValue: boolean) => updateWorkspaceRequiresTag(newValue, policyTagList.orderWeight)}
disabled={isSwitchDisabled}
/>
),
};
});
}
const sortedTags = lodashSortBy(Object.values(policyTagLists.at(0)?.tags ?? {}), 'name', localeCompare) as PolicyTag[];
return sortedTags.map((tag) => ({
Expand All @@ -131,9 +157,16 @@ function WorkspaceTagsPage({route}: WorkspaceTagsPageProps) {
errors: tag.errors ?? undefined,
enabled: tag.enabled,
isDisabled: tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
rightElement: <ListItemRightCaretWithLabel labelText={tag.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')} />,
rightElement: (
<Switch
isOn={tag.enabled}
disabled={tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}
accessibilityLabel={translate('workspace.tags.enableTag')}
onToggle={(newValue: boolean) => updateWorkspaceTagEnabled(newValue, tag.name)}
/>
),
}));
}, [isMultiLevelTags, policyTagLists, selectedTags, canSelectMultiple, translate]);
}, [isMultiLevelTags, policyTagLists, selectedTags, canSelectMultiple, translate, updateWorkspaceRequiresTag, updateWorkspaceTagEnabled]);

const tagListKeyedByName = useMemo(
() =>
Expand Down
Loading
Loading