-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add dispute steps to resolve to the transaction details page. #7206
Conversation
… the email subject used in email link for the disputed transaction customer.
… section of the dispute details on the transaction details page.
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: +1.1 kB (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
…ns about accepting the dispute.
'Y-m-d', | ||
moment( dispute.created * 1000 ).toISOString() | ||
); | ||
const emailSubject = `Problem with your purchase from ${ wcpaySettings.storeName } on ${ chargeDate }?`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally do not translate the email subject and body because it is a bit weird to translate a content that is for the customer who might not speak the same language as merchant.
What do you think @souravdebnath1986 @nikkivias? Should we send this email as merchant's language or in English? Or maybe both as I've seen that practice (sending in 2 languages in one email) a lot, but if so, what about the subject?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep it simple? This is site content, so it should be translatable. Just like content on pages, buttons, or in site emails.
The merchant has control of all of this. The default experience should not have mixed languages IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note – this will not send an email, just generate a draft email in merchant's email client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, if a multilingual merchant wants to send a localised "is everything ok with your purchase" email, they can edit in their mail client of choice or use another mailing solution such as MailPoet etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made the email subject and body translatable in 787b8d9 to let the merchant decide what language to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think to default to the merchant's language is better than to suddenly see English if they have localized configuration.
) } from ${ chargeDate }. We wanted to contact you to make sure everything was all right with your purchase and see if there's anything else we can do to resolve any problems you might have had.\n\n` + | ||
`Alternatively, if the dispute was a mistake, you could easily withdraw it by calling the number on the back of your card. Thank you so much - we appreciate your business and look forward to working with you.`; | ||
|
||
emailLink = `mailto:${ customer.email }?subject=${ encodeURIComponent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wanted to use querystring.strinfigy() but I don't see it's used at all in the codebase, so I just use encodeURIComponent()
to escape each param's value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has anyone tested clicking the link correctly works with at least one email client? If there's any issue with escaping or protocol then that would catch it! I have this behaviour disabled on my mac (because I use GMail), so wasn't able to test.
Noting so we can test in our test swarm today e.g. on mobile or if someone has mailto
configured right :)
/> | ||
), | ||
disputeduedate: ( | ||
<span className="dispute-steps__steps__response-date"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this from DisputeSummaryRow's so it looks consistent.
One doubt I have is that in the design paJDYF-9Ip-p2, when due date is still more than 7-day away, it does not show the ({n} days left to respond)
part.
but I see in DisputeSummaryRow
, it shows that text, so for consistency sake, I follow DisputeSummaryRow
.
Just to note that I'm open for suggestion to improve this PR, especially because of prop-drilling, ie since |
Even though there are couple blockers, I think this PR can be reviewed already. |
# Conflicts: # client/payment-details/dispute-details/index.tsx # client/payment-details/dispute-details/style.scss
FYI @shendy-a8c I've resolved a few of the TODOs and discussion points for this PR.
Update: I've made the email subject and body translatable in 787b8d9 to let the merchant decide what language to use. |
This PR is ready for another review. FYI @shendy-a8c 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @Jinksi !
Tested:
- With dispute, I could see the steps and actions.
- With inquiry, I did not see the steps and actions.
- The tooltip CSS move made me nervous because that means it will affect all
TooltipBase
usage, ieClickTooltip
andHoverTooltip
but I checked they all look good as if that CSS does not do anything =p. CheckingHoverTooltip
is a bit much but mainly the settings page and the payment logo (eg Visa's) on the transactions list page. I see it's used in WooPay as well but I don't know how to test that.
Thank you for the great work!
I can't approve this PR as I authored it, so go ahead approve and merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick review and tested one dispute – looking good and working well. Based on previous reviews this is good to merge so I will merge! 🚢
client/payment-details/dispute-details/dispute-awaiting-response-details.tsx
Show resolved
Hide resolved
) } from ${ chargeDate }. We wanted to contact you to make sure everything was all right with your purchase and see if there's anything else we can do to resolve any problems you might have had.\n\n` + | ||
`Alternatively, if the dispute was a mistake, you could easily withdraw it by calling the number on the back of your card. Thank you so much - we appreciate your business and look forward to working with you.`; | ||
|
||
emailLink = `mailto:${ customer.email }?subject=${ encodeURIComponent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has anyone tested clicking the link correctly works with at least one email client? If there's any issue with escaping or protocol then that would catch it! I have this behaviour disabled on my mac (because I use GMail), so wasn't able to test.
Noting so we can test in our test swarm today e.g. on mobile or if someone has mailto
configured right :)
<li> | ||
{ createInterpolateElement( | ||
__( | ||
'Challenge <challengeicon/> or accept <accepticon/> the dispute by <disputeduedate/>.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found these inline icon tooltips a bit weird, they break the flow of the sentence. It's functioning well though and super clear and usable so not a blocker.
Maybe that info could be in a single help popup, nearer the action buttons. Thinking out loud – @nikkivias is our UX expert here.
@haszari I have tested the mailto links with macOS mail, works well! I'll share a screenshot when I get the chance. |
I have tested it and it worked but in English. If we really want to be sure escaping works, it should be tested with non-ASCII charset languages like Chinese, Japanese, Arabic, etc. I will try it during testing. Good call out! Update: hmm but since the strings won't be translated yet, I think it has to be tested manually. |
Disabling auto-merge, will merge this manually once GH checks complete. |
An example email in macOS that appears when clicking cc @haszari |
Fixes #6925.
Changes proposed in this Pull Request
Add the 'Steps to resolve' section on the transaction details page for disputes (not inquiries – handled in issue #7245).
Inquiries will not see the steps rendered:
Notes:
but it's not yet ready.This has been published.Testing instructions
update_option( '_wcpay_feature_dispute_on_transaction_page', '1' );
in WP Console.4000000000000259
to get disputed right away.Problem with your purchase from {store name} on {YYYY-MM-DD of the charge date}?
daysRemaining
at your will here, or hardcode the dispute's due by date to test these cases:Last day today
.Inquiries
4000000000001976
to create an inquiry.npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge