gavin/maybe
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 28 Wiki Activity

Allow 2 decimals when adding a balance #614

Merged
pieterbeulque merged 1 commits from pieterbeulque/fix-613 into main 2024-04-15 23:27:40 +08:00
Conversation 5 Commits 1 Files Changed 1 +2 -1
pieterbeulque commented 2024-04-13 03:42:35 +08:00 (Migrated from github.com)
Copy Link

Fixes #613

This should fix money fields throughout the app.

One thing I'm not sure of is that not all currencies allow decimals, so this should maybe be dynamic based on the currency this input is in? Maybe that's a property on that Money.new(0, money&.currency || Money.default_currency) line too?

Fixes #613 This should fix money fields throughout the app. One thing I'm not sure of is that not all currencies allow decimals, so this should maybe be dynamic based on the currency this input is in? Maybe that's a property on that `Money.new(0, money&.currency || Money.default_currency)` line too?
🎉 1
zachgoll (Migrated from github.com) reviewed 2024-04-13 21:15:48 +08:00
app/helpers/application_form_builder.rb
@@ -39,2 +39,3 @@
placeholder: Money.new(0, money&.currency || Money.default_currency).format
placeholder: Money.new(0, money&.currency || Money.default_currency).format,
step: "0.01" # Not all currencies have 2 decimal places
}
zachgoll (Migrated from github.com) commented 2024-04-13 21:15:44 +08:00
Copy Link

RE dynamically setting based on currency:

That's a good point and I'm wondering if we add an instance method in Money::Currency:

def step
    return 1 if @minor_unit_conversion.nil? || @minor_unit_conversion == 1
    1.0 / @minor_unit_conversion.to_f
end

And then use it here:

step: (money&.currency || Money.default_currency).step
RE dynamically setting based on currency: That's a good point and I'm wondering if we add an instance method in `Money::Currency`: ``` def step return 1 if @minor_unit_conversion.nil? || @minor_unit_conversion == 1 1.0 / @minor_unit_conversion.to_f end ``` And then use it here: ``` step: (money&.currency || Money.default_currency).step ```
pieterbeulque (Migrated from github.com) reviewed 2024-04-15 14:38:12 +08:00
app/helpers/application_form_builder.rb
@@ -39,2 +39,3 @@
placeholder: Money.new(0, money&.currency || Money.default_currency).format
placeholder: Money.new(0, money&.currency || Money.default_currency).format,
step: "0.01" # Not all currencies have 2 decimal places
}
pieterbeulque (Migrated from github.com) commented 2024-04-15 14:38:12 +08:00
Copy Link

I'm just curious how that will behave when you change the dropdown value? If the default currency is EUR or USD it will be set to 0.01, but if you change that dropdown value to JPY, it won't auto-update the money field, right? Do we need to add in some JS to handle that on change?

I'm just curious how that will behave when you change the dropdown value? If the default currency is EUR or USD it will be set to `0.01`, but if you change that dropdown value to JPY, it won't auto-update the money field, right? Do we need to add in some JS to handle that on change?
zachgoll (Migrated from github.com) reviewed 2024-04-15 23:27:25 +08:00
app/helpers/application_form_builder.rb
@@ -39,2 +39,3 @@
placeholder: Money.new(0, money&.currency || Money.default_currency).format
placeholder: Money.new(0, money&.currency || Money.default_currency).format,
step: "0.01" # Not all currencies have 2 decimal places
}
zachgoll (Migrated from github.com) commented 2024-04-15 23:27:25 +08:00
Copy Link

Good point, wasn't thinking about that. I think your solution works for the time being, so I'll merge this and I just opened up #625 to capture this additional requirement.

Good point, wasn't thinking about that. I think your solution works for the time being, so I'll merge this and I just opened up #625 to capture this additional requirement.
pieterbeulque (Migrated from github.com) reviewed 2024-04-16 01:10:23 +08:00
app/helpers/application_form_builder.rb
@@ -39,2 +39,3 @@
placeholder: Money.new(0, money&.currency || Money.default_currency).format
placeholder: Money.new(0, money&.currency || Money.default_currency).format,
step: "0.01" # Not all currencies have 2 decimal places
}
pieterbeulque (Migrated from github.com) commented 2024-04-16 01:10:23 +08:00
Copy Link

Sweet, cheers. Curious to learn what'd be the modern Rails way to do this.

Sweet, cheers. Curious to learn what'd be the modern Rails way to do this.
zachgoll (Migrated from github.com) reviewed 2024-04-16 01:22:07 +08:00
app/helpers/application_form_builder.rb
@@ -39,2 +39,3 @@
placeholder: Money.new(0, money&.currency || Money.default_currency).format
placeholder: Money.new(0, money&.currency || Money.default_currency).format,
step: "0.01" # Not all currencies have 2 decimal places
}
zachgoll (Migrated from github.com) commented 2024-04-16 01:22:07 +08:00
Copy Link

Same, adding a JS controller just for this seems overkill, but I can't think of any other solution myself

Same, adding a JS controller just for this seems overkill, but I can't think of any other solution myself
Sign in to join this conversation.
Reviewers
No Reviewers
pieterbeulque
zachgoll
Labels
Clear labels
📆 Planned
Not ready to be worked on yet, but planned for future
 💻 Self Hosted only
Issues pertaining to self-hosted versions of Maybe
 ⚙️ Config
ℹ️ Needs info
Further information is requested
 📱 Mobile
Bugs exclusively happening on mobile devices
 1️⃣ High Priority
High priority issues will be worked on next
 🚨 Urgent
Issues that are urgent for app functionality
 3️⃣ Low priority
Contributions accepted, but Maybe team will not be working on this in the near term
 2️⃣ Medium Priority
Community contributions accepted, Maybe team only works on if there are no high priority items open
 ⚡ Performance
Performance issues to address
 🎨 UI / UX
Issues related to UI / UX interactions and design.
 💎 Bounty
💰 Rewarded
📊 Data issue
Issues where the app functionally works, but the data is missing or inaccurate.
 🙋 Bounty claim
$1.5K
Community
This is a great issue for community members to work on
 dependencies
Pull requests that update a dependency file
 Design
Hosted only
Hosted app only
 Required self hoster data migration
Self hosters must run data migrations to resolve this issue
 ruby
Pull requests that update Ruby code
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
gavin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gavin/maybe#614
Delete Branch "pieterbeulque/fix-613"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?