Closed (fixed)
Project:
Currency
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
21 Nov 2012 at 20:15 UTC
Updated:
24 Jul 2013 at 08:46 UTC
Jump to comment: Most recent file
Comments
Comment #1
xanoThe code is mostly based on Payment's payment_amount form element, with some minor changes, because Currency uses a different amount format, and some extra validation.
Before we can properly use this, we need locale-based amount display, though.
Comment #2
xanoMaybe we should also add a "currency_currency" element to select currencies, and a give the "currency_amount" element an option to retrieve the currency code to use from another form element.
Comment #3
xanoI'm thinking about moving validation to bartfeenstra/currency, because validating and parsing amounts from user input is complex business and I'd like to be able to get more feedback than just from within the Drupal community. I also haven't found any PHP internal function or 3rd party code that flexibly validates or parses such user input. Zend Currency only does calculations, conversions, and display, for instance.
The patch
is_numeric() (which is used by Drupal Commerce as well)
However
Comment #4
xanoThe code I just committed to bartfeenstra/currency parses user input to floats
Comment #5
xanoBefore the patch can be applied, a composer update needs to be performed and the patch needs to be rerolled to allow for recent changes in bartfeenstra/currency that were made to support this issue.
The patch adds a currency_amount form element that has three properties:
- currency code: if !== FALSE, then users can select the currency using a select element. Otherwise the currency is displayed.
- minimum amount: the amount entered by the user must not be less than this.
- maximum amount: the amount entered by the user must not be more than this.
Using CSS I tried to improve accessibility (hidden labels) and usability (currency and amount elements positioned next to each other).
Comment #6
amateescu commentedHmm.. can't we include the lib updates through composer inside the patch?
Comment #7
xanoWe could, but there are updates for more than one library. It wouldn't make sense to include all of them in the patch.
Comment #9
amateescu commented#5: currency_1847158_02.patch queued for re-testing.
I ran a
composer update --prefer-dist, was that the only thing holding back this patch?Comment #11
xanoRe-roll.
Comment #12
amateescu commentedCommitted to 7.x-2.x.. finally :)
Comment #13
xanoThe original patch did not save the amount as a float to $form_state. This patch adds that functionality + test update.
Comment #14
amateescu commentedCommitted the followup to 7.x-2.x.
Comment #16
xano