Expand budgeting icon options #341
Conversation
jjmata
added
the
codex
ChatGPT Codex Connector
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughReplaced and reordered entries in the Category model's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/models/category.rb(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-14T14:22:37.038Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.038Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.
Applied to files:
app/models/category.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (1)
app/models/category.rb (1)
98-98: Icon change improves semantic clarity.Changing the Home Improvement category icon from "house" to "hammer" is a sensible improvement that better represents home improvement activities. The "house" icon is already used for "Mortgage / Rent" (line 99), so this also reduces duplication.
Note: This change only affects new installations via the
bootstrap!method, as it usesfind_or_create_by!which doesn't update existing records.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/models/category.rb(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-14T14:22:37.038Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.038Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.
Applied to files:
app/models/category.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci / test
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
app/models/category.rb (1)
96-96: LGTM! Better semantic fit for Home Improvement.Changing the icon from "house" to "hammer" is more semantically appropriate for the "Home Improvement" category, as "hammer" better represents repairs and renovations while "house" remains appropriately used for "Mortgage / Rent" (line 97).
| def icon_codes | ||
| %w[ | ||
| ambulance apple award baby banknote barcode bath battery bed-single beer bike | ||
| bluetooth bone book book-open briefcase building bus cake calculator camera | ||
| car cat circle-dollar-sign coffee coins compass cookie cooking-pot credit-card | ||
| dices dog drama drill droplet drum dumbbell film flame flower fuel gamepad-2 | ||
| gift glasses globe graduation-cap hammer hand-helping headphones heart | ||
| heart-pulse home house ice-cream-cone key landmark laptop leaf lightbulb | ||
| luggage mail map-pin mic monitor moon music package palette paw-print pen | ||
| pencil phone piggy-bank pill pizza plane plug power printer puzzle receipt | ||
| ribbon scale scissors settings shield shield-plus shirt shopping-bag | ||
| shopping-cart smartphone sparkles sprout stethoscope store sun tag target | ||
| tent thermometer ticket train trees trophy truck tv umbrella users utensils | ||
| video wallet waves wifi wine wrench zap | ||
| ambulance apple award baby badge-dollar-sign banknote barcode bar-chart-3 | ||
| bath battery bed-single beer bike bluetooth bone book-open briefcase building | ||
| bus cake calculator calendar-range camera car cat circle-dollar-sign coffee | ||
| coins compass cookie cooking-pot credit-card dices dog drama drill droplet | ||
| drum dumbbell film flame flower fuel gamepad-2 gift glasses globe graduation-cap | ||
| hammer hand-helping headphones heart heart-pulse home ice-cream-cone key | ||
| landmark laptop leaf lightbulb line-chart luggage mail map-pin mic monitor moon | ||
| music package palette paw-print pencil percent phone pie-chart piggy-bank pill | ||
| pizza plane plug power printer puzzle receipt receipt-text ribbon scale scissors | ||
| settings shield shirt shopping-bag shopping-cart smartphone sparkles sprout | ||
| stethoscope store sun tag target tent thermometer ticket train trees trophy truck | ||
| tv umbrella users utensils video wallet wallet-cards waves wifi wine wrench zap | ||
| ] | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
Verify "line-chart" icon name exists in lucide-rails.
A previous review flagged that Lucide uses "chart-line" not "line-chart", but this hasn't been updated. If "line-chart" doesn't exist in the lucide-rails gem being used, it will cause runtime issues when attempting to render categories with this icon.
Run the following script to check how "line-chart" and "chart-line" are used in the codebase and verify the correct icon name:
Additionally, search the web to confirm the correct icon name in lucide-rails:
🏁 Script executed:
#!/bin/bash
# Search for any existing usage of line-chart or chart-line icons
rg -n "line-chart|chart-line" --type=ruby -C2
# Check if there are any views or helpers that might show icon usage patterns
rg -n "lucide_icon" --type=erb -C2 | head -30Length of output: 4128
🌐 Web query:
lucide-rails line-chart vs chart-line icon name
💡 Result:
Lucide's canonical icon name is chart-line (see lucide.dev). Some third‑party wrappers/components (e.g. Blade UI Kit / other icon packs) expose it as lucide-line-chart or line-chart, so you may see both names depending on the package — but use chart-line for the official Lucide set. [1][2]
Sources:
- Lucide icon page (chart-line). [1]
- Example third‑party wrapper using lucide-line-chart. [2]
Use "chart-line" instead of "line-chart" in two locations.
Lucide's canonical icon name is "chart-line" not "line-chart". Two files have this error:
app/models/category.rbline 57: Change "line-chart" to "chart-line" in icon_codesapp/models/investment.rbline 28: Change "line-chart" to "chart-line" in Investment#icon method
Using the incorrect name will cause the icon() helper to fail or render the wrong icon when displaying categories and investments.
🤖 Prompt for AI Agents
In app/models/category.rb around lines 49-64 (and also update
app/models/investment.rb at line 28), replace the incorrect Lucide icon name
"line-chart" with the canonical "chart-line" so the icon() helper resolves
correctly; locate the two string occurrences and change them to "chart-line",
run the app or relevant view tests to confirm icons render as expected.
|
You didn't like the ones in #232 |
Quite the opposite ... I liked them and wanted to complete the last row that seemed empty. Then I got into cleaning them a bit. Not done yet. |
We're still in Seriously speaking. Let's try to make it so that if people are opinionated about this the list that is in the drop-down doesn't break other people's install. |
|
Need to deal with the mobile display as well:
|
I've also added some more icons in my <div class="flex flex-wrap gap-2 justify-center flex-col w-auto md:w-87">
<h4 class="text-secondary text-sm">Icon</h4>
- <div class="flex flex-wrap gap-0.5">
+ <div class="flex flex-wrap gap-0.5 max-h-52 overflow-auto">
<% Category.icon_codes.each do |icon| %>
<label class="relative">
<%= f.radio_button :lucide_icon, icon, class: "sr-only peer", data: { action: "change->category#handleIconChange change->category#handleIconColorChange", category_target:"icon" } %>It should limit the height of the div, and allow you to scroll to see all the icons. 😉 |
Summary
Testing
Codex Task
Summary by CodeRabbit