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

Apply Template to All Categories in Group for Mobile #3737

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Apply Template to All Categories in Group for Mobile #3737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5b924ac
add function to apply template to multiple category and add button to…
Oct 14, 2024
91abab2
add function to apply template to multiple category and add button to…
Oct 14, 2024
af48d6f
Merge remote-tracking branch 'origin/goal-template-apply' into goal-t…
Oct 18, 2024
d22b5ef
add correct month
Dreptschar Oct 20, 2024
80d323c
clean up code
Dreptschar Oct 20, 2024
9a9db3c
clean up code
Dreptschar Oct 20, 2024
f2d5792
Merge remote-tracking branch 'origin/goal-template-apply' into goal-t…
Dreptschar Oct 20, 2024
415577e
clean up code
Dreptschar Oct 20, 2024
597fd9f
clean up code
Dreptschar Oct 20, 2024
3922461
Merge remote-tracking branch 'origin/goal-template-apply' into goal-t…
Dreptschar Oct 20, 2024
d51746d
add notification and clean up
Oct 21, 2024
e363cb7
add notification and clean up
Oct 21, 2024
5f69d72
add notification and clean up
Oct 21, 2024
2b620e7
add notification and clean up
Oct 21, 2024
8c5bd2b
add notification and clean up
Oct 21, 2024
f8de6e9
add release note
Oct 21, 2024
d5c05fc
Merge branch 'master' into goal-template-apply
Oct 21, 2024
846717c
excluded hidden categories
Dreptschar Oct 23, 2024
9d8466e
removed unused method from api
Dreptschar Oct 23, 2024
c88cedf
Merge branch 'refs/heads/master' into goal-template-apply
Dreptschar Oct 23, 2024
c3286d1
adjust template to run on already budgeted categories
Dreptschar Oct 23, 2024
6501668
fix typecheck
Dreptschar Oct 23, 2024
24fa59f
add apply multiple as budget action and remove from api
Dreptschar Oct 24, 2024
5b7f9d1
lint clean up
Dreptschar Oct 24, 2024
61d11a5
Merge branch 'refs/heads/master' into goal-template-apply
Dreptschar Oct 24, 2024
2459aaf
fix notification and remove log
Dreptschar Oct 24, 2024
9e7e11c
add menu for group and button to apply templates
Dreptschar Oct 25, 2024
2cb361a
fix lint
Dreptschar Oct 25, 2024
871fcdf
fix lint and add release note
Dreptschar Oct 25, 2024
22d7e0d
Merge branch 'master' into group-template-mobile
Dreptschar Oct 25, 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
13 changes: 13 additions & 0 deletions packages/desktop-client/src/components/Modals.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { CreateLocalAccountModal } from './modals/CreateLocalAccountModal';
import { EditFieldModal } from './modals/EditFieldModal';
import { EditRuleModal } from './modals/EditRuleModal';
import { EnvelopeBalanceMenuModal } from './modals/EnvelopeBalanceMenuModal';
import { EnvelopeBudgetGroupMenuModal } from './modals/EnvelopeBudgetGroupMenuModal';
import { EnvelopeBudgetMenuModal } from './modals/EnvelopeBudgetMenuModal';
import { EnvelopeBudgetMonthMenuModal } from './modals/EnvelopeBudgetMonthMenuModal';
import { EnvelopeBudgetSummaryModal } from './modals/EnvelopeBudgetSummaryModal';
Expand Down Expand Up @@ -403,6 +404,18 @@ export function Modals() {
/>
</NamespaceContext.Provider>
);
case 'envelope-budget-group-menu':
return (
<NamespaceContext.Provider
key={name}
value={monthUtils.sheetForMonth(options.month)}
>
<EnvelopeBudgetGroupMenuModal
onApplyGroupTemplate={options.onApplyGroupTemplate}
group={options.group}
/>
</NamespaceContext.Provider>
);

case 'tracking-budget-menu':
return (
Expand Down
2 changes: 2 additions & 0 deletions packages/desktop-client/src/components/budget/BudgetCategories.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const BudgetCategories = memo(
onSaveGroup,
onDeleteCategory,
onDeleteGroup,
onApplyBudgetTemplatesInGroup,
onReorderCategory,
onReorderGroup,
}) => {
Expand Down Expand Up @@ -245,6 +246,7 @@ export const BudgetCategories = memo(
onReorderCategory={onReorderCategory}
onToggleCollapse={onToggleCollapse}
onShowNewCategory={onShowNewCategory}
onApplyBudgetTemplatesInGroup={onApplyBudgetTemplatesInGroup}
/>
);
break;
Expand Down
2 changes: 2 additions & 0 deletions packages/desktop-client/src/components/budget/BudgetTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export function BudgetTable(props) {
onDeleteCategory,
onSaveGroup,
onDeleteGroup,
onApplyBudgetTemplatesInGroup,
onReorderCategory,
onReorderGroup,
onShowActivity,
Expand Down Expand Up @@ -235,6 +236,7 @@ export function BudgetTable(props) {
onReorderGroup={_onReorderGroup}
onBudgetAction={onBudgetAction}
onShowActivity={onShowActivity}
onApplyBudgetTemplatesInGroup={onApplyBudgetTemplatesInGroup}
/>
</View>
</View>
Expand Down
5 changes: 5 additions & 0 deletions packages/desktop-client/src/components/budget/ExpenseGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type ExpenseGroupProps = {
onEditName?: ComponentProps<typeof SidebarGroup>['onEdit'];
onSave?: ComponentProps<typeof SidebarGroup>['onSave'];
onDelete?: ComponentProps<typeof SidebarGroup>['onDelete'];
onApplyBudgetTemplatesInGroup?: ComponentProps<
typeof SidebarGroup
>['onApplyBudgetTemplatesInGroup'];
onDragChange: OnDragChangeCallback<
ComponentProps<typeof SidebarGroup>['group']
>;
Expand All @@ -43,6 +46,7 @@ export function ExpenseGroup({
onEditName,
onSave,
onDelete,
onApplyBudgetTemplatesInGroup,
onDragChange,
onReorderGroup,
onReorderCategory,
Expand Down Expand Up @@ -125,6 +129,7 @@ export function ExpenseGroup({
onEdit={onEditName}
onSave={onSave}
onDelete={onDelete}
onApplyBudgetTemplatesInGroup={onApplyBudgetTemplatesInGroup}
onShowNewCategory={onShowNewCategory}
/>
<RenderMonths component={MonthComponent} args={{ group }} />
Expand Down
18 changes: 18 additions & 0 deletions packages/desktop-client/src/components/budget/SidebarGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { type CSSProperties, useRef, useState } from 'react';
import { type ConnectDragSource } from 'react-dnd';
import { useTranslation } from 'react-i18next';

import { useFeatureFlag } from '../../hooks/useFeatureFlag';
import { SvgExpandArrow } from '../../icons/v0';
import { SvgCheveronDown } from '../../icons/v1';
import { theme } from '../../style';
Expand Down Expand Up @@ -32,6 +33,7 @@ type SidebarGroupProps = {
onEdit?: (id: string) => void;
onSave?: (group: object) => Promise<void>;
onDelete?: (id: string) => Promise<void>;
onApplyBudgetTemplatesInGroup?: (categories: object[]) => void;
onShowNewCategory?: (groupId: string) => void;
onHideNewGroup?: () => void;
onToggleCollapse?: (id: string) => void;
Expand All @@ -47,11 +49,13 @@ export function SidebarGroup({
onEdit,
onSave,
onDelete,
onApplyBudgetTemplatesInGroup,
onShowNewCategory,
onHideNewGroup,
onToggleCollapse,
}: SidebarGroupProps) {
const { t } = useTranslation();
const isGoalTemplatesEnabled = useFeatureFlag('goalTemplatesEnabled');

const temporary = group.id === 'new';
const [menuOpen, setMenuOpen] = useState(false);
Expand Down Expand Up @@ -122,6 +126,12 @@ export function SidebarGroup({
onDelete(group.id);
} else if (type === 'toggle-visibility') {
onSave({ ...group, hidden: !group.hidden });
} else if (type === 'apply-multiple-category-template') {
onApplyBudgetTemplatesInGroup?.(
group.categories
.filter(c => !c['hidden'])
.map(c => c['id']),
);
}
setMenuOpen(false);
}}
Expand All @@ -133,6 +143,14 @@ export function SidebarGroup({
text: group.hidden ? t('Show') : t('Hide'),
},
onDelete && { name: 'delete', text: t('Delete') },
...(isGoalTemplatesEnabled
? [
{
name: 'apply-multiple-category-template',
text: t('Apply budget templates'),
},
]
: []),
]}
/>
</Popover>
Expand Down
46 changes: 46 additions & 0 deletions packages/desktop-client/src/components/budget/envelope/BudgetMenuGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import React, { type ComponentPropsWithoutRef } from 'react';
import { useTranslation } from 'react-i18next';

import { useFeatureFlag } from '../../../hooks/useFeatureFlag';
import { Menu } from '../../common/Menu';

type BudgetMenuGroupProps = Omit<
ComponentPropsWithoutRef<typeof Menu>,
'onMenuSelect' | 'items'
> & {
onApplyGroupTemplate: () => void;
};
Comment on lines +7 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving the type definition for better type safety.

The onApplyGroupTemplate callback could be enhanced:

  1. Make it optional since you use optional chaining in the implementation
  2. Include error handling in the type signature
 type BudgetMenuGroupProps = Omit<
   ComponentPropsWithoutRef<typeof Menu>,
   'onMenuSelect' | 'items'
 > & {
-  onApplyGroupTemplate: () => void;
+  onApplyGroupTemplate?: () => void | Promise<void>;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type BudgetMenuGroupProps = Omit<
ComponentPropsWithoutRef<typeof Menu>,
'onMenuSelect' | 'items'
> & {
onApplyGroupTemplate: () => void;
};
type BudgetMenuGroupProps = Omit<
ComponentPropsWithoutRef<typeof Menu>,
'onMenuSelect' | 'items'
> & {
onApplyGroupTemplate?: () => void | Promise<void>;
};

export function BudgetMenuGroup({
onApplyGroupTemplate,
...props
}: BudgetMenuGroupProps) {
const { t } = useTranslation();

const isGoalTemplatesEnabled = useFeatureFlag('goalTemplatesEnabled');
const onMenuSelect = (name: string) => {
switch (name) {
case 'apply-group-category-template':
onApplyGroupTemplate?.();
break;
default:
throw new Error(`Unrecognized menu item: ${name}`);
}
};
Comment on lines +20 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider more graceful error handling in production.

The error throwing for unrecognized menu items might be too aggressive for production. Consider logging the error and providing a fallback behavior.

 const onMenuSelect = (name: string) => {
   switch (name) {
     case 'apply-group-category-template':
       onApplyGroupTemplate?.();
       break;
     default:
-      throw new Error(`Unrecognized menu item: ${name}`);
+      console.error(`Unrecognized menu item: ${name}`);
   }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const onMenuSelect = (name: string) => {
switch (name) {
case 'apply-group-category-template':
onApplyGroupTemplate?.();
break;
default:
throw new Error(`Unrecognized menu item: ${name}`);
}
};
const onMenuSelect = (name: string) => {
switch (name) {
case 'apply-group-category-template':
onApplyGroupTemplate?.();
break;
default:
console.error(`Unrecognized menu item: ${name}`);
}
};


return (
<Menu
{...props}
onMenuSelect={onMenuSelect}
items={[
...(isGoalTemplatesEnabled
? [
{
name: 'apply-group-category-template',
text: t('Apply budget template'),
},
]
: []),
]}
Comment on lines +34 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the Menu component implementation and usage context, I can now provide the final response:

Consider conditionally rendering the Menu component when empty

The Menu component doesn't have built-in handling for empty states, and rendering it with an empty array will result in a clickable menu button that opens an empty menu. To improve the user experience:

return isGoalTemplatesEnabled ? (
  <Menu
    {...props}
    onMenuSelect={onMenuSelect}
    items={[
      {
        name: 'apply-group-category-template',
        text: t('Apply budget template'),
      },
    ]}
  />
) : null;
🔗 Analysis chain

Handle empty menu state when feature is disabled.

When isGoalTemplatesEnabled is false, the menu will be empty. Consider:

  1. Either hiding the menu completely when empty
  2. Or providing a disabled state explaining why features are unavailable
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are other places in the codebase handling empty menus
rg -l "items={(\[\])}" --type ts --type tsx

Length of output: 149

Script:

#!/bin/bash
# Search for empty menu items array patterns with proper escaping
rg "items=\{\[\]\}" --type ts --type tsx

# Search for Menu component usage to understand empty state handling
rg -A 3 "<Menu" --type ts --type tsx

# Look for the Menu component definition
ast-grep --pattern 'interface $_Props {
  $$$
  items: $_
  $$$
}'

Length of output: 206

Script:

#!/bin/bash
# Search for Menu component usage with correct file extension
rg -A 3 "<Menu" .

# Look for the Menu component definition
rg -A 10 "interface.*Menu.*Props" .

# Search for empty menu items array patterns
rg "items=\{(\[\]|[])" .

Length of output: 17212

Script:

#!/bin/bash
# Look for Menu component definition and its props
rg -A 5 "export.*Menu.*=.*" ./packages/desktop-client/src/components/common/Menu.tsx

# Check if there's any handling of empty items
rg -A 5 "items\.length.*===.*0" .

# Look for conditional rendering of Menu component
rg "{\s*items.*length.*[><=].*0.*}" .

Length of output: 963

Script:

#!/bin/bash
# Look at the Menu component implementation
cat ./packages/desktop-client/src/components/common/Menu.tsx

# Look for conditional rendering patterns around Menu
rg -B 2 -A 2 "<Menu.*" ./packages/desktop-client/src/components/budget/envelope/BudgetMenuGroup.tsx

Length of output: 6928

/>
);
}
10 changes: 10 additions & 0 deletions packages/desktop-client/src/components/budget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,15 @@ function BudgetInner(props: BudgetInnerProps) {
}
};

const onApplyBudgetTemplatesInGroup = async categories => {
dispatch(
applyBudgetAction(startMonth, 'apply-multiple-templates', {
month: startMonth,
categories,
}),
);
};

const onBudgetAction = (month, type, args) => {
dispatch(applyBudgetAction(month, type, args));
};
Expand Down Expand Up @@ -366,6 +375,7 @@ function BudgetInner(props: BudgetInnerProps) {
onMonthSelect={onMonthSelect}
onDeleteCategory={onDeleteCategory}
onDeleteGroup={onDeleteGroup}
onApplyBudgetTemplatesInGroup={onApplyBudgetTemplatesInGroup}
onSaveCategory={onSaveCategory}
onSaveGroup={onSaveGroup}
onBudgetAction={onBudgetAction}
Expand Down
66 changes: 48 additions & 18 deletions packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ function BudgetCell({
const categoryNotes = useNotes(category.id);

const onOpenCategoryBudgetMenu = () => {
console.log('open');
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove debug logging statement.

Debug logging statements should not be committed to production code.

-    console.log('open');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log('open');

dispatch(
pushModal(categoryBudgetMenuModal, {
categoryId: category.id,
Expand Down Expand Up @@ -749,6 +750,7 @@ const ExpenseCategory = memo(function ExpenseCategory({

const ExpenseGroupHeader = memo(function ExpenseGroupHeader({
group,
month,
budgeted,
spent,
balance,
Expand All @@ -759,7 +761,26 @@ const ExpenseGroupHeader = memo(function ExpenseGroupHeader({
showBudgetedCol,
collapsed,
onToggleCollapse,
onBudgetAction,
}) {
const dispatch = useDispatch();
const onOpenGroupBudgetMenu = () => {
console.log('OPEN Group header menu' + month);
dispatch(
pushModal('envelope-budget-group-menu', {
month,
group,
onApplyGroupTemplate: () => {
onBudgetAction(month, 'apply-multiple-templates', {
month,
categories: group.categories
.filter(c => !c['hidden'])
.map(c => c['id']),
});
},
}),
);
};
Comment on lines +766 to +783
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Clean up the implementation.

The function has several issues that should be addressed:

  1. Remove the debug logging statement
  2. Remove redundant month parameter in the action payload
  3. Simplify the hidden categories filter
-    console.log('OPEN Group header menu' + month);
     dispatch(
       pushModal('envelope-budget-group-menu', {
         month,
         group,
         onApplyGroupTemplate: () => {
           onBudgetAction(month, 'apply-multiple-templates', {
-            month,
             categories: group.categories
-              .filter(c => !c['hidden'])
-              .map(c => c['id']),
+              .filter(c => !c.hidden)
+              .map(c => c.id),
           });
         },
       }),
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const dispatch = useDispatch();
const onOpenGroupBudgetMenu = () => {
console.log('OPEN Group header menu' + month);
dispatch(
pushModal('envelope-budget-group-menu', {
month,
group,
onApplyGroupTemplate: () => {
onBudgetAction(month, 'apply-multiple-templates', {
month,
categories: group.categories
.filter(c => !c['hidden'])
.map(c => c['id']),
});
},
}),
);
};
const dispatch = useDispatch();
const onOpenGroupBudgetMenu = () => {
dispatch(
pushModal('envelope-budget-group-menu', {
month,
group,
onApplyGroupTemplate: () => {
onBudgetAction(month, 'apply-multiple-templates', {
categories: group.categories
.filter(c => !c.hidden)
.map(c => c.id),
});
},
}),
);
};

const opacity = blank ? 0 : 1;
const listItemRef = useRef();
const format = useFormat();
Expand Down Expand Up @@ -866,24 +887,30 @@ const ExpenseGroupHeader = memo(function ExpenseGroupHeader({
<View
style={{ ...(!show3Cols && !showBudgetedCol && { display: 'none' }) }}
>
<CellValue binding={budgeted} type="financial">
{({ type, value }) => (
<View>
<PrivacyFilter>
<AutoTextSize
key={value}
as={Text}
minFontSizePx={6}
maxFontSizePx={12}
mode="oneline"
style={amountStyle}
>
{format(value, type)}
</AutoTextSize>
</PrivacyFilter>
</View>
)}
</CellValue>
<Button
variant="bare"
style={{ backgroundColor: 'transparent' }}
onPress={() => onOpenGroupBudgetMenu()}
>
<CellValue binding={budgeted} type="financial">
{({ type, value }) => (
<View>
<PrivacyFilter>
<AutoTextSize
key={value}
as={Text}
minFontSizePx={6}
maxFontSizePx={12}
mode="oneline"
style={amountStyle}
>
{format(value, type)}
</AutoTextSize>
</PrivacyFilter>
</View>
)}
</CellValue>
</Button>
</View>
<View
style={{ ...(!show3Cols && showBudgetedCol && { display: 'none' }) }}
Expand Down Expand Up @@ -1016,6 +1043,7 @@ const IncomeGroupHeader = memo(function IncomeGroupHeader({
<Button
variant="bare"
style={{
paddingRight: 4,
maxWidth: sidebarColumnWidth,
}}
onPress={() => onEdit?.(group.id)}
Expand Down Expand Up @@ -1320,6 +1348,8 @@ const ExpenseGroup = memo(function ExpenseGroup({
onEdit={onEditGroup}
collapsed={collapsed}
onToggleCollapse={onToggleCollapse}
month={month}
onBudgetAction={onBudgetAction}
// onReorderCategory={onReorderCategory}
/>

Expand Down
Loading
Loading