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

[Wave Collect] [Xero] More features text #42662

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
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
7 changes: 5 additions & 2 deletions src/components/Switch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,17 @@ type SwitchProps = {

/** Whether the switch is disabled */
disabled?: boolean;

/** Whether to show the lock icon even if the switch is enabled */
showLockIcon?: boolean;
};

const OFFSET_X = {
OFF: 0,
ON: 20,
};

function Switch({isOn, onToggle, accessibilityLabel, disabled}: SwitchProps) {
function Switch({isOn, onToggle, accessibilityLabel, disabled, showLockIcon}: SwitchProps) {
const styles = useThemeStyles();
const offsetX = useRef(new Animated.Value(isOn ? OFFSET_X.ON : OFFSET_X.OFF));
const theme = useTheme();
Expand Down Expand Up @@ -60,7 +63,7 @@ function Switch({isOn, onToggle, accessibilityLabel, disabled}: SwitchProps) {
pressDimmingValue={0.8}
>
<Animated.View style={[styles.switchThumb, styles.switchThumbTransformation(offsetX.current)]}>
{disabled && (
{(Boolean(disabled) || Boolean(showLockIcon)) && (
<Icon
src={Expensicons.Lock}
fill={isOn ? theme.text : theme.icon}
Expand Down
6 changes: 6 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2184,6 +2184,12 @@ export default {
title: 'Accounting',
subtitle: 'Sync your chart of accounts and more.',
},
connectionsWarningModal: {
featureEnabledTitle: `Not so fast...`,
featureEnabledText: 'To enable or disable this feature change your accounting import settings.',
disconnectText: 'Disconnect your accounting connection from the workspace if you want to disable Accounting.',
manageSettings: 'Manage settings',
},
},
reportFields: {
delete: 'Delete field',
Expand Down
6 changes: 6 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2220,6 +2220,12 @@ export default {
title: 'Contabilidad',
subtitle: 'Sincroniza tu plan de cuentas y otras opciones.',
},
connectionsWarningModal: {
featureEnabledTitle: 'No tan rápido...',
featureEnabledText: 'Para activar o desactivar esta función, cambie la configuración de importación contable.',
disconnectText: 'Desconecte su conexión contable del espacio de trabajo si desea desactivar la Contabilidad.',
manageSettings: 'Gestionar la configuración',
},
},
reportFields: {
delete: 'Eliminar campos',
Expand Down
76 changes: 74 additions & 2 deletions src/pages/workspace/WorkspaceMoreFeaturesPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {useFocusEffect} from '@react-navigation/native';
import type {StackScreenProps} from '@react-navigation/stack';
import React, {useCallback} from 'react';
import React, {useCallback, useState} from 'react';
import {View} from 'react-native';
import ConfirmModal from '@components/ConfirmModal';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
Expand All @@ -13,10 +14,12 @@ import usePermissions from '@hooks/usePermissions';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import * as ErrorUtils from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import type {FullScreenNavigatorParamList} from '@libs/Navigation/types';
import * as Policy from '@userActions/Policy';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
Expand All @@ -26,6 +29,12 @@ import type {WithPolicyAndFullscreenLoadingProps} from './withPolicyAndFullscree
import withPolicyAndFullscreenLoading from './withPolicyAndFullscreenLoading';
import ToggleSettingOptionRow from './workflows/ToggleSettingsOptionRow';

type ItemType = 'organize' | 'integrate';
type ConnectionWarningModalState = {
isOpen: boolean;
itemType?: ItemType;
};

type WorkspaceMoreFeaturesPageProps = WithPolicyAndFullscreenLoadingProps & StackScreenProps<FullScreenNavigatorParamList, typeof SCREENS.WORKSPACE.MORE_FEATURES>;

type Item = {
Expand Down Expand Up @@ -53,6 +62,9 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
const {canUseAccountingIntegrations} = usePermissions();
const hasAccountingConnection = !!policy?.areConnectionsEnabled && !isEmptyObject(policy?.connections);
const isSyncTaxEnabled = !!policy?.connections?.quickbooksOnline?.config.syncTax || !!policy?.connections?.xero?.config.importTaxRates;
const policyID = policy?.id ?? '';

const [connectionWarningModalState, setConnectionWarningModalState] = useState<ConnectionWarningModalState>({isOpen: false});

const spendItems: Item[] = [
{
Expand Down Expand Up @@ -86,6 +98,13 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
disabled: hasAccountingConnection,
pendingAction: policy?.pendingFields?.areCategoriesEnabled,
action: (isEnabled: boolean) => {
if (hasAccountingConnection) {
setConnectionWarningModalState({
isOpen: true,
itemType: 'organize',
});
return;
}
Comment on lines +101 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

We repeated this piece of code few times, I think we should reuse it with an useCallback?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure if it is necessary, we're not relying on the previous state that we really need to put it inside useCallback. The hasAccountingConnection is also dependent on the prop.

And about repetition, it's a setState call. I could wrap it inside the method but looks like an overkill.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should, including Policy.enablePolicyCategories(policy?.id ?? '', isEnabled); line below.
Also can you complete the checklist please 😄 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Working through the checklist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With enablePolicyCategories it might make sense. Let me check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lakchote Do you think it makes sense to move this code inside a useCallback?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do agree with @mananjadhav, I don't think there's a benefit here to use useCallback(). It's not a computational intensive operation, we're just setting state. It can be tempting to use it to avoid repetition, but using these memorized functions come at a cost on intial render. Given the fact it's just setting state, I'd say it's a premature optimization scenario and we can keep it that way.

Happy to be proven wrong though, if you can give me arguments proving me otherwise @hungvu193 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's fine though. We can keep it this way :)

Policy.enablePolicyCategories(policy?.id ?? '', isEnabled);
},
},
Expand All @@ -97,6 +116,13 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
disabled: hasAccountingConnection,
pendingAction: policy?.pendingFields?.areTagsEnabled,
action: (isEnabled: boolean) => {
if (hasAccountingConnection) {
setConnectionWarningModalState({
isOpen: true,
itemType: 'organize',
});
return;
}
Policy.enablePolicyTags(policy?.id ?? '', isEnabled);
},
},
Expand All @@ -108,6 +134,13 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
disabled: hasAccountingConnection,
pendingAction: policy?.pendingFields?.tax,
action: (isEnabled: boolean) => {
if (hasAccountingConnection) {
setConnectionWarningModalState({
isOpen: true,
itemType: 'organize',
});
return;
}
Policy.enablePolicyTaxes(policy?.id ?? '', isEnabled);
},
},
Expand All @@ -121,6 +154,13 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
isActive: !!policy?.areConnectionsEnabled,
pendingAction: policy?.pendingFields?.areConnectionsEnabled,
action: (isEnabled: boolean) => {
if (hasAccountingConnection) {
setConnectionWarningModalState({
isOpen: true,
itemType: 'integrate',
});
return;
}
Policy.enablePolicyConnections(policy?.id ?? '', isEnabled);
},
disabled: hasAccountingConnection,
Expand Down Expand Up @@ -165,7 +205,7 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
isActive={item.isActive}
pendingAction={item.pendingAction}
onToggle={item.action}
disabled={item.disabled}
showLockIcon={item.disabled}
errors={item.errors}
onCloseError={item.onCloseError}
/>
Expand Down Expand Up @@ -206,6 +246,17 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
}, [fetchFeatures]),
);

const getConnectionWarningPrompt = useCallback(() => {
switch (connectionWarningModalState.itemType) {
case 'organize':
return translate('workspace.moreFeatures.connectionsWarningModal.featureEnabledText');
case 'integrate':
return translate('workspace.moreFeatures.connectionsWarningModal.disconnectText');
default:
return undefined;
}
}, [connectionWarningModalState.itemType, translate]);

return (
<AccessOrNotFoundWrapper
accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
Expand All @@ -224,6 +275,27 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
/>

<ScrollView contentContainerStyle={styles.pb2}>{sections.map(renderSection)}</ScrollView>

<ConfirmModal
title={translate('workspace.moreFeatures.connectionsWarningModal.featureEnabledTitle')}
onConfirm={() => {
setConnectionWarningModalState({
isOpen: false,
itemType: undefined,
});
Navigation.navigate(ROUTES.POLICY_ACCOUNTING.getRoute(policyID));
}}
onCancel={() =>
setConnectionWarningModalState({
isOpen: false,
itemType: undefined,
})
}
isVisible={connectionWarningModalState.isOpen}
prompt={getConnectionWarningPrompt()}
confirmText={translate('workspace.moreFeatures.connectionsWarningModal.manageSettings')}
cancelText={translate('common.cancel')}
/>
</ScreenWrapper>
</AccessOrNotFoundWrapper>
);
Expand Down
5 changes: 5 additions & 0 deletions src/pages/workspace/workflows/ToggleSettingsOptionRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ type ToggleSettingOptionRowProps = {
onCloseError?: () => void;
/** Whether the toggle should be disabled */
disabled?: boolean;

/** Whether to show the lock icon even if the switch is enabled */
showLockIcon?: boolean;
};
const ICON_SIZE = 48;

Expand All @@ -56,6 +59,7 @@ function ToggleSettingOptionRow({
errors,
onCloseError,
disabled = false,
showLockIcon = false,
}: ToggleSettingOptionRowProps) {
const styles = useThemeStyles();

Expand Down Expand Up @@ -93,6 +97,7 @@ function ToggleSettingOptionRow({
onToggle={onToggle}
isOn={isActive}
disabled={disabled}
showLockIcon={showLockIcon}
/>
</View>
{shouldPlaceSubtitleBelowSwitch && subtitle && subTitleView}
Expand Down
Loading