✨ Add multiple expenses via bank transactions at the same time #477
Conversation
| "i18next-http-backend": "^3.0.2", | ||
| "input-otp": "^1.2.3", | ||
| "lucide-react": "^0.544.0", | ||
| "motion": "^12.23.24", |
There was a problem hiding this comment.
Huh? Any reasoning behind this?
| return ( | ||
| <BankingTransactionList | ||
| add={addViaBankTransaction} | ||
| // addMultipleExpenses={addMultipleExpenses} | ||
| // multipleTransactions={multipleTransactions} | ||
| // setMultipleTransactions={handleSetMultipleTransactions} | ||
| // isTransactionLoading={isTransactionLoading} | ||
| addAllMultipleExpenses={addAllMultipleExpenses} | ||
| addOneByOneMultipleExpenses={addOneByOneMultipleExpenses} | ||
| multipleTransactions={multipleTransactions} | ||
| setMultipleTransactions={handleSetMultipleTransactions} | ||
| isTransactionLoading={isTransactionLoading} | ||
| bankConnectionEnabled={bankConnectionEnabled} | ||
| // clearFields={clearFields} | ||
| clearFields={clearFields} | ||
| > | ||
| {children} | ||
| </BankingTransactionList> |
There was a problem hiding this comment.
I don't really get the point of this component. It's just prop drilling with no UI added? Why not merge it with BankTransactionList?
| [setAmount, setAmountStr], | ||
| ); | ||
|
|
||
| const addViaBankTransaction = useCallback( |
There was a problem hiding this comment.
This should be a store action. Also the zustand store is supposed to prevent prop drilling, so once you convert it into an action, use the store hook to fetch it where it's needed rather than passing via prop
| [ | ||
| { | ||
| name: description, | ||
| currency, | ||
| amount: amount * sign, | ||
| groupId: group?.id ?? null, | ||
| splitType, | ||
| participants: participants.map((p) => ({ | ||
| userId: p.id, | ||
| amount: (p.amount ?? 0n) * sign, | ||
| })), | ||
| paidBy: paidBy.id, | ||
| category, | ||
| fileKey, | ||
| expenseDate, | ||
| expenseId, | ||
| transactionId, | ||
| cronExpression: cronExpression ? cronToBackend(cronExpression) : undefined, |
There was a problem hiding this comment.
I think that updating the zod input schema with something like:
const arrayify = <T extends ZodTypeAny>(schema: T) => {
return z.preprocess((val) => (Array.isArray(val) ? val : [val]), z.array(schema))
}would be nicer and minimize the code changes.
| <div className="flex gap-2"> | ||
| <AddBankTransactions | ||
| // clearFields={clearFields} | ||
| clearFields={clearFields} |
| <AddBankTransactions | ||
| // clearFields={clearFields} | ||
| clearFields={clearFields} | ||
| bankConnectionEnabled={bankConnectionEnabled} |
There was a problem hiding this comment.
I guess we could save it to the store. However, the banking transaction functionality is isolated enough to maybe make it a slice? In general I wanted to break apart the huge add store so this could be a good starting point.
Reference: https://zustand.docs.pmnd.rs/guides/advanced-typescript#slices-pattern
| <AnimatePresence> | ||
| {shouldShowCheckbox && ( | ||
| <motion.div | ||
| key="checkbox" | ||
| initial={checkboxAnimationInitial} | ||
| animate={checkboxAnimationAnimate} | ||
| exit={checkboxAnimationExit} | ||
| transition={checkboxAnimationTransition} | ||
| > | ||
| <Checkbox | ||
| checked={isChecked} | ||
| disabled={alreadyAdded} | ||
| onCheckedChange={createCheckboxHandler(item)} | ||
| className="h-6 w-6 md:h-4 md:w-4" | ||
| /> | ||
| </motion.div> | ||
| )} |
There was a problem hiding this comment.
Adding framer motion for just one place in the UI seems a bit like an overkill, especially that the app does not feature animations elsewhere. I would welcome a general PR that makes it more snappy and fluid and I'm fine with using framer motion, but this is outside the scope of this PR.
| multipleTransactions: TransactionAddInputModel[]; | ||
| }> = ({ index, alreadyAdded, item, onTransactionRowClick, groupName, multipleTransactions }) => { | ||
| const { t, toUIDate } = useTranslationWithUtils(); | ||
| const [isHovered, setIsHovered] = useState(false); |
There was a problem hiding this comment.
If we remove framer, would it be enough to rely on CSS and tailwind groups?
| paidBy: sender.id, | ||
| category: DEFAULT_CATEGORY, | ||
| }, | ||
| [ |
There was a problem hiding this comment.
Should be gone with zod changes.
| category: DEFAULT_CATEGORY, | ||
| groupId: null, | ||
| }, | ||
| [ |
There was a problem hiding this comment.
Should be gone with zod changes.
|
Regarding the dialog text, I would change it to:
|
|
One more thing, can you bump the axios and form-data deps introduced by |
2 participants
Description
Have implemented add multiple expenses via bank transactions at the same time.
Both one by one so you can edit them separate or add all at the same time.
@krokosik Please help me with the text on the modal.
Demo
Nov-08-2025.15-13-501.mp4
Checklist
CONTRIBUTING.mdin its entirety