Add Dispute Challenge and Accept buttons to Transaction Details #7093
Conversation
Note the final return statement will return the non-awaiting-response UI
| @@ -56,6 +69,7 @@ export interface Dispute { | |||
| currency: string; | |||
| created: number; | |||
| balance_transactions: BalanceTransaction[]; | |||
| payment_intent: string; | |||
There was a problem hiding this comment.
This was missing from the Dispute TS interface, but is a property of the Stripe API dispute object: https://stripe.com/docs/api/disputes/object#dispute_object-payment_intent.
This PR uses dispute.payment_intent to refresh the payment_intent data for the Transaction Details when a dispute is accepted.
| // This function handles the dispute acceptance flow from the Transaction Details screen. | ||
| // It will become the default acceptDispute function once the feature flag | ||
| // '_wcpay_feature_dispute_on_transaction_page' is enabled by default. | ||
| export function* acceptTransactionDetailsDispute( dispute ) { |
There was a problem hiding this comment.
Rather than modify the existing acceptDispute action (used on the Dispute → Details screen), I've created a new one since there are some differences in the approach:
- The existing action will force a redirect to Payments → Disputes.
- This new action requires fetching and updating the payment intent to refresh the current Transaction Details page.
Once the feature flag is lifted, _wcpay_feature_dispute_on_transaction_page, we can remove and replace the previous action with the new one.
I've left a TODO item on #6883 to make this change.
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. |
Jinksi
changed the title
|
Size Change: +2 kB (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
When a Dispute is nested within a charge, it will be charge_id rather than charge object
There was a problem hiding this comment.
Challenge dispute → Dispute challenge screen ✅
Accept dispute → A modal will be shown. ✅
On error, a snackbar will show an error message in the bottom-left corner. ✅
The dispute should be accepted ✅
The Transaction Details screen will be hydrated with new data (Disputed: Lost status chip). ✅
The dispute actions should no longer be available. ❌
The dispute actions should not be visible when won, lost or under review. ❌
View a disputed transaction for a dispute where a challenge has been staged (click "Save for Later" on the dispute challenge form.) ✅
When testing I was able to see the buttons after the dispute was lost or won.
This surprises me, since the parent won't render the component at all if the woocommerce-payments/client/payment-details/summary/index.tsx Lines 383 to 387 in d8ca8e6 I'll have a look. Thanks for testing @brucealdridge ! |
| <Link | ||
| href={ | ||
| // Prevent the user navigating to the challenge screen if the accept request is in progress. | ||
| isLoading | ||
| ? '' | ||
| : getAdminUrl( { | ||
| page: 'wc-admin', | ||
| path: | ||
| '/payments/disputes/challenge', | ||
| id: dispute.id, | ||
| } ) | ||
| } | ||
| > | ||
| <Button |
There was a problem hiding this comment.
Is there a reason for a button inside a link here? Would it be better to just have a button with a href attribute on it?
There is the wcTracks event on the <Button> and the href on the <Link>
Can we combine these so there are two actions on the one element, rather than two actions over two elements?
There was a problem hiding this comment.
Ah, yeh this pattern came out of the discussion here: #7047 (comment).
We want the client-side routing AND native browser navigation via href when CMD+clicking. Link offers this, whereas Button doesn't – but we want the Button UI.
I'll consider if there's another way, perhaps there is a component that I don't know about.
There was a problem hiding this comment.
I've attempted a compromise in 3709cc0 by using a Link with a span child for the button styles.
<Link
href={ getAdminUrl( ... ) }
onClick={ () => { wcpayTracks.recordEvent( ... ); } }
>
<span className="components-button is-primary">
I am styled like a ButtonOne risk with this approach is that we're using classNames for styling components-button detached from the Button component.
I'm curious to get your thoughts on this problem, @haszari, as you noticed this issue in a recent PR: #7047 (comment).
If I'm thinking about the project goals, getting this button right is critical. Using client-side routing is desirable, IMO, since the merchant will see the challenge form more quickly and smoothly → more challenges being initiated.
There was a problem hiding this comment.
If I'm thinking about the project goals, getting this button right is critical. Using client-side routing is desirable, IMO, since the merchant will see the challenge form more quickly and smoothly → more challenges being initiated.
💯 totally agree. If we can speed up state transitions, that should improve "conversions" (responses to disputes), and of course make merchants' lives easier and more efficient.
One risk with this approach is that we're using classNames for styling components-button detached from the Button component.
I don't think using a raw <span> is much leaner than using a <button>. I'd lean towards using components as is (not borrowing classes or other hackery), and if there's a gap in the library, follow up by logging issues (and submitting PRs?) so the component library meets our needs. So personally I prefer the original `
This reveals a potential UX/design issue – we might be using Button inappropriately when we should be using Link.
A bigger topic really, might be a good discussion on P2 e.g. codep2 or with other front end experts :)
There was a problem hiding this comment.
Thanks for your feedback @haszari and @brucealdridge.
Based on this discussion, I'm going to proceed with the <Link><Button /></Link> pattern to get this PR shipped (203f161 & 163514b).
I'll log a follow-up issue and move this conversation elsewhere.
| sprintf( | ||
| /* translators: %s: dispute fee, <em>: emphasis HTML element. */ | ||
| __( | ||
| 'Accepting the dispute marks it as <em>Lost</em>. The disputed amount will be returned to the cardholder, with a %s dispute fee deducted from your account.', |
There was a problem hiding this comment.
I think we need to review this language. This implies the fee will only be deducted if they accept the dispute. When the fee has already been deducted.
There was a problem hiding this comment.
I'll log a follow-up issue to allow this PR to be shipped and we can iterate on the wording.
There was a problem hiding this comment.
New issue for this: #7254
but keep functionality and styles
Hmmmm, I'm sure I did a pull just before testing. Doing a pull again says I was running be9ff75. False alarm sorry, all working as expected. |
Fixes #6926
Changes proposed in this Pull Request
DisputeDetailscomponentNote: this PR won't handle CTA buttons for inquiries – this will be handled in issue #7193.
Screenshots
Action buttons:
Accept modal:
Snackbar on successful accept:
Wide buttons on mobile (see design: RiEQmaKRI7u9USAI3lyZz0VX-fi-7430_30973):
The below video demonstrates:
→ clicking the Challenge button
→ staging evidence via the challenge form
→ returning to the dispute details
→ opening the accept dispute modal
→ accepting the dispute
dispute-actions.mov
Testing instructions
4000000000000259at checkout.Disputed: Loststatus chip).Continue with challengenpm run changelogto add a changelog file, choosepatchto 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