Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Donation amount component #248

Draft
wants to merge 6 commits into
base: 7.x-2.x
Choose a base branch
from
Draft

Conversation

torotil
Copy link
Contributor

@torotil torotil commented Oct 5, 2020

Aim is to improve the usability of configuring donation amount components. In order to do this we add a new customized webform component.

  1. Pre-defined values are entered as numbers (no need to care about the mapping of the label to the value).
  2. Functionality is based on the number webform component so it has the same functionality
    • Setting values using conditionals is possible
    • Configuration of the validation is possible (min / max amount)
    • Numeric operators for conditionals.

@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #248 (b3af0e6) into 7.x-2.x (9323fa0) will increase coverage by 0.26%.
The diff coverage is 67.15%.

Impacted file tree graph

@@              Coverage Diff              @@
##             7.x-2.x     #248      +/-   ##
=============================================
+ Coverage      46.58%   46.85%   +0.26%     
+ Complexity      1827     1823       -4     
=============================================
  Files            287      285       -2     
  Lines          10144    10225      +81     
  Branches         154      154              
=============================================
+ Hits            4726     4791      +65     
- Misses          5415     5431      +16     
  Partials           3        3              
Impacted Files Coverage Δ Complexity Δ
..._donation/src/FormBuilderElementDonationAmount.php 0.00% <0.00%> (ø) 2.00 <2.00> (?)
campaignion_donation/campaignion_donation.module 51.76% <40.00%> (+2.41%) 0.00 <0.00> (ø)
...ampaignion_donation/components/donation_amount.php 87.75% <87.75%> (ø) 0.00 <0.00> (?)
...email_to_target/campaignion_email_to_target.module 64.17% <0.00%> (-0.53%) 0.00% <0.00%> (ø%)
campaignion_bar/campaignion_bar.module 5.98% <0.00%> (-0.15%) 0.00% <0.00%> (ø%)
campaignion_layout/campaignion_layout.module 54.66% <0.00%> (ø) 0.00% <0.00%> (ø%)
campaignion_overlay/campaignion_overlay.module 92.85% <0.00%> (ø) 0.00% <0.00%> (ø%)
campaignion_giftaid/campaignion_giftaid.tokens.inc 100.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
campaignion_donation_amount/src/Utils.php
...donation_amount/campaignion_donation_amount.module
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9323fa0...65cf1b9. Read the comment docs.

@torotil torotil force-pushed the donation-amount-component branch 3 times, most recently from 31fef67 to b3af0e6 Compare October 5, 2020 15:53
$defaults['extra'] += [
'options' => [],
'other_option' => TRUE,
'other_text' => t('Other'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not "Other amount"? Seems a better fit as it can only be another amount, not another "something".

if ($component['extra']['options']) {
$element['#type'] = 'select_or_other';
$element['#select_type'] = 'radios';
$element['#options'] = drupal_map_assoc($component['extra']['options']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great if we can toggle whether the currency symbol gets prepended before the amount. This is a very common use case in rendering. (Just rendering it an hiding via styles would work too)

The currency symbol would be best wrapped in a span with an appropriate class...

$element['#type'] = 'select_or_other';
$element['#select_type'] = 'radios';
$element['#options'] = drupal_map_assoc($component['extra']['options']);
$element['#other'] = !empty($component['extra']['other_text']) ? check_plain($component['extra']['other_text']) : t('Other...');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: why not "Other amount"?

@torotil torotil marked this pull request as draft October 7, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants