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

Saved Filters Page #1122

Merged
merged 73 commits into from
Jul 2, 2023
Merged

Saved Filters Page #1122

merged 73 commits into from
Jul 2, 2023

Conversation

carkom
Copy link
Contributor

@carkom carkom commented Jun 12, 2023

Hey all, this is still under construction. Just wanted to share my progress and get any feedback as I go so as to not go down any wrong roads.

DONE:

  • New sql table to house the list (Big shout out to @jlongster for setting this up for me)
  • Filters sidebar menu under the "More" heading
  • New filters landing page to display all the currently saved filters
  • Filters form for creating filter (includes any/all logic)
  • Delete menu item
  • OnClick pulls up the Edit menu but needs work *

TODO/Bugs:

  • - Fix Edit - currently not pulling existing data into the form
  • - Add "Filter Filters" box at the top so you can quickly find your filter (DONE)
  • Auto adjust row height (I played around with this for days and couldn't get it working right) (Depreciated)
  • - Form preview table seems to not be working (this is new, had it working a couple days ago) (DONE)
  • - Obviously still needs a link the the filter menu in transactions tables (DONE)
  • - Add "Save Filter" and "Edit Filter" option in the Accounts menu (DONE)
  • Consider addition of "Save Filter" and "Edit Filter" option in the Reports menu (Postponed)
  • - Check that duplicate filter conditions don't already exist (eg. prevent duplicate exact conditions) (DONE)
  • - Apply conditionsOp to Account/Reports pages when pulled from saved filters

@netlify
Copy link

netlify bot commented Jun 12, 2023

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 9983a77
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/64a09cbc1a6d0c0008104c08
😎 Deploy Preview https://deploy-preview-1122--actualbudget.netlify.app/
📱 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.

@rich-howell rich-howell changed the title <WIP> Saved Filters Page [WIP] Saved Filters Page Jun 12, 2023
@MatissJanis
Copy link
Member

👋 Overall: great stuff! The ability to save filters and re-use them is long overdue.

However, I don't think we should build a new page for managing them. One of the design principles of Actual is - simplicity first with progressive feature discovery for more advanced customers. Take for example the notes field - it is shown in various parts of the product when you hover a specific element. It is not shown by default. Only if you provide notes and save - only then the icon is always visible.

Similarly for saved filters: I think instead of reserving prime real-estate for them, we could build them more as a discoverable feature. So for example: you add some filters to the transaction page. At that point a "save" button appears. Clicking that maybe opens a modal where you type in the name of the filter. If you want to edit or delete a filter - select it in the dropdown to apply it to the current state. And then do any modifications you want. Or click "delete".

What do you think about that?

@carkom
Copy link
Contributor Author

carkom commented Jun 13, 2023

What do you think about that?

I get it. Your suggestion makes sense. It could be pretty visible (eg. a purple button pops up next to the "collapse transactions" button) or it could be hidden under the "export" option in the "..." menu.

That said, I'm happy doing it either way, just need concensus from the community on how we want to implement it. My plan was to do both - allowing creation and editing of saved filters on the accounts page via modals but also having a place to see them all in one place and manage them as needed.

Personally I like the option to be able to manage the filters in a single place without having to deal with hidden menus and the like. If having the page link hidden in the "more" drop-down sidebar is considered to be too visible we could also look at burying it in the settings page somewhere.

@youngcw
Copy link
Member

youngcw commented Jun 13, 2023

I like the idea of saving a current filter vs building one then using it after. I do worry that people could easily create multiple filters that do the same thing with different names if its not easy/obvious to look at what they do and modify them.

@carkom
Copy link
Contributor Author

carkom commented Jun 13, 2023

I do worry that people could easily create multiple filters that do the same thing with different names if its not easy/obvious to look at what they do and modify them.

I plan to implement a block/warning for any filters that fall into this category (eg. duplicating existing saved filter conditions exactly).

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@carkom
Copy link
Contributor Author

carkom commented Jul 1, 2023

Fixed the margins, I think I've fixed all bugs so far. Let me know if there's any others.

Only thing that's outstanding right now is the ctrl+z after create/update/delete actions. The undo works but it doesn't change the rendered window which is confusing. The only thing I've managed to do is to refresh the entire page after an undo, not sure this is what's wanted. So my choices are refresh after successful undo action or disable the undo actions. Thoughts?

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@MatissJanis
Copy link
Member

Ok, so I see three blocking issues now:

  1. Saved Filters Page #1122 (comment)
  2. undo (ctrl+z) operation does not work as expected - it should undo the last operation (delete/rename/edit) without reloading the page
  3. a condition_op: any filer opens correctly in the transaction list page, but opens as condition_op: all in reports page

We're nearly there.. :)

@carkom
Copy link
Contributor Author

carkom commented Jul 1, 2023

1. [Saved Filters Page #1122 (comment)](https://github.com/actualbudget/actual/pull/1122#discussion_r1248055289)

Fixed

2. undo (ctrl+z) operation does not work as expected - it should undo the last operation (delete/rename/edit) without reloading the page

Okay, the delete undo works fine with no refresh (this is the most neccessary IMO). The create undo is not neccessary because you can just delete the newly created filter via the UI. Same with rename, instead of undoing it you can just rename it back to previous name. Is that an acceptable UX for this feature and those acitons?

3. a `condition_op: any` filer opens correctly in the transaction list page, but opens as `condition_op: all` in reports page

This now pulls the conditionsOp from the saved filter on the Reports Header in all report pages.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

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
13 2.34 MB -> 2.35 MB (+12.87 KB) +0.54%
Changeset
File Δ Size
src/components/filters/FiltersMenu.js 🆕 +9.55 KB 0 B -> 9.55 KB
src/components/filters/SavedFilters.js 🆕 +5.88 KB 0 B -> 5.88 KB
src/components/autocomplete/SavedFilterAutocomplete.js 🆕 +1.22 KB 0 B -> 1.22 KB
../loot-core/src/client/data-hooks/filters.tsx 🆕 +371 B 0 B -> 371 B
src/hooks/useFilters.ts 📈 +422 B (+64.72%) 652 B -> 1.05 KB
src/components/reports/Header.js 📈 +432 B (+15.36%) 2.75 KB -> 3.17 KB
src/components/util/GenericInput.js 📈 +238 B (+7.41%) 3.14 KB -> 3.37 KB
src/components/reports/graphs/net-worth-spreadsheet.tsx 📈 +157 B (+5.21%) 2.94 KB -> 3.09 KB
src/components/accounts/Account.js 📈 +2 KB (+4.81%) 41.55 KB -> 43.55 KB
src/components/reports/graphs/cash-flow-spreadsheet.tsx 📈 +157 B (+4.23%) 3.63 KB -> 3.78 KB
../loot-core/src/shared/rules.ts 📈 +44 B (+1.22%) 3.51 KB -> 3.55 KB
src/components/ManageRules.js 📈 +33 B (+0.21%) 15.59 KB -> 15.62 KB
src/components/common.tsx 📈 +12 B (+0.11%) 10.38 KB -> 10.39 KB
src/components/modals/EditRule.js 📈 +7 B (+0.04%) 17.46 KB -> 17.47 KB
src/components/reports/NetWorth.js 📉 -111 B (-2.98%) 3.64 KB -> 3.53 KB
src/components/reports/CashFlow.js 📉 -159 B (-3.96%) 3.92 KB -> 3.76 KB
src/components/accounts/Filters.js 🔥 -9.02 KB (-100%) 9.02 KB -> 0 B
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
static/js/main.js 1.04 MB -> 1.05 MB (+12.32 KB) +1.16%
static/js/reports.chunk.js 19.58 KB -> 20.13 KB (+566 B) +2.82%

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
static/js/457.chunk.js 387.21 KB 0%
static/media/Inter.var.woff2 317.25 KB 0%
static/media/Inter-italic.var.woff2 239.29 KB 0%
static/media/Inter-roman.var.woff2 221.86 KB 0%
static/media/bg.svg 116.73 KB 0%
static/js/969.chunk.js 12.94 KB 0%
static/js/resize-observer-polyfill.chunk.js 8.12 KB 0%
static/css/main.css 6.08 KB 0%
index.html 1.68 KB 0%
asset-manifest.json 1.47 KB 0%
static/media/browser-server.js 963 B 0%

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

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
2 1.97 MB -> 1.97 MB (+2.21 KB) +0.11%
Changeset
File Δ Size
packages/loot-core/src/server/filters/app.ts 🆕 +4.41 KB 0 B -> 4.41 KB
packages/loot-core/src/server/aql/schema/index.ts 📈 +157 B (+1.50%) 10.23 KB -> 10.38 KB
packages/loot-core/src/shared/rules.ts 📈 +67 B (+1.27%) 5.15 KB -> 5.21 KB
packages/loot-core/src/server/main.ts 📈 +52 B (+0.08%) 62.34 KB -> 62.4 KB
packages/loot-core/src/server/accounts/transaction-rules.ts 📈 +16 B (+0.08%) 20.17 KB -> 20.19 KB
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
kcab.worker.js 1014.86 KB -> 1017.07 KB (+2.21 KB) +0.22%

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
xfo.xfo.kcab.worker.js 1004.04 KB 0%

Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

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

Perfect! Thank you so much for this and for your patience with all the feedback!

I'll allow @j-f1 to review too since he's been quite involved, but on my end this looks good to go.

Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for adding this!

@j-f1 j-f1 merged commit f5ea9d0 into actualbudget:master Jul 2, 2023
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed 🔍 Ready for Review labels Jul 2, 2023
@carkom
Copy link
Contributor Author

carkom commented Jul 2, 2023

Woo!! Thanks all.

@Kidglove57
Copy link

Looks great in the preview! Cant wait for the full release.
Especial thanks @carkom !!

@trafico-bot trafico-bot bot added 🔍 Ready for Review and removed ✨ Merged Pull Request has been merged successfully labels Jul 2, 2023
@carkom carkom deleted the filters branch July 3, 2023 09:21
MatissJanis pushed a commit that referenced this pull request Jul 3, 2023
Adding background update to cleared/uncleared boxes on accounts page.
This is needed due to Accounts page background change in previous PR
(#1122)
TomAFrench added a commit to TomAFrench/actual that referenced this pull request Jul 4, 2023
* master: (34 commits)
  chore: add types to `crdt` package (actualbudget#1076)
  Fix layout of the management app with the demo bar in place (actualbudget#1267)
  ♻️ (select) removing 2x usages of the Select component (actualbudget#1259)
  Fix transaction list scrolling behavior (actualbudget#1260)
  🐛  fix link-schedule option in transaction table (actualbudget#1250)
  cleared/uncleared background update (actualbudget#1265)
  Fix calculation of how many table rows to render (actualbudget#1262)
  ♻️  moving more components out of common.tsx (actualbudget#1257)
  ♻️  moving some components from common.tsx to separate files (actualbudget#1248)
  🐛  fix toggling of balances in all-accounts view (actualbudget#1252)
  🐛 (mobile) reduce the size of account cards (actualbudget#1247)
  Fix electron export issue (actualbudget#1242)
  Saved Filters Page (actualbudget#1122)
  🔧  cancel previous CI runs if a new push is made (actualbudget#1251)
  .gitattributes Check line endings for tsx files (actualbudget#1246)
  Fix importing transfers from YNAB4/5 (actualbudget#1224)
  Auto-close the local/nordigen picker modal after creating an account (actualbudget#1219)
  Run “Handle completed feature requests” in pull_request_target (actualbudget#1243)
  Add electron options to bug-report.yml (actualbudget#1239)
  Add onClick handlers to the schedule and transaction icons in the transaction list (actualbudget#1228)
  ...
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
Adding background update to cleared/uncleared boxes on accounts page.
This is needed due to Accounts page background change in previous PR
(actualbudget#1122)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants