Improve rules - add name, allow sorting, improve UI
This PR adds and improves a bunch of stuff around rules! I've been playing around with these more recently and thought they could use a few upgrades
- Add ability to name a rule - this is currently an optional field. I thought about doing some kind of database seed to give all current rules a name, something like "Custom Rule" and then require a name, but this felt a bit heavy handed. Happy to add this if others feel like its a good idea.
- Add sorting for rules, by name and date. I'd like to add some filtering options here eventually as well.
- Something I went back and forth on here is, when sorting desc name, if it should always show named rules first or not. This ended up requiring a bit more custom sql with
Arel(), so I opted to just keep the default behavior, which is null values first. Also happy to switch this up if that makes more sense. A checkbox option as a filter might be a middle ground here.
- Something I went back and forth on here is, when sorting desc name, if it should always show named rules first or not. This ended up requiring a bit more custom sql with
- Improve rule page and form design - On top of adding the name to the main card, I also added some new styling to kind of show the format of the rules of
IFthis,THENthis,FORthis period of time with some icons. This carries over to the form as well.
As a side note - I see that #2154 is in progress and will probably change a bunch of things (like the icon() helper). If it makes more sense to wait for this (and my other PRs) until after that is merged, I can handle the changes to cut down on conflicts/ongoing changes.
Rule page:
Form:
Video showing creation of a rule + sorting:
https://github.com/user-attachments/assets/da47dd8b-2ad1-4834-bc97-3aec9af7bb29
@ahatzz11 overall looking good! I'll take a closer look at this PR and #2174, #2175 once I get our pre-launch stuff done today/tomorrow!
Good to know about Brakeman!
I do like like updated_at over created_at as the default, I always like seeing more recently touched things as well - that has been updated now.
Personally, I think being able to sort alphabetically, especially when tied to a name, would be super helpful. Some of this comes down to how people use the naming convention, but I can see myself having a lot of rules and being able to give them meaningful order other than a timestamp would be great, e.g.
Auto - merchants
Auto - categories
Income - {job1}
Income - {job2}
Category - {something}
Category - {something}
I think it's probably a bit hard to see how alphabetical sorting would fit into usage today since there aren't any names at this point. There was also a feature request for this today - #2198, so I think others would have similar cases. Having ability to search would be an awesome additional feature.
Would love to hear design feedback! I think it looks great, but don't want to veer too far off a larger vision 🙂
@ahatzz11 those are some valid points. I do agree if we had a name property in there then alphabetical sorting could allow users to create their own naming convention for organization.
@justinfar thoughts on all of this?
Fantastic work here @ahatzz11, I made some slight adustments in terms of the UI (so that it's more consistent with other views across the product) but other than that not much else from my end in terms of feedback.
Changes made:
- Removed icons for if/then/for
- Changed icon for condition group
- Changed sorting by name/date to dropdown selection and then ascending/descending is seperated onto an icon button
- Followed your format for how rules are displayed, but they're seperated by a divider (rather than seperate containers)
- Disabled rules are given a
text/secondarycolor
Link to the the Figma file: https://www.figma.com/design/lonJmVk3HYkwZoIO7xYP2w/Maybe-App--Community-?node-id=6370-1960
Preview of the changes made:
@justinfar thanks for the feedback! I'll work on this over the next couple of days. Can you link the figma file? I think a copy/paste got missed in your comment.
One question I see right away - on your screenshot the font-mono on the IF/THIS/FOR is gone, do you want that to be normal font on the edit screen? Or should we keep those on the edit to match the list page? (I personally like the font-mono, but again happy to do whatever you think is best for the bigger vision!)
@ahatzz11 Ah thanks for letting me know, just updated the original comment, here's the link again just in case: https://www.figma.com/design/lonJmVk3HYkwZoIO7xYP2w/Maybe-App--Community-?node-id=6370-1960
One question I see right away - on your screenshot the
font-monoon theIF/THIS/FORis gone, do you want that to be normal font on the edit screen? Or should we keep those on the edit to match the list page? (I personally like the font-mono, but again happy to do whatever you think is best for the bigger vision!)
Yep I'd like us to keep using the normal font on the edit rules view because I'd like us to use mono only for cases where we have numericals/data viz/value comparison etc (where it makes sense)
@justinfar While I'm working on this I've got a few design questions!
- I can make the form.select match your design with
bg-transparent, but do we want to keep that consistent with other dropdowns and have it look something like this? I can't seem to find another example. The transparent does look nice, but without cursor pointer I think is tough to notice it's a clickable area. - Right now I noticed that none of our selects have
cursor-pointer- is that something we should have across the board?
Alright the current code is now updated with most of the design changes:
- [x] Removed icons for if/then/for
- [x] Changed icon for condition group
- [x] Changed sorting by name/date to dropdown selection and then ascending/descending is separated onto an icon button
- Still waiting for some answers from Justin above
- [x] Followed your format for how rules are displayed, but they're seperated by a divider (rather than separate containers)
- [x] Disabled rules are given a text/secondary color
~@zachgoll I could use some help/advice on the implementation on displaying the rules with a divider. I messed around with this for quite a while and couldn't get to anything perfect or that I really liked, but the current code is about as close as I could get it. The shadow-border-xs utility is great for being around a single thing (like the div that holds all the cards), but once I add it to each card with 0 spacing between it, I get a sloppy border because the border is in the margin and it becomes 2× bigger than intended. I also tried to make some new shadow-divider-{size} utilities, but didn't want to start making new utilities if we didn't think there would be other reasons for them. I ended up landing on using the divider from the sidebar. It isn't either (although close enough that most eyes wouldn't notice) <div class="h-px bg-alpha-black-100 w-full"></div>~
Big edit: I decided to forego the shadow-box here because we're not really making a shadow around a card anymore, but instead having a border around each rule. I ended up just using <div class="border-b border-subdued"></div> to make the divider line, utilizing the existing border styling from the border-utils. I think this turned out excellent:
One note - the figma says the border is subdued but that color seemed to not match very well, so I opted for border-secondary. I'm not sure if this is just a mismatch between figma and the design system or if I'm missing something.
Additionally, I updated some text- and bg- attributes and rules now has a much nicer dark mode
One weird thing I've found with sorting - because it's within a form right now, changing either option submits the form. A side effect of this, because of the direction toggle, is that the direction always toggles. This means if you're on name/asc and you move to updated_at, the direction becomes desc. If you're on name/desc and you move to updated_at, the direction becomes asc.
This is a bit strange and not quite what I expect to happen. I've got a couple ways to solve this, and wanted some feedback:
-
Move from a button and a
form.hidden_fieldto aLinkComponentthat just changes the route:<%= render LinkComponent.new( icon: "arrow-up-down", size: :sm, title: "Toggle sort direction", href: rules_path(sort_by: @sort_by, direction: @direction == "asc" ? "desc" : "asc"), variant: :icon ) do %>This has a slightly odd side effect that it's a link, so browsers show the link on hover. I'm not too bothers by this (obviously there are a lot of other links in the app).
-
Add a new controller (
sort_direction_controller):import { Controller } from "@hotwired/stimulus"; export default class extends Controller { static targets = ["direction"]; toggle(event) { event.preventDefault(); const directionInput = this.directionTarget; directionInput.value = directionInput.value === "asc" ? "desc" : "asc"; this.element.requestSubmit(); } }In theory this could be reused in other places that had sorting direction, but I don't think that exists anywhere else at the moment.
@justinfar While I'm working on this I've got a few design questions!
- I can make the form.select match your design with
bg-transparent, but do we want to keep that consistent with other dropdowns and have it look something like this? I can't seem to find another example. The transparent does look nice, but without cursor pointer I think is tough to notice it's a clickable area.- Right now I noticed that none of our selects have
cursor-pointer- is that something we should have across the board?
@ahatzz11
Would rather keep it matching the design (as it'll stand out a bit too much within this context if we use the same input style).
As for the selects not having cursor-pointer that's definitely something we should address and have across the board, as it's an affordance that lets people know they can interact.
Hi @ahatzz11,
As we discussed earlier in #2198, I found another bug in the conditional group rule UI. I’ve created a new issue with a screen recording and explanation (#2221).
Since you're working on improving the UI as part of this task, could you take a look and see if the issue is resolved by your changes? If not, perhaps you can identify the root cause and address it?
Thanks!
@justinfar
Would rather keep it matching the design (as it'll stand out a bit too much within this context if we use the same input style).
As for the selects not having
cursor-pointerthat's definitely something we should address and have across the board, as it's an affordance that lets people know they can interact.
I have updated the dropdown to match the figma design! I do like it blending in a bit more.
One final question on the design - The figma for rules currently has a full width border separating out each rule. I have an implementation for this with border instead of a shadowbox, but as I was clicking around the site I noticed it's a bit different than the other pages where there are a list of many items of the same type, e.g. tags and categories. Specifically - these use a shadowbox around the main card and then a not-quite-full-width divider (which alleviates the issue of two shadow borders overlapping and changing the colors a bit):
I ended up changing the rules list to match tags/categories in my latest push, but wanted to run this change by you before saying this PR is 100% ready to go. I personally like the exact match to the other pages. Here is a comparison:
Let me know what you think!
@justinfar we all set on this one?
@zachgoll I'll fix this merge conflict here quick so it's ready once justin gives a 👍
sorry just seeing this now!
I ended up changing the rules list to match tags/categories in my latest push, but wanted to run this change by you before saying this PR is 100% ready to go. I personally like the exact match to the other pages. Here is a comparison:
This works better than my solution, didn't even notice the shadowbox overlap so thank you for bringing it to my attention! Appreciate you looking out for consistency :)
all good to go on this one, excellent work @ahatzz11!
@zachgoll Alright this one is ready to go from my side - I added a new bg-divider utility that we can use on dividers/rulers so dark mode is handled nicely as well. I started down the rabbit hole to clean up some of the other rulers/dividers in the app, but I'll keep that as a separate PR so this one doesn't hang out for too long. Let me know if you think this utility belongs elsewhere!
New bg-divier utility in dark mode (implemented on rules):
New bg-divider not implemented on categories (yet, I'll do this in a followup):
@ahatzz11 I haven't deeply thought through this, but just an FYI, I had started following this pattern for dividers in a few places:
<hr class="border-tertiary" />
https://github.com/maybe-finance/maybe/blob/050d5ebaadaaf35f7e51d0111197426ebc689899/app/components/menu_item_component.html.erb#L2
Not a big deal either way, just thought I'd call it out.
How can I apply the rule to categorize every transaction using AI without using an IF statement?
