feat(donate-block): hide buttons if only one frequency is available

[adekbadek]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

If the Donate block is set to render just one frequency, the frequency buttons section should not be displayed.
Instead, a frequency label is added, if applicable.

Before	After

How to test the changes in this Pull Request:

1. On trunk, insert Donate blocks (see below for a copy-paste)
2. Observe the frequency choosing UI is rendered, with a single button
3. Switch to this branch, reload editor & frontend – observe the selection UI is not rendered and frequency labels added instead, where applicable

Blocks code for testing

<!-- wp:newspack-blocks/donate {"className":"is-style-default","manual":true,"amounts":{"once":[9,20,90,20],"month":[7,15,30,15],"year":[84,180,360,180]},"disabledFrequencies":{"once":false,"month":true,"year":true},"defaultFrequency":"once","minimumDonation":5} /-->
<!-- wp:newspack-blocks/donate {"className":"is-style-default","manual":true,"amounts":{"once":[9,20,90,20],"month":[7,15,30,15],"year":[84,180,360,180]},"disabledFrequencies":{"once":true,"month":false,"year":true},"minimumDonation":5} /-->
<!-- wp:newspack-blocks/donate {"className":"is-style-default","manual":true,"amounts":{"once":[9,20,90,20],"month":[7,15,30,15],"year":[84,180,360,180]},"disabledFrequencies":{"once":true,"month":true,"year":false},"defaultFrequency":"year","minimumDonation":5} /-->
<!-- wp:newspack-blocks/donate {"className":"is-style-default","manual":true,"tiered":false,"amounts":{"once":[9,20,90,20],"month":[7,15,30,15],"year":[84,180,360,180]},"disabledFrequencies":{"once":false,"month":true,"year":true},"defaultFrequency":"once","minimumDonation":5} /-->
<!-- wp:newspack-blocks/donate {"className":"is-style-default","manual":true,"tiered":false,"amounts":{"once":[9,20,90,20],"month":[7,15,30,15],"year":[84,180,360,180]},"disabledFrequencies":{"once":true,"month":false,"year":true},"minimumDonation":5} /-->
<!-- wp:newspack-blocks/donate {"className":"is-style-default","manual":true,"tiered":false,"amounts":{"once":[9,20,90,20],"month":[7,15,30,15],"year":[84,180,360,180]},"disabledFrequencies":{"once":true,"month":true,"year":false},"defaultFrequency":"year","minimumDonation":5} /-->

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1208106329606187

[kevinzweerink]

Hmm, I'm a bit hesitant about this — it seems too easy for a publisher to accidentally set up something where their description doesn't match the behavior, i.e. a donation block is set to weekly, and the publisher mistakenly adds copy above it saying "Make a one-time donation." Even if the user notices that it's actually recurring in the checkout flow, they might feel mislead and abandon the donation.

What do you think about this idea: in cases where only one frequency is selected, that title is expanded to "Make a {frequency} donation."? That way it's more clear, but removes the risk of mislabeling a recurring product.

[adekbadek]

@kevinzweerink – the original request from a publisher was to hide the label. WDYT about making this an option? Then a publisher can hide the label (as an explicit step) and add their own using e.g. a paragraph block above the Donate block?

[kevinzweerink]

Sure, we could do that. I don't love this direction in general because I feel like it introduces more hidden functionality to the donation block, which already features a lot of configuration options that you have to know to look for. At this stage I'm not sure it makes sense to revisit the general approach though.

If we want to go ahead and implement this, here's a mockup of where I think the control should live — I would make it a toggle that appears under the tier options when only one tier is selected; this way it's clear which label will be removed.

[ronchambers]

Hi Adam and Kevin. I was invited by Claudiu to get up to speed on a number of Newspack products and dev environment in order to help out as needed. He recommended I give a review to a number of PRs, including this one.

I'll provide my review here if it helps in anyway 😃

Overall I think this is a tough one. I understand publishers wanting to remove an unneeded label when there is only one "tab", but I also understand that it could cause a publisher to accidentally sell a frequency that the user doesn't see.

How about the following:

I noticed there are other configurations of the Donate Block that already show a "redundant" frequency in both the tab and in the content area.

Please see these two screenshots:

So maybe this same "redundancy" can be applied to the following layouts. This way, if there is only one tab/label and it is auto-hidden, the user can still see the frequency in the content box.

Please see these 3 mockups:

In the end, my thoughts would be to do:

1. Keep the code in this PR that automatically hides the single tab when there is only one label/frequency. I think this is good and "makes sense" to not have an unneeded single tab. Also I would assume (but can't confirm) that from an Accessibility point of view it's probably best to remove the single tab or at minimum remove the single tab's click-action/link so accessible browsers are not confused by a single tab that doesn't change anything when clicked.

2. Add the redundant frequency into the block content area for the 3 "mockup" layouts above. This will make all 5 layouts more similar in design. Be sure to keep this new text on all screen sizes, don't hide on mobile.

3. With this approach we should be able to avoid any new toggles since I agree there is quite a bit going on in the configurations.

I hope this might help :-)

Thanks!

[adekbadek]

Thanks @ronchambers ! I like the idea. WDYT @kevinzweerink ?

[adekbadek]

Thanks @ronchambers! I've applied your suggestions.

[ronchambers]

@adekbadek - Thanks for these changes. If you ever need me to QA/review sooner, feel free to let me know a deadline or ping me on slack too :-)

Overall I really like the new "per month" and "per year" labels on all the different styles. And I really like the new sentence "Donation amount per month" and "Donation amount per year". I also noticed that a single-tab will disappear when there is only one tab. Awesome!

I have two pieces of feedback:

1. I noticed a small bug for the "default" style with the attached configuration. When clicking "other" the label should be hidden/removed like in the other styles.

2. I think we should put a label for "one-time or once" like you just did for "per month" / "per year". For the sentence I would recommend "Donation amount (one-time)" with parenthesis. I'll attached the idea. (I prefer the word "one-time" instead of "once" if possible...) If it's easier to put the "()" parenthesis around the label too, that seems fine with me.

[adekbadek]

I noticed a small bug for the "default" style with the attached configuration. […]

Good catch! Fixed in bbed5ad

I think we should put a label for "one-time or once" like you just did for "per month" / "per year".

I'm not a fan of how this looks. I think it's fine to assume that a donation is a one-time donation.

[ronchambers]

Nice, I retested and bug is fixed. As for adding "one-time" or "once", personally I think something is needed since users and publishers "could" get confused, but I also think what you said is reasonable in that most people would probably assume a donation is "one-time" unless stated otherwise. So cool with me either way :-)

[ronchambers]

I tested the functionality and it works as described. I did not do a code review - I just tested via the wp-admin / blocks editor.

[dkoo]

@adekbadek is this ready to merge?