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

Create WorkspaceTagsPage #37621

Merged
merged 7 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
16 changes: 16 additions & 0 deletions assets/images/simple-illustrations/simple-illustration__tag.svg
Copy link
Contributor

Choose a reason for hiding this comment

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

Just confirming that the size of this is 68x68? Where did this asset come from? Try this one maybe:
simple-illustration__tag

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Downloaded from the figma and adjusted as all illustrations without any additional sizing

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, in the future you can just get the assets from the Design team so we can make sure you're pulling the right stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shawnborton okay, thank you!

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/images/tag.svg
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did we get the asset for this? I think all of our icons we put into the system are 20x20 in pure black (#000000).

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated version for ya! tag.svg.zip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Downloaded from figma😅

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 4 additions & 1 deletion src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,10 @@ const ROUTES = {
route: 'workspace/:policyID/categories/settings',
getRoute: (policyID: string) => `workspace/${policyID}/categories/settings` as const,
},

WORKSPACE_TAGS: {
route: 'workspace/:policyID/tags',
getRoute: (policyID: string) => `workspace/${policyID}/tags` as const,
},
// Referral program promotion
REFERRAL_DETAILS_MODAL: {
route: 'referral/:contentType',
Expand Down
1 change: 1 addition & 0 deletions src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ const SCREENS = {
INVITE: 'Workspace_Invite',
INVITE_MESSAGE: 'Workspace_Invite_Message',
CATEGORIES: 'Workspace_Categories',
TAGS: 'Workspace_Tags',
CURRENCY: 'Workspace_Profile_Currency',
WORKFLOWS: 'Workspace_Workflows',
WORKFLOWS_AUTO_REPORTING_FREQUENCY: 'Workspace_Workflows_Auto_Reporting_Frequency',
Expand Down
2 changes: 2 additions & 0 deletions src/components/Icon/Expensicons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ import Twitter from '@assets/images/social-twitter.svg';
import Youtube from '@assets/images/social-youtube.svg';
import Stopwatch from '@assets/images/stopwatch.svg';
import Sync from '@assets/images/sync.svg';
import Tag from '@assets/images/tag.svg';
import Task from '@assets/images/task.svg';
import ThreeDots from '@assets/images/three-dots.svg';
import ThumbsUp from '@assets/images/thumbs-up.svg';
Expand Down Expand Up @@ -222,6 +223,7 @@ export {
FlagLevelThree,
Fullscreen,
Folder,
Tag,
Gallery,
Gear,
Globe,
Expand Down
2 changes: 2 additions & 0 deletions src/components/Icon/Illustrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import ReceiptWrangler from '@assets/images/simple-illustrations/simple-illustra
import SanFrancisco from '@assets/images/simple-illustrations/simple-illustration__sanfrancisco.svg';
import ShieldYellow from '@assets/images/simple-illustrations/simple-illustration__shield.svg';
import SmallRocket from '@assets/images/simple-illustrations/simple-illustration__smallrocket.svg';
import Tag from '@assets/images/simple-illustrations/simple-illustration__tag.svg';
import ThumbsUpStars from '@assets/images/simple-illustrations/simple-illustration__thumbsupstars.svg';
import TrackShoe from '@assets/images/simple-illustrations/simple-illustration__track-shoe.svg';
import TrashCan from '@assets/images/simple-illustrations/simple-illustration__trashcan.svg';
Expand Down Expand Up @@ -146,4 +147,5 @@ export {
Workflows,
ThreeLeggedLaptopWoman,
House,
Tag,
};
10 changes: 10 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1727,6 +1727,7 @@ export default {
settings: 'Settings',
reimburse: 'Reimbursements',
categories: 'Categories',
tags: 'Tags',
bills: 'Bills',
invoices: 'Invoices',
travel: 'Travel',
Expand Down Expand Up @@ -1766,6 +1767,15 @@ export default {
},
genericFailureMessage: 'An error occurred while updating the category, please try again.',
},
tags: {
requiresTag: 'Members must tag all spend',
enableTag: 'Enable tag',
subtitle: 'Tags add more detailed ways to classify costs.',
emptyTags: {
title: "You haven't created any tags",
subtitle: 'Add a tag to track projects, locations, departments, and more.',
},
},
emptyWorkspace: {
title: 'Create a workspace',
subtitle: 'Workspaces are where you’ll chat with your team, reimburse expenses, issue cards, send invoices, pay bills, and more - all in one place.',
Expand Down
10 changes: 10 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1751,6 +1751,7 @@ export default {
settings: 'Configuración',
reimburse: 'Reembolsos',
categories: 'Categorías',
tags: 'Etiquetas',
bills: 'Pagar facturas',
invoices: 'Enviar facturas',
travel: 'Viajes',
Expand Down Expand Up @@ -1790,6 +1791,15 @@ export default {
},
genericFailureMessage: 'Se ha producido un error al intentar eliminar la categoría. Por favor, inténtalo más tarde.',
},
tags: {
requiresTag: 'Los miembros deben etiquetar todos los gastos',
enableTag: 'Habilitar etiqueta',
subtitle: 'Las etiquetas agregan formas más detalladas de clasificar los costos.',
waterim marked this conversation as resolved.
Show resolved Hide resolved
emptyTags: {
title: 'No has creado ninguna etiqueta',
subtitle: 'Agregue una etiqueta para realizar un seguimiento de proyectos, ubicaciones, departamentos y más.',
waterim marked this conversation as resolved.
Show resolved Hide resolved
},
},
emptyWorkspace: {
title: 'Crea un espacio de trabajo',
subtitle: 'En los espacios de trabajo podrás chatear con tu equipo, reembolsar gastos, emitir tarjetas, enviar y pagar facturas, y mucho más - todo en un mismo lugar.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const workspaceSettingsScreens = {
[SCREENS.WORKSPACE.TRAVEL]: () => require('../../../../../pages/workspace/travel/WorkspaceTravelPage').default as React.ComponentType,
[SCREENS.WORKSPACE.MEMBERS]: () => require('../../../../../pages/workspace/WorkspaceMembersPage').default as React.ComponentType,
[SCREENS.WORKSPACE.CATEGORIES]: () => require('../../../../../pages/workspace/categories/WorkspaceCategoriesPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAGS]: () => require('../../../../../pages/workspace/tags/WorkspaceTagsPage').default as React.ComponentType,
} satisfies Screens;

function BaseCentralPaneNavigator() {
Expand Down
3 changes: 3 additions & 0 deletions src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
[SCREENS.WORKSPACE.CATEGORIES]: {
path: ROUTES.WORKSPACE_CATEGORIES.route,
},
[SCREENS.WORKSPACE.TAGS]: {
path: ROUTES.WORKSPACE_TAGS.route,
},
},
},
[SCREENS.NOT_FOUND]: '*',
Expand Down
3 changes: 3 additions & 0 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ type CentralPaneNavigatorParamList = {
[SCREENS.WORKSPACE.CATEGORIES]: {
policyID: string;
};
[SCREENS.WORKSPACE.TAGS]: {
policyID: string;
};
};

type WorkspaceSwitcherNavigatorParamList = {
Expand Down
6 changes: 6 additions & 0 deletions src/pages/workspace/WorkspaceInitialPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, policyMembers, r
action: singleExecution(waitForNavigate(() => Navigation.navigate(ROUTES.WORKSPACE_CATEGORIES.getRoute(policyID)))),
routeName: SCREENS.WORKSPACE.CATEGORIES,
},
{
translationKey: 'workspace.common.tags',
icon: Expensicons.Tag,
action: singleExecution(waitForNavigate(() => Navigation.navigate(ROUTES.WORKSPACE_TAGS.getRoute(policyID)))),
routeName: SCREENS.WORKSPACE.TAGS,
},
];

const menuItems: WorkspaceMenuItem[] = [
Expand Down
144 changes: 144 additions & 0 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import type {StackScreenProps} from '@react-navigation/stack';
import React, {useMemo, useState} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
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 TableListItem from '@components/SelectionList/TableListItem';
import Text from '@components/Text';
import WorkspaceEmptyStateSection from '@components/WorkspaceEmptyStateSection';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import * as PolicyUtils from '@libs/PolicyUtils';
import type {CentralPaneNavigatorParamList} from '@navigation/types';
import AdminPolicyAccessOrNotFoundWrapper from '@pages/workspace/AdminPolicyAccessOrNotFoundWrapper';
import PaidPolicyAccessOrNotFoundWrapper from '@pages/workspace/PaidPolicyAccessOrNotFoundWrapper';
import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';

type PolicyForList = {
value: string;
text: string;
keyForList: string;
isSelected: boolean;
rightElement: React.ReactNode;
};

type WorkspaceTagsOnyxProps = {
/** Collection of tags attached to a policy */
policyTags: OnyxEntry<OnyxTypes.PolicyTagList>;
};

type WorkspaceTagsPageProps = WorkspaceTagsOnyxProps & StackScreenProps<CentralPaneNavigatorParamList, typeof SCREENS.WORKSPACE.TAGS>;

function WorkspaceTagsPage({policyTags, route}: WorkspaceTagsPageProps) {
const {isSmallScreenWidth} = useWindowDimensions();
const styles = useThemeStyles();
const theme = useTheme();
const {translate} = useLocalize();
const [selectedTags, setSelectedTags] = useState<Record<string, boolean>>({});
const policyTagLists = useMemo(() => PolicyUtils.getTagLists(policyTags), [policyTags]);
const tagList = useMemo<PolicyForList[]>(
Copy link
Contributor

@ntdiary ntdiary Apr 8, 2024

Choose a reason for hiding this comment

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

Coming from #38663.
We add a isDisabled flag to take care of the behavior when deleting a tag offline. :)

() =>
policyTagLists
.map((tagList) =>

Check failure on line 52 in src/pages/workspace/tags/WorkspaceTagsPage.tsx

View workflow job for this annotation

GitHub Actions / lint

'tagList' is already declared in the upper scope on line 49 column 11
Object.values(tagList.tags).map((value) => ({
value: value.name,
text: value.name,
keyForList: value.name,
waterim marked this conversation as resolved.
Show resolved Hide resolved
isSelected: !!selectedTags[value.name],
rightElement: (
<View style={styles.flexRow}>
<Text style={[styles.disabledText, styles.alignSelfCenter]}>
{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>
),
})),
)
.flat(),
[policyTags, selectedTags, styles.alignSelfCenter, styles.disabledText, styles.flexRow, styles.p1, styles.pl2, theme.icon, translate],

Check warning on line 74 in src/pages/workspace/tags/WorkspaceTagsPage.tsx

View workflow job for this annotation

GitHub Actions / lint

React Hook useMemo has a missing dependency: 'policyTagLists'. Either include it or remove the dependency array
);

const toggleTag = (tag: PolicyForList) => {
setSelectedTags((prev) => ({
...prev,
[tag.value]: !prev[tag.value],
}));
};

const toggleAllTags = () => {
const isAllSelected = tagList.every((tag) => !!selectedTags[tag.value]);
setSelectedTags(isAllSelected ? {} : Object.fromEntries(tagList.map((item) => [item.value, true])));
};

const getCustomListHeader = () => (
<View style={[styles.flex1, styles.flexRow, styles.justifyContentBetween, styles.pl3, styles.pr9]}>
<Text style={styles.searchInputStyle}>{translate('common.name')}</Text>
<Text style={[styles.searchInputStyle, styles.textAlignCenter]}>{translate('statusPage.status')}</Text>
</View>
);

return (
<AdminPolicyAccessOrNotFoundWrapper policyID={route.params.policyID}>
<PaidPolicyAccessOrNotFoundWrapper policyID={route.params.policyID}>
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
style={[styles.defaultModalContainer]}
testID={WorkspaceTagsPage.displayName}
shouldShowOfflineIndicatorInWideScreen
>
<HeaderWithBackButton
icon={Illustrations.Tag}
title={translate('workspace.common.tags')}
shouldShowBackButton={isSmallScreenWidth}
/>
Comment on lines +99 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, I have a question related to this change. Every workspace page uses the WorkspacePageWithSections wrapper except for this one. Is this intentional?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the new pages do, e.g. WorkspaceMembersPage, WorkspaceCategoriesPage

Copy link
Contributor

Choose a reason for hiding this comment

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

The WorkspaceMembersPage copies the logic from the WorkspacePageWithSections.

I'm using this wrapper to fix multiple not found views, and part of the PR is making the above page use the wrapper.

So what is the aim of the WorkspacePageWithSections wrapper? When should it be used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm interesting case, maybe we should be using WorkspacePageWithSections here too and on all other pages. I think we can update these, but maybe in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably have to think it over as currently, we have three different wrappers for workspaces, and we don't have clear guidelines on when to use which one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, that makes sense. I'll create an issue for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Created issue - #37898

Copy link
Contributor

Choose a reason for hiding this comment

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

Those wrappers also break the not-found views.

Going to e.g. https://staging.new.expensify.com/workspace/dssaddssdaads/categories` we see duplicated not found (that's what is the scope of the other issue) and after pressing the back button we see another ones...

<View style={[styles.ph5, styles.pb5]}>
<Text style={[styles.textNormal, styles.colorMuted]}>{translate('workspace.tags.subtitle')}</Text>
</View>
{tagList.length ? (
<SelectionList
canSelectMultiple
sections={[{data: tagList, indexOffset: 0, isDisabled: false}]}
onCheckboxPress={toggleTag}
onSelectRow={() => {}}
waterim marked this conversation as resolved.
Show resolved Hide resolved
onSelectAll={toggleAllTags}
showScrollIndicator
ListItem={TableListItem}
customListHeader={getCustomListHeader()}
listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]}
/>
) : (
<WorkspaceEmptyStateSection
title={translate('workspace.tags.emptyTags.title')}
icon={Illustrations.EmptyStateExpenses}
subtitle={translate('workspace.tags.emptyTags.subtitle')}
/>
)}
</ScreenWrapper>
</PaidPolicyAccessOrNotFoundWrapper>
</AdminPolicyAccessOrNotFoundWrapper>
);
}

WorkspaceTagsPage.displayName = 'WorkspaceTagsPage';

export default withOnyx<WorkspaceTagsPageProps, WorkspaceTagsOnyxProps>({
policyTags: {
key: ({route}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${route.params.policyID}`,
},
})(WorkspaceTagsPage);
Loading