actualbudget / actual

A local-first personal finance app
https://actualbudget.org
MIT License
13.96k stars 1.11k forks source link

[Mobile] Long press transaction to reveal floating action bar with bulk actions #2892

Closed joel-jeremy closed 1 month ago

joel-jeremy commented 3 months ago

image image image

netlify[bot] commented 3 months ago

Deploy Preview for actualbudget ready!

Name Link
Latest commit 97e42034e8ce906647d3f33d4534621ce1a01e9a
Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/66ba4dfe62cc9b0008cc3e2a
Deploy Preview https://deploy-preview-2892.demo.actualbudget.org
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions[bot] commented 3 months ago

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count Total bundle size % Changed
9 4.92 MB → 4.94 MB (+21.23 kB) +0.42%
Changeset File | Δ | Size ---- | - | ---- `src/hooks/useTransactionBatchActions.ts` | 🆕 +7.74 kB | 0 B → 7.74 kB `src/hooks/useUndo.ts` | 🆕 +992 B | 0 B → 992 B `src/components/mobile/FloatingActionBar.tsx` | 🆕 +468 B | 0 B → 468 B `node_modules/@react-aria/interactions/dist/PressResponder.mjs` | 📈 +1.8 kB (+581.96%) | 316 B → 2.1 kB `src/components/mobile/transactions/TransactionList.jsx` | 📈 +12.35 kB (+375.72%) | 3.29 kB → 15.63 kB `home/runner/work/actual/actual/packages/loot-core/src/client/actions/notifications.ts` | 📈 +121 B (+23.77%) | 509 B → 630 B `home/runner/work/actual/actual/packages/loot-core/src/client/reducers/notifications.ts` | 📈 +116 B (+20.32%) | 571 B → 687 B `src/components/mobile/transactions/Transaction.jsx` | 📈 +971 B (+18.53%) | 5.12 kB → 6.07 kB `src/components/Notifications.tsx` | 📈 +452 B (+6.78%) | 6.51 kB → 6.95 kB `home/runner/work/actual/actual/packages/loot-core/src/client/constants.ts` | 📈 +57 B (+4.41%) | 1.26 kB → 1.32 kB `src/components/schedules/ScheduleLink.tsx` | 📈 +94 B (+2.72%) | 3.38 kB → 3.47 kB `src/style/themes/development.ts` | 📈 +201 B (+2.70%) | 7.26 kB → 7.46 kB `src/style/themes/dark.ts` | 📈 +202 B (+2.69%) | 7.33 kB → 7.53 kB `src/style/themes/light.ts` | 📈 +201 B (+2.65%) | 7.4 kB → 7.59 kB `src/components/rules/RulesHeader.tsx` | 📈 +17 B (+2.44%) | 697 B → 714 B `src/style/themes/midnight.ts` | 📈 +174 B (+2.40%) | 7.07 kB → 7.24 kB `src/components/mobile/transactions/TransactionListWithBalances.jsx` | 📈 +70 B (+1.32%) | 5.17 kB → 5.23 kB `src/components/modals/ConfirmTransactionDelete.tsx` | 📈 +19 B (+1.05%) | 1.77 kB → 1.79 kB `src/components/schedules/DiscoverSchedules.tsx` | 📈 +59 B (+0.86%) | 6.66 kB → 6.72 kB `package.json` | 📈 +26 B (+0.84%) | 3.02 kB → 3.05 kB `home/runner/work/actual/actual/packages/loot-core/src/shared/transactions.ts` | 📈 +68 B (+0.81%) | 8.19 kB → 8.25 kB `src/components/Modals.tsx` | 📈 +91 B (+0.63%) | 14 kB → 14.09 kB `src/components/accounts/Header.jsx` | 📈 +98 B (+0.59%) | 16.27 kB → 16.37 kB `src/components/transactions/SimpleTransactionsTable.jsx` | 📈 +34 B (+0.53%) | 6.28 kB → 6.31 kB `src/components/payees/PayeeTableRow.tsx` | 📈 +17 B (+0.42%) | 3.91 kB → 3.93 kB `src/components/mobile/budget/CategoryTransactions.jsx` | 📈 +15 B (+0.39%) | 3.72 kB → 3.73 kB `src/components/rules/RuleRow.tsx` | 📈 +17 B (+0.32%) | 5.12 kB → 5.13 kB `src/components/payees/ManagePayees.jsx` | 📈 +17 B (+0.18%) | 9.37 kB → 9.39 kB `src/components/transactions/TransactionsTable.jsx` | 📈 +34 B (+0.05%) | 63 kB → 63.04 kB `src/components/mobile/accounts/AccountTransactions.jsx` | 📉 -4 B (-0.06%) | 6.3 kB → 6.3 kB `src/hooks/useSelected.tsx` | 📉 -78 B (-1.01%) | 7.52 kB → 7.44 kB `src/components/transactions/SelectedTransactionsButton.jsx` | 📉 -147 B (-1.80%) | 7.98 kB → 7.83 kB `src/components/accounts/Account.jsx` | 📉 -4.95 kB (-10.40%) | 47.6 kB → 42.65 kB
View detailed bundle breakdown
**Added** No assets were added **Removed** No assets were removed **Bigger** Asset | File Size | % Changed ----- | --------- | --------- static/js/index.js | 3.19 MB → 3.22 MB (+26.2 kB) | +0.80% **Smaller** Asset | File Size | % Changed ----- | --------- | --------- static/js/wide.js | 246.21 kB → 241.25 kB (-4.96 kB) | -2.02% static/js/narrow.js | 77.56 kB → 77.55 kB (-4 B) | -0.01% **Unchanged** Asset | File Size | % Changed ----- | --------- | --------- static/js/indexeddb-main-thread-worker-e59fee74.js | 13.5 kB | 0% static/js/resize-observer.js | 18.37 kB | 0% static/js/BackgroundImage.js | 122.29 kB | 0% static/js/usePreviewTransactions.js | 790 B | 0% static/js/AppliedFilters.js | 27.55 kB | 0% static/js/ReportRouter.js | 1.24 MB | 0%
github-actions[bot] commented 3 months ago

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count Total bundle size % Changed
1 1.14 MB → 1.14 MB (+184 B) +0.02%
Changeset File | Δ | Size ---- | - | ---- `packages/loot-core/src/mocks/budget.ts` | 📈 +680 B (+2.31%) | 28.71 kB → 29.38 kB `packages/loot-core/src/shared/transactions.ts` | 📈 +94 B (+0.91%) | 10.14 kB → 10.23 kB
View detailed bundle breakdown
**Added** No assets were added **Removed** No assets were removed **Bigger** Asset | File Size | % Changed ----- | --------- | --------- kcab.worker.js | 1.14 MB → 1.14 MB (+184 B) | +0.02% **Smaller** No assets were smaller **Unchanged** No assets were unchanged
youngcw commented 3 months ago

If you bulk change the account on the transactions they stay selected even after they are visually gone. That's maybe ok but feels a bit weird.

joel-jeremy commented 2 months ago

@youngcw Updated to clear selection after the batch operations and added undo notifications since I realized these operations can easily mess up the transactions if a mis click happened.

youngcw commented 2 months ago

Nice, I like the undo option. Im thinking it would be nice to leave the transactions selected so other changes can be made. Something like, edit the notes the change the category for a batch. I do that on occasion. Maybe we can leave this as is and see how it goes? What do you think?

joel-jeremy commented 2 months ago

Nice, I like the undo option. Im thinking it would be nice to leave the transactions selected so other changes can be made. Something like, edit the notes the change the category for a batch. I do that on occasion. Maybe we can leave this as is and see how it goes? What do you think?

Agreed, it makes sense to leave them selected. Updated the code :)

joel-jeremy commented 2 months ago

I also added a useUndo hook here which I plan to use in the budget page to show undo notification when applying goal templates.

youngcw commented 2 months ago

@MatissJanis @twk3 Could one of you look over this? With the new hooks and such this deserves more experienced eyes.

MatissJanis commented 2 months ago

Code looks good :+1:

Not sure if you missed this comment with a bug report + a feedback item on the "undo" notifications.

joel-jeremy commented 2 months ago

Small bug (not sure if related to this PR though): "link schedule" opens a modal, but after you select a schedule to link - the modal stays open. So it feels like nothing happened.

I can't seem to replicate this one. Maybe the recent commits fixed it?

Really like the thought that went into the undo feature! That's super nice. Though I find the undo notification quite disruptive. What if we instead added a "undo" button besides the "edit" button (maybe with a undo icon instead? just a thought)?

I'm planning to reuse the undo notification on the budget page too (https://github.com/actualbudget/actual/pull/3085) so I would like to keep the "undo" mechanism consistent across the pages for now. Once we have that running, we can definitely revamp the undo notification to be less disruptive or maybe we can just use less text on the notification so it takes up less screen real estate?

MatissJanis commented 2 months ago

This is what I see:

https://github.com/user-attachments/assets/b9ddb896-ed9e-4a3f-949e-8816379f0232

And now that I think about it more - there's also no "undo" notification for this action. But there is one for un-linking which is strange.

I'm planning to reuse the undo notification on the budget page too (https://github.com/actualbudget/actual/pull/3085) so I would like to keep the "undo" mechanism consistent across the pages for now. Once we have that running, we can definitely revamp the undo notification to be less disruptive or maybe we can just use less text on the notification so it takes up less screen real estate?

Less text works fine. And then we can nicely position it to be above the purple action bar. (+ would be really nice to auto-close it after a delay).

Teprifer commented 2 months ago

Yay more functionality! I did pick up two niggles when trying it out.

1) The edit notes Prepend and Append options give an incorrect undo notification as the wording indicates the note from one of the selected is what all were set to. When in reality each note add the text prepended/appended to their existing content.

Example: Pre-edit notes for the 3 transactions were in turn:

Edit notes Prepend 'prepend' Result: image


2) The Edit Amount dialog brings up the qwerty keyboard and not the numbers keyboard(Android, firefox incognito mode). Obviously this generates a red something internally went wrong notice if text is attempted to be entered.

joel-jeremy commented 2 months ago

Thank you for your feedback @Teprifer. I have fixed #1. I guess for number we're going to have to address that in a separate PR. Maybe I should disable bulk edit for amount for now - just like Date is currently disabled. WDYT?

Teprifer commented 2 months ago

Thank you for your feedback @Teprifer. I have fixed #1. I guess for number we're going to have to address that in a separate PR. Maybe I should disable bulk edit for amount for now - just like Date is currently disabled. WDYT?

Cheers, not strongly opinionated, although I edge slightly towards your suggestion of disabling as errors are unfriendly.

MatissJanis commented 2 months ago

Sorry, but I'm still really disliking the undo notification UI. As a minimum - could we make it so it doesn't overlay the action bar? To push it up.

Also I think the close delay should be increased to something much higher (~10s?). To give people time to react before it closes.

joel-jeremy commented 2 months ago

@MatissJanis Updated :)

MatissJanis commented 2 months ago

Thanks! The positioning is much better now. But now we have one more bug: if you perform an action - the undo notification opens up. But there is no way to close it. And without closing it - the entire page is not usable (clicks are blocked everywhere). So you have to wait for the notification to dissapear.

Would it be possible to allow clicking in the action bar if the notification is open? And perhaps to also add a "x" for the undo notification so it can be dismissed more quickly if the user wants to.

joel-jeremy commented 2 months ago

Floating action bar should now be clickable even if a notification is present :)

Teprifer commented 2 months ago

Appears to be temporarily not working? Can't long press or tap to edit/view a transaction.

Browser console: image

joel-jeremy commented 2 months ago

@Teprifer Nice catch! I went into a deep dive into react aria's codebase and found out that one needs to use PressResponder to handle both press and long press on react-aria-components's Button component - which is not documented anywhere in their docs.