Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
171 stars 69 forks source link

Allow reset/customization of "policies and custom text" on WooPay checkout #7196

Open frosso opened 11 months ago

frosso commented 11 months ago

Description

From https://github.com/Automattic/woocommerce-payments/issues/6993#issuecomment-1711310926

Acceptance criteria

Designs

267342832-1203ab18-30b6-408d-945b-d6865c3fb0bf

Dev notes

Additional context

pierorocca commented 11 months ago

@stephendharmadi how does the merchant know what the default text says or what we might change it to in the future?

pierorocca commented 11 months ago

@frosso @stephendharmadi my copy recommendations are as follows:

Checkbox label states: "Link my Privacy Policy and Terms of Service pages at checkout"

Recommendation: "Display links to my privacy policy and terms of service" or more simply "Display links to store policies"

"We'll link your Privacy Policy and Terms of Service pages on checkout. Uncheck this box to set your custom text instead. Learn more {external link icon}"

Recommendation: "We'll link your Privacy Policy and Terms of Service pages on checkout. Uncheck this box to set custom text instead. Learn more {external link icon}"

Will the removal of "your" allow the text to fit on one line on desktop? If not would removing "instead" fit it on one line?

When unchecked: The textarea for the custom text is displayed The textarea's help text states: "Override the default Privacy Policy and Terms of Service, or add custom text to WooPay checkout."

Recommendation: Remove this helper text. The helper text above the textarea provides similar guidance, the default copy itself provides guidance, as does the "Learn more" documentation.

If the merchant changes the textarea content, decides to check the checkbox, then unchecks the checkbox again, the textarea content should display the text that the merchant entered previously

I think this is opposite of what's shown in the GIF?

Thoughts on adding support for the usage of a refund_returns shortcode? We wont include it in the default, but the merchant can add it to the custom text if needed.

frosso commented 11 months ago

I think your recommendations look good @pierorocca 👍

I think this is opposite of what's shown in the GIF?

Yeah, in the GIF above I was trying to showcase a merchant coming from a "base" state/new installation.

I was thinking that it might become frustrating to a merchant to see the changes being reverted if they had made a change to the custom text and clicked the checkbox by mistake. I guess they could refresh the page (without saving). But if they clicked "save" in between, they'd have the "default" state (which I could see as both a pro and a con).

Thoughts on adding support for the usage of a refund_returns shortcode? We wont include it in the default, but the merchant can add it to the custom text if needed.

What setting would we use for this shortcode? FWIW, although it's not user-friendly, merchants can currently set their own links, which will be displayed on the platform checkout.

pierorocca commented 11 months ago

What setting would we use for this shortcode?

image

I was thinking that it might become frustrating to a merchant to see the changes being reverted if they had made a change to the custom text and clicked the checkbox by mistake. I guess they could refresh the page (without saving). But if they clicked "save" in between, they'd have the "default" state (which I could see as both a pro and a con).

Yup totally agree with this. That triggered my comment about how does the merchant actually see what's meant by default. I think when the new UI Preview enhancement is made, that will show the merchant exactly what will be rendered. Do you see any issue with that and keeping your model of retaining edited text?

frosso commented 11 months ago

image

Ah, I see - I couldn't find it in the WC settings. The page is created as a draft on WC installation, and the option is kept as a "reminder" that the page has been created. I am not confident the option can be used as a stable reference since somebody could trash the page and create a new one without the option being refreshed. So I asked here p1694774366765429-slack-C3ECCGUVC .

That triggered my comment about how does the merchant actually see what's meant by default. I think when the new UI Preview enhancement is made, that will show the merchant exactly what will be rendered. Do you see any issue with that and keeping your model of retaining edited text?

I don't think so - but maybe the solution can be open to whoever implements it, if one option is easier than the other.

pierorocca commented 11 months ago

Do you know if it's theme specific? My Tsubaki theme seems to have the page clearly listed and the page id matches. Terms of service also isn't in the WC settings, only in the theme editors as far as I can tell.

image

frosso commented 11 months ago

@pierorocca the page seems to be in "draft" - I checked your site and the page's content, it's the one created during the WC installation.

pierorocca commented 11 months ago

Correct I've not published it. It's one of those policies that may or may not exist or that may simply be a part of the standard terms of service. Given there's a chance it could be published as separate to the terms of service, does it make sense to support a refund_returns shortcode? Between the upcoming UI preview and making this shortcode optional for inclusion into the custom text field, I'm seeing more risk not including this than including it. I'm not suggesting we include it in the default copy, only as a supported shortcode if the merchants opts for custom.

Yes a merchant could add 'https://proccaatomic.wpcomstaging.com/refund_returns/' to the custom text along with other copy. Our standards need to be higher than supporting only a plain text url. The shortcode approach is the most rigid while a markdown or rich text field + editor would be the most flexible option. Since we've gone with the first option for expediency, does it make sense to cover this very plausible use case? Do we have a viable path to an editor that's more capable than a textarea?

frosso commented 11 months ago

@pierorocca adding the shortcode makes sense. Sorry if I wasn't clear - the problem I was trying to explain in this comment ( https://github.com/Automattic/woocommerce-payments/issues/7196#issuecomment-1721075837 ) is that the woocommerce_refund_returns_page_id option is not configurable by the merchant. It is only used as a value that tells WooCommerce "I added a 'refund and returns' page as part of the installation, so I don't need to add it again". Since the page ID constant is not part of the WC settings, the page could be trashed and re-created. In this scenario, the option would not be updated, and if we were to leverage the refund_returns shortcode, we wouldn't be able to link it. The merchant would have to use the HTML in any case. If, instead, we wanted to say "let's get the refund_returns page by its slug", we would still incur in a possible scenario where the merchants renamed the page to a different URL. All of this, regardless of whether it's included in the default copy or not.

Both scenarios are susceptible to bugs, because the merchant doesn't have the ability to set the page in its WC (or WP) settings, unlike the privacy policy or the TOS pages.

pierorocca commented 11 months ago

Got it. That's definitely a miss that refund and returns is not a page type that's assignable. How plausible do you think it is a merchant will trash the default refunds and returns page and then create a different page altogether?

Do you have another alternative to suggest? e.g. open an enhancement request with WooCommerce core to add ability to assign the page? Convert textarea into a markdown or rich text capable field? Or for now require merchants to link to or embed that policy in their ToS?

frosso commented 11 months ago

How plausible do you think it is a merchant will trash the default refunds and returns page and then create a different page altogether?

I'm not sure how plausible it would be. The way I am thinking of it is that if we went with an implementation that doesn't consider this scenario, we could get an irresolvible bug on our hands (or HE time spent communicating with the merchant).

Do you have another alternative to suggest? e.g. open an enhancement request with WooCommerce core to add ability to assign the page? Convert textarea into a markdown or rich text capable field? Or for now require merchants to link to or embed that policy in their ToS?

If the page is indeed something that is frequently utilized by merchants, an improvement in the WC settings would definitely be beneficial. I think converting the textarea into a more user-friendly markdown/WYSIWYG editor would also be feasible, with the caveat that we would need to strip unwanted tags to prevent XSS. I'm not sure if providing a WYSIWYG editor would be overkill, since we would need to be very restrictive in its capabilities. We would probably allow links and a few other tags, to prevent scenarios where merchants become too overzealous with their inputs. The checkout preview above the editor could help visualize the extent of the changes. But at the end of the day, it's an approximation of the checkout UI (it's not an iframe, and it's not making a request to WooPay to inquire how the input would be displayed). The end result might vary depending on what it's implemented on the platform site.

In the current state of the settings, merchants can use the textarea to embed links - it's just not very user-friendly.

pierorocca commented 11 months ago

I'm not sure how plausible it would be. The way I am thinking of it is that if we went with an implementation that doesn't consider this scenario, we could get an irresolvible bug on our hands (or HE time spent communicating with the merchant).

I feel that not having anything may be equally bad or worse of an issue for those merchants that need that policy displayed. Worst case they disable WooPay. i.e. it's problematic either way.

Another and probably better long term option is for core to enhance policy settings and persist the data so there's only 1 place to set these policies regardless of what checkout is used and presented to shoppers. I'll begin work on highlighting the existing state and potential improvements in a post for core platform consideration.

zmaglica commented 11 months ago

This issue impacts WooPay, so assigning to team Heisenberg. (based on team responsibilities Pc2DNy-3z-p2) @frosso . Assigning as part of Gamma Triage process PcreKM-yM-p2.

c-shultz commented 11 months ago

@frosso noting here that I see one live store currently has this text displayed in hosted checkout

image
frosso commented 11 months ago

@c-shultz I tried to reproduce the issue locally with no success. However, I do see it for the site in question. I created a separate ticket to investigate this problem: https://github.com/Automattic/woocommerce-payments/issues/7302