actual icon indicating copy to clipboard operation
actual copied to clipboard

Saved Filters Page

Open carkom opened this issue 1 year ago • 30 comments

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:

  • [x] - Fix Edit - currently not pulling existing data into the form
  • [x] - 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)
  • [x] - Form preview table seems to not be working (this is new, had it working a couple days ago) (DONE)
  • [x] - Obviously still needs a link the the filter menu in transactions tables (DONE)
  • [x] - 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)
  • [x] - Check that duplicate filter conditions don't already exist (eg. prevent duplicate exact conditions) (DONE)
  • [x] - Apply conditionsOp to Account/Reports pages when pulled from saved filters

carkom avatar Jun 12 '23 17:06 carkom

Deploy Preview for actualbudget ready!

Name Link
Latest commit 9983a7774456e7bb66b1ec72814e707206885d8d
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.

netlify[bot] avatar Jun 12 '23 17:06 netlify[bot]

👋 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?

MatissJanis avatar Jun 13 '23 19:06 MatissJanis

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.

carkom avatar Jun 13 '23 22:06 carkom

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.

youngcw avatar Jun 13 '23 22:06 youngcw

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).

carkom avatar Jun 13 '23 23:06 carkom

Hi all, I'm ready to put this into code review. Cheers!

carkom avatar Jun 21 '23 09:06 carkom

Hi @carkom! The autocomplete for the payee seems to be intermittent. The first time I tried adding a payee it didn't give me any autocomplete dropdown options. If I navigate away to the 'payee' page and then go back to the 'filters' page, the payees have a dropdown.

That's the only thing I noticed. I think it's an intuitive workflow once you know the feature is there :) Nice!

shall0pass avatar Jun 21 '23 11:06 shall0pass

Overall this is super cool! A bit of UI feedback before I start looking at the code

  • Selecting Filter → Saved, then clicking “Apply” causes a “saved (nothing)” filter token to be added which doesn’t seem to be correct
  • There should be a way to convert the current filter set to a saved filter from the account page (this can go into another PR if you want)
    • similarly, it would be nice to have the option to update and re-save a saved filter that has been applied, or save as a new filter.
    • EDIT: it looks like these show up in the main “…” menu. That’s a bit confusing IMO since they only appear once you’ve made a selection. I’m wondering if it would make sense to add another “…” menu to the row of filters once that’s visible to contain the edit/save actions, but I’m not sure that is the best UI.
  • “Existing filters will be cleared” should be left-aligned
  • I would remove the padding around the “Filter results:” and button section of the filter editor to better fit with other similar dialogs
  • IMO the error message should be something along the lines of “Filters must have a name” or “Filter name is required” instead of ”Filters must be named” (or, better, generate a suitable name automatically from the conditions

j-f1 avatar Jun 21 '23 12:06 j-f1

  Selecting Filter → Saved, then clicking “Apply” causes a “saved (nothing)” filter token to be added which doesn’t seem to be correct

This is duplicating existing process. For instance if you do the same with date or payee or category they all have the same result. I'm happy to fix this if it's a bug or unintended functionality but should likely be a different PR.

  * EDIT: it looks like these show up in the main “…” menu. That’s a bit confusing IMO since they only appear once you’ve made a selection. I’m wondering if it would make sense to add another “…” menu to the row of filters once that’s visible to contain the edit/save actions, but I’m not sure that is the best UI.

Yea, the functinality is all there. I wasn't sure where best to place it as well. Open to suggestions on where the best UI is.

I've added another option: a button that gets revealed when a filter is applied. Happy to hear anyone's thoughts on either implementation or if there's a a 3rd option let me know!

* “Existing filters will be cleared” should be left-aligned

Done

* I would remove the padding around the “Filter results:” and button section of the filter editor to better fit with other similar dialogs

Done. Also had to fix scroll bars.

* IMO the error message should be something along the lines of “Filters must have a name” or “Filter name is required” instead of ”Filters must be named” (or, better, generate a suitable name automatically from the conditions

Done

carkom avatar Jun 21 '23 16:06 carkom

I just wanted to showcase what I saw this morning so it doesn't get forgotten. payee

shall0pass avatar Jun 22 '23 02:06 shall0pass

I just wanted to showcase what I saw this morning so it doesn't get forgotten.

Yes, I saw your first note... I'm aware of the issue. Same thing happens with category. If you have any suggestions on a fix I'd appreciate the guidance. Cheers.

It also happens with categories in the Rules Editor.

carkom avatar Jun 22 '23 06:06 carkom

Hi! We love this feature over all but we’re not fully on board with adding a whole new page to manage filters. How do you feel about something like this:

  • change the “Add/Edit saved filter” to a dropdown menu with the following items:
    • if the filter is not yet saved, “Save” to create a new filter
    • if the filter is saved and not modified, “Rename” and “Delete”
    • if the filter is saved and modified, “Update existing” and “Save new”
    • “Clear” to remove all conditions (not convinced this should be here)
    • the dropdown title would either be “Not saved” if unsaved, the filter name if saved, and filterName + "*" if the filter is modified
  • ideally there would be some kind of affordance to quickly switch between saved filters but I’m not sure what that would look like, feel free to skip
  • remove the Filters page since all the filters can be explored from the accounts page
  • remove the “edit filter” modal since the filter can be edited inline, and then saved if desired
  • remove the “edit filter” button from the “…” menu

j-f1 avatar Jun 22 '23 18:06 j-f1

Yea I'm not set on the filters page, thought it might be useful.

I'm having trouble visualizing the ask. Can you share a mockup?

carkom avatar Jun 22 '23 19:06 carkom

Something like this. (let me know if you want a more detailed mockup / something else)

Screenshot_2023-06-22 18 22 09

j-f1 avatar Jun 22 '23 22:06 j-f1

* remove the “edit filter” modal since the filter can be edited inline, and then saved if desired

This makes me sad as I spent a lot of time on the modal to get it just right. That said, the new look is cleaner and more in line with Actuals principle of "discovered features" so my sadness is short-lived.

As a note: This type of detailed feedback is super helpful to understand how best to contribute. Without it, I wasted a lot of time developing things that I'm now just bulk erasing (still plenty of code I can re-use with the refactor though). I know both you and @MatissJanis mentioned some things similar to this previously, but this is the first time it's been clearly communicated in this amount of detail. It might also be that you all didn't know exactly what you were looking for until recently which is all good. I tried to engage with both of you by replying to your comments but in the absence of any replys back I forged ahead with my own ideas - which is on me.

No hard feelings here, just trying to figure out a way for us to all work as efficiently as possible. Cheers!

carkom avatar Jun 23 '23 12:06 carkom

I've commited an update. Removes the page and the sidebar item and the need for the modal. The buttons are all dead right now as I'd like to get the UI right before I link-up all the functionality. Let me know if there's anything that needs changing. Cheers!

I was debating with myself on the name of the menu (currently static name "Saved filters"). Other option I was playing with was changing from "save filter" to "edit filter" depending on if a saved filter has been loaded.

I like the idea of a "clear all conditions" option as well.

Thoughts?

carkom avatar Jun 23 '23 12:06 carkom

Please accept my personal thanks for all your hard work @carkom. I'm sorry for the extra “grief” you have had to go through. But I have to say that I do, like the others, hold dear to the fundamental Actual design principles.

This saved filters feature is absolutely my No 1 desired feature in Actual (and I speak as a 4 year user of Actual but regrettably not a Dev). The next thing I would love to see is some way to sort each filtered view by payee or by category and so on 🙏😁.

Many thanks again.

Kidglove57 avatar Jun 23 '23 13:06 Kidglove57

It's no grief, all part of the process. I'm enjoying it. Cheers!

carkom avatar Jun 23 '23 15:06 carkom

As a note: This type of detailed feedback is super helpful to understand how best to contribute. Without it, I wasted a lot of time developing things that I'm now just bulk erasing (still plenty of code I can re-use with the refactor though). I know both you and MatissJanis mentioned some things similar to this previously, but this is the first time it's been clearly communicated in this amount of detail. It might also be that you all didn't know exactly what you were looking for until recently which is all good. I tried to engage with both of you by replying to your comments but in the absence of any replys back I forged ahead with my own ideas - which is on me.

This is very good feedback, and I’m sorry you had to rip out code you spent time on. I definitely didn’t have a solid idea of what it should look like until you had the implementation you shared — that helped me see the strengths and weaknesses of the different options (in particular, I hadn’t considered placing the saved filters action button all the way to the right before, and having the actions be there instead of the regular “filters” button wasn’t something I had deeply considered before.


Heads up that I just merged a PR (#1066) that probably caused the merge conflicts you’re seeing. Let me know if you have any trouble adapting to the new React Router code patterns.


I took a look at your updated design and I really like it! It fits in very well with the rest of the UI. Two pieces of feedback:

  • I like the way you’ve laid out the actions in the menu. I have some concerns about their naming but we can address that later (interested to hear what Matiss thinks and also that should hopefully be easy to swap out since the underlying intended actions seem great)
  • For the “If all of these conditions match:” text — it feels a bit long and out of place on its own line. I think it would feel nicer if everything was on one line: “all▾ of payee is Extra Watery notes is Test”. What do you think about that?

I would definitely like to hear what @MatissJanis thinks too!

j-f1 avatar Jun 23 '23 18:06 j-f1

Just looked at the netlify demo. Overall: I'm very excited on where this is going! I would be totally happy to see this released once we iron out the bugs and small style issues. Let me know when you're ready and I'll do a bug-bash session to see if I spot any issue.

Great work!

MatissJanis avatar Jun 23 '23 19:06 MatissJanis

This is very good feedback, and I’m sorry you had to rip out code you spent time on. No worries at all, I should be able to use the code on #1162 so it won't go to waste.

  • For the “If all of these conditions match:” text — it feels a bit long and out of place on its own line. I think it would feel nicer if everything was on one line: “all▾ of payee is Extra Watery notes is Test”. What do you think about that? I'd say it's easier to understand as a new user with the longer line, but cleaner and more compact in the truncated form. If I had to vote I'd go for the longer one because I think it loses a bit of its intuitiveness the other way. Happy to hear other opinions on it.

Thanks for the feedback @j-f1 and @MatissJanis. I'll crack on with linking my functionality code to this UI for now and we can make any tweaks to it soon.

carkom avatar Jun 23 '23 22:06 carkom

Hey all, I have everything working as intended. Please don't spend any time with a code review just yet as I still need to clean up files from my previous implementation that are no longer used.

I would definitely welcome any reviews of the UI and any changes or bug fixes that need done. Cheers!

carkom avatar Jun 26 '23 15:06 carkom

Functionality seems great! Some UI feedback:

  • the “saved filters” button should not have a fixed width — we don’t do this anywhere else in the app so it looks a bit out of place with extra horizontal padding.
  • the save/rename modal should respond to the user pressing return to submit the form and click the “add” button. (If you put the whole thing in a <form> and mark the Button as type="submit", this should happen automatically)
    • also in that modal, the “Filter Name” heading should be put into a <label> so that clicking on it focuses the input field
    • also in that modal, the input field should auto-focus when it appears so the user can immediately start typing without having to click
  • IMO the heading at the top of the list should be moved into the button title:
    • “Filter not saved” or “Unsaved filter” when it currently says “NOT SAVED”
    • just the filter name when a filter has been applied
    • when the filter is modified, put (modified) or (changed) or something at the end of the filter name
  • the “clear all conditions” action feels a bit out of place, and I think it should be its own button since it doesn’t act on the saved filters in any way.
    • feel free to ignore this for now, I will think about this more and maybe code up an alternative UI for that action.
  • “Reload” should be renamed to “Revert changes” because “reload” doesn’t give me the same sense of it being a destructive action.
  • “Create new filter” should be “Save new filter” because the filters are saved somewhere else.

j-f1 avatar Jun 26 '23 16:06 j-f1

Functionality seems great! Some UI feedback: Thanks for the review!

* the “saved filters” button should not have a fixed width — we don’t do this anywhere else in the app so it looks a bit out of place with extra horizontal padding.

I've fixed this. If this statement is true, we may want to consider revising the "Apply" button in the filters list as it does not follow this logic. I've put an example solution into my PR.

* the save/rename modal should respond to the user pressing return to submit the form and click the “add” button. (If you put the whole thing in a `<form>` and mark the `Button` as `type="submit"`, this should happen automatically)

Good call out. Got it working.

  * also in that modal, the “Filter Name” heading should be put into a `<label>` so that clicking on it focuses the input field

👍

  * also in that modal, the input field should auto-focus when it appears so the user can immediately start typing without having to click

👍

* IMO the heading at the top of the list should be moved into the button title:

Yea, I tried it both ways. Ultimately I didn't like the UI changing button names everytime I selected something new. Happy to set this however we think is best.

* the “clear all conditions” action feels a bit out of place, and I think it should be its own button since it doesn’t act on the saved filters in any way.
  * feel free to ignore this for now, I will think about this more and maybe code up an alternative UI for that action.

Yea, I'm with you. It feels slightly out of place here, however it feels highly out of place in any other place on the page. Again, happy to move it as we see fit. I do think it is a neccessary function because if you have 5 filters applied, clicking to delete them 1 by 1 is tedious. I tried to keep the menu as consistant as possible (keeping things in the same places and in the same order). I also focused on grouping items based on their use.

* “Reload” should be renamed to “Revert changes” because “reload” doesn’t give me the same sense of it being a destructive action.

👍

* “Create new filter” should be “Save new filter” because the filters are saved somewhere else.

👍

carkom avatar Jun 26 '23 23:06 carkom

Ultimately I didn't like the UI changing button names everytime I selected something new. Happy to set this however we think is best.

I think it is valuable to see what filter you’re applying in the words you as a user chose (not just the actual filter tokens). But definitely open to opinions from other folks to help resolve this.

I do think it is a neccessary function because if you have 5 filters applied, clicking to delete them 1 by 1 is tedious. I tried to keep the menu as consistant as possible (keeping things in the same places and in the same order).

💯


I just pushed a change that matches the any/all dropdown with my vision; feel free to revert or change further: Screenshot_2023-06-26 19 23 57

I’m not convinced on whether the : is a good idea or not (same with the “of”)

j-f1 avatar Jun 26 '23 23:06 j-f1

Alright, I think the old modals code is mostly cleaned out. Let's get this over the finish line...

Ready for code review and any bug bashing anyone wants to throw at it.

carkom avatar Jun 27 '23 00:06 carkom

So here's an illustration of my concern regarding static menu button name vs changing with name of filter. This may be an extreme example that's not worth considering but I've found the users always find a way to break something...

The UI is really inconsistent when filters spill over into 2 rows and/or the filter name is needlessly long.

Static Example: (UI is very resilient to basically anything the user throws at it: long filter names are hidden in the menu, long filter conditions can fall into extra rows and/or hidden with "..." without any changes to the UI) image

Dynamic Example: (while smaller/cleaner footprint for short and simple filters (which I like), it gets wonky with more extreme examples: ConditionsOp menu falls into 2 rows, arrow in filter button menu is tiny, button size changes with each new row added/removed, etc.) image

carkom avatar Jun 27 '23 10:06 carkom

👋 This is amazing! Love the progress and love how it looks and works now!

Small bug in the reports page, but apart from that I did not spot any issues. Will do a code-review shortly.

Screenshot 2023-06-27 at 17 39 51

MatissJanis avatar Jun 27 '23 16:06 MatissJanis

Small bug in the reports page

That’s my fault, will take a look

j-f1 avatar Jun 27 '23 16:06 j-f1

I see 2 issues there:

  • if the filters wrap onto multiple lines, they should wrap around the all/any label: Screenshot_2023-06-27 13 06 58
  • if the name is too long, a reasonable max width should be set on the button and the text truncated with text-overflow: ellipsis Screenshot_2023-06-27 13 10 41

j-f1 avatar Jun 27 '23 17:06 j-f1