Donations block: Make currency and amounts editable #16593

mmtr · 2020-07-27T14:19:01Z

Part of Automattic/wp-calypso#42856

Changes proposed in this Pull Request:

Adds the ability to edit the currency and amounts in a donations block.

Jetpack product discussion

pbMlHh-dW-p2

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

With a WordPress instance of your choice, install Jetpack Beta and activate this branch. Or visit https://jurassic.ninja/create/?jetpack-beta&branch=update/donation-block-edit-amount.
Connect Jetpack to WordPress.com with a paid plan.
Make sure Jetpack beta blocks are visible by going to /wp-admin/options-general.php?page=companion_settings and activating JETPACK_BETA_BLOCKS.
Note that if folks are using the sandbox store method (PCYsg-lW4-p2) to also set the JETPACK__SANDBOX_DOMAIN value.
Go to Posts > New and insert a Donations block.
Make sure the block toolbar displays the current currency. Clicking on it should display a dropdown for changing it.
Try changing the currency.
Make sure the amounts automatically change to new defaults based on the minimum amount valid for a transaction as follows:
- 1st tier: Minimum amount x10 (e.g. USD 5).
- 2nd tier: Minimum amount x30 (e.g. USD 15).
- 3rd tier: Minimum amount x200 (e.g. USD 100).
Make sure you can edit the amounts by clicking in any of them.
Try inserting an invalid value for an amount (e.g. a value that doesn't contain any number or a value below the minimum amount). Invalid characters should be removed.
Save the post and reload the editor.
Make sure the new amount is preserved.

Proposed changelog entry for your changes:

N/A

mmtr · 2020-07-27T14:21:29Z

@Automattic/serenity folks, you're welcome to hop onto this PR and push commits addressing any feedback you might have.

jetpackbot · 2020-07-27T14:33:08Z

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16593

Scheduled Jetpack release: August 4, 2020.
Scheduled code freeze: July 28, 2020

Generated by 🚫 dangerJS against c5c6769

extensions/blocks/donations/attributes.js

gwwar · 2020-07-27T20:41:59Z

extensions/blocks/donations/controls.js

+	const { currency, amounts, monthlyPlanId, annuallyPlanId, showCustomAmount } = attributes;
+	const [ editedAmounts, setEditedAmounts ] = useState( amounts );
+
+	const minAmount = minimumTransactionAmountForCurrency( currency );


We can wait on this for follow up, but we can maybe build a new currency validation package, since format-currency has the inverse job of making sure currency displays correctly given some known amount from the server.

As a reminder, we'll need to also mirror logic on the server API checks.

gwwar · 2020-07-27T20:53:13Z

extensions/blocks/donations/controls.js

+		if ( ! amount ) {
+			isValidAmount = false;
+		}
+		amount = parseFloat( amount );


Leaving some general reminders for us when working with money:

JavaScript is unfortunately all floating point (so we end up needing to do string->float), but for later work on the server, as a general reminder:

use ints to the desired precision level (eg a number in the smallest unit eg cents or fractions of a cent)

Avoid type conversions until the end of computations to avoid compounding rounding errors. As a simple example, with floats (0.1+0.2) > 0.3 evaluates to true.

gwwar · 2020-07-27T21:57:07Z

Good start here @mmtr ✨! I think we need to revisit the design here for editing tiers. For primary actions we'll want to be able to do this in the edit view, the block options in the sidebar are for advanced, optional settings, or providing another way to do something they can already do in the block. (For example, notice how we don't put every text field that's editable as an option in the sidebar settings. It's more intuitive for folks to double click on text).

One way of doing this, when clicking on a tier button, we transform this into a plain text field or number field. Validation can run when we click outside of the area.

cc @danhauk @sixhours @davemart-in in case y'all had thoughts

davemart-in · 2020-07-28T14:46:25Z

I'll leave this for @danhauk to weigh in.

danhauk · 2020-07-28T18:04:18Z

As @gwwar noted, we should make the buttons directly editable. This was the intention with the earlier mockups but things may have gotten confused and/or lost in the shuffle of reshaping and scoping. Here's an older mockup of what that would look like (note: I still have a link to manage donations in Calypso so please ignore that):

I modeled the behavior after the core button block where simply clicking into the button text would make it editable. A plain text field with number validation would be better IMO. The number field is sometimes cumbersome to use. The currency symbol would be uneditable, similar to the custom amount field at the bottom of the form.

mmtr · 2020-07-29T08:15:15Z

Thanks @danhauk!

In regards to the amount validation, how we should highlight errors to users? Say a user enters a word (since it's a text field) or an invalid number (such an amount below the minimum threshold). Should we keep the invalid value user and indicate somehow in the UI there is an error (e.g. change the border color to red)? Or should we replace the invalid value with the previous valid value once the user focus another area of the editor?

matticbot · 2020-07-29T14:10:27Z

Caution: This PR has changes that must be merged to WordPress.com
Hello mmtr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D47185-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

mmtr · 2020-07-29T14:19:35Z

Just pushed an update that makes the amounts directly editable within the editor as suggested above. The currency selector has been moved away as well from the sidebar to the block toolbar.

Also added some styles so the border color of the inputs changes when hovering them to make more clear they are editable, although I have the feeling there is room for improvement there. @danhauk feel free to propose any other idea!

Didn't include anything for highlighting invalid amounts since I guess we can work on that on a follow-up PR once we have more feedback about the desired behavior. So far, the amounts are validated and only stored as attributes if they are valid.

danhauk · 2020-07-29T19:50:31Z

This is looking great @mmtr! As for the validation, what are your thoughts on using an HTML input pattern to take advantage of browser validation rather than a custom solution? I'm honestly not sure how the interaction would feel here. Are there any existing block examples you can think of that we could model the same behavior?

I'll think a little bit on what a custom solution would look like.

fgiannar · 2020-08-03T10:36:07Z

Hi @mmtr ,

Nice work! Tested this (Chrome, Safari and Firefox) and can verify it works as expected.

When testing this with the Gutenberg plugin activated, the following notices appeared:

Finally, some UX related comments:

I was unable to edit the "Donate" button styles (color, background color, border-radius etc)
It wasn't very clear to me that some elements' text changes via switching tabs (eg Donate button text and block title) vs "Choose an amount". It does make perfect sense, but when for example a changed the "Donate" button text, I expected it to also change for monthly/yearly donations. Then after getting this, I expected that the same would happen when changing the "Choose an amount" text. Instead, it changed in all tabs.

mmtr · 2020-08-03T10:56:44Z

Thanks for the review @fgiannar!

When testing this with the Gutenberg plugin activated, the following notices appeared:

Do you see the same notices when testing this with Jetpack master? They seem to be unrelated to changes here.

I was unable to edit the "Donate" button styles (color, background color, border-radius etc)

Yup, this is expected at this moment. It'll be definitely nice to have the ability to customize the button styles, but it's not required for the initial launch of the Donations block. In a follow-up, we'll try to reuse either the core button block or the JP button block so we can get the styling controls out of the box.

It wasn't very clear to me that some elements' text changes via switching tabs

I'll defer on @danhauk for this. I guess we have to approaches here:

All tabs are independent, so we can have different labels on each of them. This will require to expand the block attributes so all of the ones storing labels are triplicated (one for each interval).
Or we keep in sync all labels that don't depend on the selected interval that are common to all tabs, while keeping independent the ones that are different in the default design (such as the heading label).

I opted for the latter in this implementation, but I don't have any strong preference here.

changed the "Donate" button text, I expected it to also change for monthly/yearly donations

I don't think we can automatically change the button label for other tabs, because it can cause semantic issues. Let's say the label in the monthly donations is built dynamically as $buttonLabel + ' monthly'. If a user changes the label to "Want to donate? Click here", the button label auto-generated for the monthly/yearly donations will display "Want to donate? Click here monthly" which is grammatically incorrect (and probably can get worse in other languages).

fgiannar · 2020-08-03T11:22:49Z

Do you see the same notices when testing this with Jetpack master? They seem to be unrelated to changes here.

Those notices were only reproduced in a post with a Donation block. I had tested it in an empty blog post and couldn't see them, so I assumed there was a relation to the Donation block. Since you checked it and can confirm it is unrelated, please ignore my comment.

I don't think we can automatically change the button label for other tabs, because it can cause semantic issues.

Yes, you are right in terms of dynamically generated content. I was only expecting the static text to replace the dynamically generated one in all tabs. Eg all buttons share the same static text: "Want to donate? Click here".

If you decide to defer any further UX related opts for a next release, please feel free to mark this as "Ready to merge", otherwise we could add the "Needs Design Review" label.

Thanks for your prompt response!

mmtr · 2020-08-03T14:40:53Z

As per discussion with @danhauk in p1596458577169300-slack-C0Q664T29, we want to keep common items in sync while more specific items should not be synced.

Items to sync

✅ Text before fixed amounts (e.g. "Choose an amount").
✅ Text before custom amount input (e.g. "Or enter a custom amount").
❌ Donate buttons labels (e.g. "Donate"). They should still have defaults that are specific to tabs ("Donate", "Donate monthly", "Donate yearly"), but changing any of those would override and sync across all tabs.

Items to not sync

✅ Heading (e.g. "Make a one-time donation").
❌ Fixed amounts.
❌ Text before donate button (e.g. "Your contribution is appreciated.").

Items marked with ✅ are currently covered in this PR, while items with ❌ need additional implementation. Would folks be open to keep things as they are now and do a follow-up for improvements? Don't want this PR to become much more complex, and since the block is still in beta we have flexibility for shipping small and iterative changes.

kwight · 2020-08-03T23:03:16Z

Would folks be open to keep things as they are now and do a follow-up for improvements? Don't want this PR to become much more complex, and since the block is still in beta we have flexibility for shipping small and iterative changes.

Yes, followups please! Let's keep iterating forward while it's beta.

@mmtr Fantastic, this feels even more bullet-proof. I removed references to the red border in the description, since we no longer see them (invalid characters are removed on focus change). Really great work 🎉

resolved

mmtr · 2020-08-04T09:38:54Z

@fgiannar I added the "Ready to merge" label as advised, but I think the "Required review" still needs an explicit approval from your end (or anyone in @Automattic/jetpack-crew) to unblock merging.

fgiannar

LGTM

…ic/jetpack into add/jetpack-lazy-images-package * 'add/jetpack-lazy-images-package' of github.com:Automattic/jetpack: (40 commits) Lodash: Revert to previous version (#16735) New class to handle async XML-RPC requests (#16674) Social Previews: Sidebar design updates (#16725) Update E2E locator for classic connection flow (#16708) Woo Services: update to use existing Jetpack plugin install tools (#16672) Admin Page: avoid blank dashboard when notice is not a string (#16721) Admin Page: update string still using old i18n format (#16722) Social Previews: Add sidebar UI (#16633) Fix recurring payments block disconnecting (sometimes) when existing article is reopened in block editor. (#16640) Connection Register: Add current user email to connection register request (#16712) Update versions to start 8.9 Release cycle (#16706) Donations block: Make currency and amounts editable (#16593) Update dependency @automattic/calypso-color-schemes to v2 Error Notice: removing HTML code, adjusting maximum width. (#16690) Update dependency swiper to v6 (#16587) Site Scan: Short-circuit update_threats_link() if Admin Bar is not present (#16677) Update vulnerable NPM packages (#16659) E2E Tests: Add Jetpack updater test (#16437) check for subdir site before rendering Ads.txt section (#16671) VideoPress Block: Retain alignment support (#16651) ...

* Adds support for saving the content of a donations block into the post content, so the published view contains the block output. * It also follows up #16593 (comment) and implements the content sync logic as suggested there.

…16593) Adds the ability to edit the currency and amounts in a donations block.

* Adds support for saving the content of a donations block into the post content, so the published view contains the block output. * It also follows up #16593 (comment) and implements the content sync logic as suggested there.

Donations block: Make currency and amounts editable

56c2599

mmtr added [Status] Needs Review This PR is ready for review. [Block] Donation labels Jul 27, 2020

mmtr requested a review from a team July 27, 2020 14:19

mmtr self-assigned this Jul 27, 2020

matticbot added Touches WP.com Files labels Jul 27, 2020

jeherve previously requested changes Jul 27, 2020

View reviewed changes

extensions/blocks/donations/attributes.js Outdated Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jul 27, 2020

mmtr added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jul 27, 2020

gwwar reviewed Jul 27, 2020

View reviewed changes

jeherve added [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Jul 28, 2020

Edit currency in block toolbar

1f5cf89

Remove currency code from choose amount text

0108276

Make amounts directly editable in editor

78a4b2b

mmtr added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Jul 29, 2020

fgiannar added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Aug 3, 2020

mmtr added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Aug 3, 2020

mmtr added the [Status] Needs Design Review Design has been added. Needs a review! label Aug 3, 2020

mmtr removed the [Status] Needs Design Review Design has been added. Needs a review! label Aug 3, 2020

mmtr mentioned this pull request Aug 3, 2020

Donations block: Save content #16689

Closed

kwight approved these changes Aug 3, 2020

View reviewed changes

mmtr added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 4, 2020

fgiannar approved these changes Aug 4, 2020

View reviewed changes

mmtr merged commit aefee69 into master Aug 4, 2020

mmtr deleted the update/donation-block-edit-amount branch August 4, 2020 11:24

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 4, 2020

github-actions bot added this to the 8.9 milestone Aug 4, 2020

jeherve removed the [Status] Needs Changelog label Aug 4, 2020

mmtr mentioned this pull request Aug 4, 2020

Donations: Save block content #16705

Merged

kwight mentioned this pull request Aug 6, 2020

Donations: Implement block serialization. Automattic/wp-calypso#44109

Closed

pereirinha pushed a commit that referenced this pull request Sep 10, 2020

[not verified] Donations block: Make currency and amounts editable (#…

5a63637

…16593) Adds the ability to edit the currency and amounts in a donations block.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Donations block: Make currency and amounts editable #16593

Donations block: Make currency and amounts editable #16593

mmtr commented Jul 27, 2020 •

edited by kwight

Loading

mmtr commented Jul 27, 2020

jetpackbot commented Jul 27, 2020 •

edited

Loading

gwwar Jul 27, 2020

gwwar Jul 27, 2020 •

edited

Loading

gwwar commented Jul 27, 2020

davemart-in commented Jul 28, 2020

danhauk commented Jul 28, 2020

mmtr commented Jul 29, 2020

matticbot commented Jul 29, 2020

mmtr commented Jul 29, 2020

danhauk commented Jul 29, 2020 •

edited

Loading

fgiannar commented Aug 3, 2020

mmtr commented Aug 3, 2020

fgiannar commented Aug 3, 2020

mmtr commented Aug 3, 2020

kwight commented Aug 3, 2020

mmtr commented Aug 4, 2020

fgiannar left a comment

Donations block: Make currency and amounts editable #16593

Donations block: Make currency and amounts editable #16593

Conversation

mmtr commented Jul 27, 2020 • edited by kwight Loading

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Proposed changelog entry for your changes:

mmtr commented Jul 27, 2020

jetpackbot commented Jul 27, 2020 • edited Loading

gwwar Jul 27, 2020

Choose a reason for hiding this comment

gwwar Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

gwwar commented Jul 27, 2020

davemart-in commented Jul 28, 2020

danhauk commented Jul 28, 2020

mmtr commented Jul 29, 2020

matticbot commented Jul 29, 2020

mmtr commented Jul 29, 2020

danhauk commented Jul 29, 2020 • edited Loading

fgiannar commented Aug 3, 2020

mmtr commented Aug 3, 2020

fgiannar commented Aug 3, 2020

mmtr commented Aug 3, 2020

kwight commented Aug 3, 2020

mmtr commented Aug 4, 2020

fgiannar left a comment

Choose a reason for hiding this comment

mmtr commented Jul 27, 2020 •

edited by kwight

Loading

jetpackbot commented Jul 27, 2020 •

edited

Loading

gwwar Jul 27, 2020 •

edited

Loading

danhauk commented Jul 29, 2020 •

edited

Loading