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

Feature: add option to disable one time donations for recurring enabled forms #6886

Merged
merged 15 commits into from
Aug 28, 2023

Conversation

jonwaldstein
Copy link
Contributor

@jonwaldstein jonwaldstein commented Aug 17, 2023

Description

This updates the donation amount recurring donation settings to:

  1. Add setting for enable/disable one-time donations.
  2. Remove donation choice in favor of implicit derivative
  3. Update labels for admin facing terms "Frequency" and "Donations"

Affects

The amount block and the donation form.

Visuals

Screenshot 2023-08-24 at 12 25 43 PM

Testing Instructions

  • Create a new v3 form w/ recurring installed
  • Enable recurring and toggle the new settings
  • Publish changes and make sure the donation form reflects the change and works properly

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@jonwaldstein jonwaldstein marked this pull request as ready for review August 17, 2023 21:49
@jonwaldstein
Copy link
Contributor Author

@kjohnson this is ready for review.

cc: @JasonTheAdams @jdghinson

@JasonTheAdams
Copy link
Contributor

I kind of feel like it should be in the "Billing Period" list instead of its own toggle. But I'm open to @jdghinson's suggestion.

@jonwaldstein
Copy link
Contributor Author

I kind of feel like it should be in the "Billing Period" list instead of its own toggle. But I'm open to @jdghinson's suggestion.

I understand that from a visual standpoint looking at the block - but technically one-time is not considered a recurring billing period. Clicking a billing period is auto-opting into a recurring donation. Furthermore, this is just our interpretation of the amount block that lists the donation options as buttons. Some folks have requested the amount field to work more like v2 where the billing periods are a checkbox & dropdown - in which case a one-time billing period option would not make sense because that would not be listed in the opt-in options.

@JasonTheAdams
Copy link
Contributor

Some folks have requested the amount field to work more like v2 where the billing periods are a checkbox & dropdown - in which case a one-time billing period option would not make sense because that would not be listed in the opt-in options.

I believe you're referring to this post by a single person, who hasn't replied to my question. I'd really need to be convinced that going from 1 click to 2-3 clicks for a recurring decision is in fact better.

I'd be open to changing the "Billing Period" label to "Payment Options" or something like. I get what you're saying from a technical perspective, but from a builder UX perspective I feel like having a checkbox in the same place as all the other checkboxes that affect those options makes the most sense. As the user is checking them it's visible changing the button group, but then to change the "One time" is somewhere else. I think that's less intuitive.

I'm going to ping Jeffrey for his perspective.

@jdghinson
Copy link
Contributor

jdghinson commented Aug 18, 2023

Some folks have requested the amount field to work more like v2 where the billing periods are a checkbox & dropdown - in which case a one-time billing period option would not make sense because that would not be listed in the opt-in options.

I believe you're referring to this post by a single person, who hasn't replied to my question. I'd really need to be convinced that going from 1 click to 2-3 clicks for a recurring decision is in fact better.

I'd be open to changing the "Billing Period" label to "Payment Options" or something like. I get what you're saying from a technical perspective, but from a builder UX perspective I feel like having a checkbox in the same place as all the other checkboxes that affect those options makes the most sense. As the user is checking them it's visible changing the button group, but then to change the "One time" is somewhere else. I think that's less intuitive.

I'm going to ping Jeffrey for his perspective.

I agree with @JasonTheAdams on this. It makes sense if its included in the billing period options for a better experience. Maybe the label "Billing period" would be an issue if we are bringing "one-time" as an option, so we could look at a different label to avoid this "possible" confusion with users. We could be overthinking this as well cos I believe having the "one time" option in the Billing period wouldn't be that confusing as we perceive it.

@JasonTheAdams
Copy link
Contributor

I personally don't find it too confusing as is. Maybe "Billing Options" would be a good label as well, to avoid the "period" term?

@angelablake
Copy link
Contributor

I agree with @JasonTheAdams and @jdghinson, and I don't think "billing period" is confusing when it includes "one time." I think we should keep that label, but I'm open to other labels as long as they're not too vague & make sense.

IMO, when there's a trade-off between what makes sense to users and what makes sense for development, we should choose users as often as possible.

@angelablake
Copy link
Contributor

I personally don't find it too confusing as is. Maybe "Billing Options" would be a good label as well, to avoid the "period" term?

Billing Options sounds okay to me.

@jonwaldstein
Copy link
Contributor Author

jonwaldstein commented Aug 18, 2023

@JasonTheAdams regarding this comment:

I believe you're referring to this post by a single person, who hasn't replied to my question. I'd really need to be convinced that going from 1 click to 2-3 clicks for a recurring decision is in fact better.

I would caution against negating customer feedback even if its only reported by one person. It's quality feedback, regardless of how many votes (in fact they did reply with an explanation). We are making this change to the donation form with the assumption it's a better UX which is fine, but should consider other options & feedback for the future. It's a pretty big change from v2 forms. Having a more descriptive form could also be a good UX - wherein it's more of a question if they want to make a recurring donation instead. I'm not trying to convince you that this person's feedback is the better experience - but it should be considered as its coming from a customer who has their donors in mind.

@JasonTheAdams @jdghinson @angelablake

Something I noticed is that we have an option for "one-time" as the default billing period within the recurring options. So technically what the group is learning toward would be valid. Personally what I find confusing is the order of declarative recurring options with "one-time" in there. Particularly when you are choosing "Billing Interval" it seems to describe the next option for "Billing Period". "Every.. month", "Every...one-time" 😕

Screenshot 2023-08-18 at 12 47 05 PM

@JasonTheAdams
Copy link
Contributor

I would caution against negating customer feedback even if its only reported by one person. It's quality feedback, regardless of how many votes (in fact they did reply with an explanation).

Yes, always good to weigh customer feedback carefully. I apologize, I'm a bit rushed today so I'm erring on the side of brevity, which tends to sound dismissive. I appreciate their feedback and have weighed it carefully with support/customer success feedback — wherein the vast majority of folks have requested the exact UI we're moving towards. Based on their feedback, I don't see us implementing the checkbox UI any time soon.

Personally what I find confusing is the order of declarative recurring options with "one-time" in there. Particularly when you are choosing "Billing Interval" it seems to describe the next option for "Billing Period". "Every.. month", "Every...one-time" 😕

Fair point. I think one thing to keep in mind here is that the admin is already choosing to enable recurring donations, so the chances of them unticking everything except one-time is highly unlikely. So I think the fact that there will always be recurring options will help keep the other options understandable.

@angelablake
Copy link
Contributor

@jonwaldstein makes some extremely valid points. The "every" part is a little awkward, and I don't want to discount this person's feedback entirely. But, I think we should roll with this and then keep a close eye on feedback or add it to the user testing backlog. @jdghinson what do you think?

@jdghinson
Copy link
Contributor

@jonwaldstein makes some extremely valid points. The "every" part is a little awkward, and I don't want to discount this person's feedback entirely. But, I think we should roll with this and then keep a close eye on feedback or add it to the user testing backlog. @jdghinson what do you think?

Yeah @jonwaldstein has some valid points I agree with however we should roll with this and see how it turns out. Yeah @angelablake we can add it to the user testing backlog

@jonwaldstein
Copy link
Contributor Author

@JasonTheAdams appreciate the awareness & clarification.

@angelablake @jdghinson can you guys clarify what we're "rolling with"? lol

@angelablake
Copy link
Contributor

@JasonTheAdams appreciate the awareness & clarification.

@angelablake @jdghinson can you guys clarify what we're "rolling with"? lol

LOL Sorry, I meant having the "One-Time" option as a check box under Billing Period. I would like to "roll with" that.

@jonwaldstein
Copy link
Contributor Author

This PR has been updated to add "One Time" as a billing option 56c3949

One side effect of this approach ill point out, is it added some additional complexity around supplying billing periods to the donation form that has the potential to conflict with our amount field design.

Let me try to explain...

Before, we were requiring at least one billing period option to be enabled. This is because (per design) our amount field gives donors "options" as buttons for the billing periods. Since "One Time" was not in the billing period options we were confidently supplying more than one option for the donor to choose between. Now that "One Time" is in the block options, we now need to require at least two billing period options to be enabled. If we did not do this, a form could end up with one "option" selected on the form.

Here's a visual:

If we did not enforce the 2 billing period options, this could happen:

Screenshot 2023-08-18 at 4 20 42 PM

If we do enforce the 2 billing period options, this would be prevented:

Screenshot 2023-08-18 at 4 21 06 PM

In this PR, I made the executive decision to enforce the minimum of 2 billing options with additional checks to make sure the first example does not happen. Feel free to challenge that if you wish 😄

Copy link
Member

@kjohnson kjohnson left a comment

Choose a reason for hiding this comment

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

Having pulled this down I was, at first, confused by the minimum of 2 billing options. I then realized that what I was trying to achieve (only monthly) was the equivalent of "admin choice" - but that is actually a different setting (that I overlooked).

@jdghinson @JasonTheAdams @angelablake @jonwaldstein what are you thoughts on simplifying the settings by removing the "donor choice" vs "admin choice"?

If there is only one selected period then it is essentially "admin choice".
If there is more than one selected period then it is essentially "donor choice".

Now that "one time" donations are included in the period options I think we can make the "Donation Choice" setting moot.

@angelablake
Copy link
Contributor

@jdghinson @JasonTheAdams @angelablake @jonwaldstein what are you thoughts on simplifying the settings by removing the "donor choice" vs "admin choice"?

If there is only one selected period then it is essentially "admin choice".
If there is more than one selected period then it is essentially "donor choice".

Now that "one time" donations are included in the period options I think we can make the "Donation Choice" setting moot.

I actually really like that, and I think it would make more sense, especially to new users. Possible tradeoffs: Will existing users understand they can still effectively make it "admin choice" even if there's not a specific setting that says that? How much longer will it take to develop?

@JasonTheAdams
Copy link
Contributor

Thanks for thinking outside the box, @kjohnson! I'd like to get Ben and Amanda's thoughts on that option.

Thinking out loud, the juxtaposition here is whether the decision should be implicit or explicit, and which is more intuitive. The explicit option (how it is now) is clear in the sense that it's all laid out, but more decisions can mean more confusion. The implicit option is great for those who intuit the implications — "well, if there's only one, there's no donor decision to be made" — but confusing for those who don't get it.

I think my conclusion is that I like the implicit option, but I think it's very important that the shift in behavior is reflected in the form builder — which I believe it already is! That is, when I check multiple, I can see the buttons showing/hiding in the form in real time. When I reduce it to one, it displays the "donation is monthly" donor notice in real time. The important thing here is immediate feedback to the user.

As a note, I think if the admin has only a single option and attempts to uncheck the last period, such that there would be none checked, it should simply do nothing. That is, I click the box and it refuses to uncheck.

@kjohnson
Copy link
Member

Will existing users understand they can still effectively make it "admin choice" even if there's not a specific setting that says that?

Good question. Originally, the setting correlated to whether or not an opt-in checkbox was shown, but that isn't really relevant anymore. The visual update would communicate the changes via the UI, so the feedback loop is very short for understanding the resulting form.

How much longer will it take to develop?

I would say the additional development is minimal and I could make the changes to this PR today (unless @jonwaldstein wanted to implement it).

@kjohnson
Copy link
Member

Also... what @JasonTheAdams very eloquently said while I typed up my terse response. 😅

@jonwaldstein
Copy link
Contributor Author

@kjohnson good thoughts. One big difference between the donor and admin options is the setting for what the admin defined "billing period" is, as the admin defined settings are supposed to be fixed. The could potentially add confusion if we were merging these because donor just has "default billing period" with the purpose of allowing the donor to choose.

Another observation is our definition of "Donor's choice" is changing. Previously, donor's choice meant "allows the donor to choose whether or not they would like to make their donation recurring". Since one-time was always there, donor's choice included a way to opt in (from one-time to recurring). With the addition of "One-Time" in the billing period options, we are putting the responsibility on the admin to define both their billing period options for the donor to choose between and being able to (or not) opt in to a recurring donation.

@angelablake
Copy link
Contributor

@jonwaldstein Not sure I understand your first point, but the second one (putting more responsibility on the admin) is definitely one we need to keep in mind.

However, we've been defaulting to position that it's easier to add settings later than remove them, and I think that holds true here.

@JasonTheAdams @jdghinson and I hashed this out over the past hour and a half & came up with the following solution:

  • Get rid of the Donation Choice setting. (Donor can choose from whatever options the admin enables in Billing Period.)
  • As few as 1 single billing period can be selected. (Since we're getting rid of Donation Choice, they need to be able to select a single option under Billing Period.)
    -- When only 1 billing is selected, the Default Billing Period setting is hidden. (Because it's irrelevant.)
  • When multiple Billing Period options are selected, and the Default Billing Period setting is set to One Time, the "This donation occurs every ..." message does not display in the editor at all.
  • When only 1 Billing Period option is selected, and it is not One Time, we should make sure the "This donation occurs every ..." message is displayed. (It might already work this way.)

That doesn't solve the issue of increased admin responsibility, but it should reduce confusion on both the admin and donor sides while keeping the settings as simple as possible. Jason's out to lunch for a bit, but @jonwaldstein let us know if there are any additional complications with doing it that way.

@kjohnson your input on the new direction is very welcome too.

@jonwaldstein
Copy link
Contributor Author

@angelablake I think you covered my point here:

When only 1 billing is selected, the Default Billing Period setting is hidden. (Because it's irrelevant.)

I'm fine with this direction, but I would like to request some updated designs from @jdghinson before proceeding with implementation. It's a significant enough change in the UI that we should record and sync up to existing designs to avoid any more pivots during dev.

@angelablake
Copy link
Contributor

@jonwaldstein That makes sense. @jdghinson we'll need to work on prioritizing this over other things in this sprint. I'll add an item to your backlog & I would argue it's higher priority than Funds & Designations.

@jdghinson
Copy link
Contributor

@jonwaldstein That makes sense. @jdghinson we'll need to work on prioritizing this over other things in this sprint. I'll add an item to your backlog & I would argue it's higher priority than Funds & Designations.

Alright I will work on this then @angelablake

@jonwaldstein
Copy link
Contributor Author

Call summary
Date: August 23, 2023
Attendees: @jonwaldstein @JasonTheAdams @jdghinson @angelablake @Benunc @taylorfromteamgive

We decided to move forward with the direction outlined by the designs below:

  • Add setting for enable/disable one-time donations.
  • Remove donation choice in favor of implicit derivative
  • Update labels for admin facing terms "Frequency" and "Donations"
Screenshot 2023-08-24 at 1 07 03 PM Screenshot 2023-08-24 at 1 07 15 PM

@jonwaldstein
Copy link
Contributor Author

@JasonTheAdams @kjohnson this PR has been updated with the latest direction and is ready for review.

Copy link
Contributor

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

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

Good work, @jonwaldstein! I just had one question in the code, otherwise it doesn't look like we're updating the v2 to v3 migration code in this PR, and we probably need to, right?

@jonwaldstein
Copy link
Contributor Author

Good work, @jonwaldstein! I just had one question in the code, otherwise it doesn't look like we're updating the v2 to v3 migration code in this PR, and we probably need to, right?

Good question, I can't find any migration logic for the recurring attributes so i'm assuming we still need to add that. Maybe @kjohnson can confirm? Otherwise we'll need to create a new PR to handle that either in core or give-recurring.

Copy link
Contributor

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

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

This looks good to me, @jonwaldstein! Please touch base with @kjohnson. If the recurring migration logic doesn't exist, go ahead and merge and we'll need to add that in another PR. If it already exists, please update it here before merging.

Copy link
Member

@rickalday rickalday left a comment

Choose a reason for hiding this comment

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

Passed manual QA tests.

@jonwaldstein jonwaldstein merged commit 4bbd975 into epic/givewp-3.0 Aug 28, 2023
20 checks passed
@jonwaldstein jonwaldstein deleted the feature/recurring-disable-onetime branch August 28, 2023 19:58
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.

6 participants