Skip to content
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

Show a loading UI when accepting a dispute is in progress #7476

Merged
merged 14 commits into from
Oct 25, 2023

Conversation

Jinksi
Copy link
Member

@Jinksi Jinksi commented Oct 13, 2023

Fixes #7473

Changes proposed in this Pull Request

This PR improves the "Accept dispute" merchant flow by showing a loading state within the Accept dispute modal when the accept dispute fetch request is in progress.

Before:

  • the merchant clicks "accept dispute"
  • the modal closes
  • there is no indication of a network request in progress
  • on success: a success snackbar message appears and the UI is updated with new dispute details
  • on error: an error snackbar message appears

After:

  • the merchant clicks "accept dispute"
  • the modal remains open with a loading animation
  • buttons are disabled and the modal cannot be closed
  • on success: the modal closes, a success snackbar message appears and the UI is updated with new dispute details
  • on error: the modal remains open, buttons are re-enabled and an error snackbar message appears

TODO

  • Change button text to Accepting... when in progress.
  • Reduce modal max-width to match design

Testing instructions

Dispute: Needs response

  • As a shopper, create a disputed order by purchasing a test product using the card 4000000000000259 at checkout.
  • As a merchant, navigate to Payments -> Disputes and click the row for the dispute you just created. You will be taken to the "Transaction Details" screen.
  • Click the Accept dispute button to view the modal.
    • Ensure the modal can be closed by pressing cancel, close or clicking outside.
  • Error handling
    • Disable network (Network tab → throttling) in your browser dev tools
    • Click Accept dispute and ensure the error is handled with a relevant message. The UI buttons should be re-enabled after the error event.
  • Loading state:
    • Re-enable network (Network tab → throttling) in your browser dev tools
    • Click Accept dispute and enjoy the new loading animation for a few moments. Buttons should be disabled while the network request is in progress. The modal should not be dismissible.
    • Ensure the "Transaction Details" screen is refreshed with the following details:
      • Disputed: Lost status.
      • A message beginning with This dispute was accepted and lost on {date}.

Inquiry: Needs response

  • As a shopper, create a disputed order by purchasing a test product using the card 4000000000001976 at checkout.
  • As a merchant, navigate to Payments -> Disputes and click the row for the dispute you just created. You will be taken to the "Transaction Details" screen.
  • Click Issue refund to view the modal.
    • Ensure the modal can be closed by pressing cancel, close or clicking outside.
  • Ensure clicking View order to issue refund takes you to the "Edit order" screen.

Screen recordings

Screen recording of before without loading state

Accept.dispute.before.mov

Screen recording of happy path (with a slow network to demonstrate loading state)

Accept.dispute.loading.state.mov

Screen recording of network error

Accept.dispute.network.error.mov

  • Run npm run changelog to add a changelog file, choose patch 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.
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Post merge

@botwoo
Copy link
Collaborator

botwoo commented Oct 13, 2023

Test the build

Option 1. Jetpack Beta

  • Install and activate Jetpack Beta.
  • Use this build by searching for PR number 7476 or branch name fix/7473-accept-dispute-loading-state-ui in your-test.site/wp-admin/admin.php?page=jetpack-beta&plugin=woocommerce-payments

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:

  • Latest commit: 3e8f9ca
  • Build time: 2023-10-25 03:39:02 UTC

Note: the build is updated when a new commit is pushed to this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 13, 2023

Size Change: +36 B (0%)

Total Size: 1.43 MB

Filename Size Change
release/woocommerce-payments/dist/index-rtl.css 36.4 kB +1 B (0%)
release/woocommerce-payments/dist/index.css 36.5 kB +1 B (0%)
release/woocommerce-payments/dist/index.js 284 kB +34 B (0%)
ℹ️ View Unchanged
Filename Size
release/woocommerce-payments/assets/css/admin.css 1.04 kB
release/woocommerce-payments/assets/css/success.css 158 B
release/woocommerce-payments/dist/blocks-checkout-rtl.css 1.8 kB
release/woocommerce-payments/dist/blocks-checkout.css 1.8 kB
release/woocommerce-payments/dist/blocks-checkout.js 75.2 kB
release/woocommerce-payments/dist/checkout-rtl.css 440 B
release/woocommerce-payments/dist/checkout.css 441 B
release/woocommerce-payments/dist/checkout.js 29 kB
release/woocommerce-payments/dist/multi-currency-analytics.js 1.05 kB
release/woocommerce-payments/dist/multi-currency-rtl.css 2.88 kB
release/woocommerce-payments/dist/multi-currency-switcher-block.js 60.3 kB
release/woocommerce-payments/dist/multi-currency.css 2.88 kB
release/woocommerce-payments/dist/multi-currency.js 55 kB
release/woocommerce-payments/dist/order-rtl.css 676 B
release/woocommerce-payments/dist/order.css 679 B
release/woocommerce-payments/dist/order.js 41.1 kB
release/woocommerce-payments/dist/payment-gateways-rtl.css 690 B
release/woocommerce-payments/dist/payment-gateways.css 692 B
release/woocommerce-payments/dist/payment-gateways.js 38.6 kB
release/woocommerce-payments/dist/payment-request-rtl.css 146 B
release/woocommerce-payments/dist/payment-request.css 146 B
release/woocommerce-payments/dist/payment-request.js 13.2 kB
release/woocommerce-payments/dist/product-details.js 898 B
release/woocommerce-payments/dist/settings-rtl.css 9.09 kB
release/woocommerce-payments/dist/settings.css 9.09 kB
release/woocommerce-payments/dist/settings.js 234 kB
release/woocommerce-payments/dist/subscription-edit-page.js 669 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal-rtl.css 519 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.css 519 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.js 20.4 kB
release/woocommerce-payments/dist/subscription-product-onboarding-toast.js 693 B
release/woocommerce-payments/dist/subscriptions-empty-state-rtl.css 117 B
release/woocommerce-payments/dist/subscriptions-empty-state.css 117 B
release/woocommerce-payments/dist/subscriptions-empty-state.js 19.6 kB
release/woocommerce-payments/dist/tos-rtl.css 230 B
release/woocommerce-payments/dist/tos.css 231 B
release/woocommerce-payments/dist/tos.js 22 kB
release/woocommerce-payments/dist/upe_checkout-rtl.css 440 B
release/woocommerce-payments/dist/upe_checkout.css 441 B
release/woocommerce-payments/dist/upe_checkout.js 34.1 kB
release/woocommerce-payments/dist/upe_split_checkout-rtl.css 440 B
release/woocommerce-payments/dist/upe_split_checkout.css 441 B
release/woocommerce-payments/dist/upe_split_checkout.js 34.7 kB
release/woocommerce-payments/dist/upe_with_deferred_intent_creation_checkout.js 37 kB
release/woocommerce-payments/dist/upe-blocks-checkout-rtl.css 1.8 kB
release/woocommerce-payments/dist/upe-blocks-checkout.css 1.8 kB
release/woocommerce-payments/dist/upe-blocks-checkout.js 41 kB
release/woocommerce-payments/dist/upe-split-blocks-checkout-rtl.css 1.8 kB
release/woocommerce-payments/dist/upe-split-blocks-checkout.css 1.8 kB
release/woocommerce-payments/dist/upe-split-blocks-checkout.js 42.5 kB
release/woocommerce-payments/dist/woopay-express-button-rtl.css 146 B
release/woocommerce-payments/dist/woopay-express-button.css 146 B
release/woocommerce-payments/dist/woopay-express-button.js 52.1 kB
release/woocommerce-payments/dist/woopay-rtl.css 3.91 kB
release/woocommerce-payments/dist/woopay.css 3.91 kB
release/woocommerce-payments/dist/woopay.js 71.8 kB
release/woocommerce-payments/includes/subscriptions/assets/css/plugin-page.css 622 B
release/woocommerce-payments/includes/subscriptions/assets/js/plugin-page.js 814 B
release/woocommerce-payments/vendor/automattic/jetpack-assets/build/i18n-loader.js 2.43 kB
release/woocommerce-payments/vendor/automattic/jetpack-assets/src/js/i18n-loader.js 1.01 kB
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-ajax.js 522 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-callables.js 581 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/babel.config.js 160 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.css 2.32 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.js 13.8 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.rtl.css 2.32 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/about.css 1.2 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-empty-state.css 291 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-order-statuses.css 403 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin.css 3.56 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/checkout.css 299 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/modal.css 742 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/view-subscription.css 572 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/wcs-upgrade.css 411 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin-pointers.js 544 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin.js 9.63 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.js 6.8 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.min.js 3.83 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-coupon.js 544 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-subscription.js 2.38 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.js 22.1 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.min.js 11.6 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/payment-method-restrictions.js 1.29 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/wcs-meta-boxes-order.js 502 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/payment-methods.js 355 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/single-product.js 429 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/view-subscription.js 1.38 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/wcs-cart.js 781 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/modal.js 1.1 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/wcs-upgrade.js 1.27 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.css 392 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.js 3.05 kB

compressed-size-action

@Jinksi Jinksi marked this pull request as ready for review October 13, 2023 05:56
@Jinksi Jinksi changed the title Disable accept dispute modal close when request in progress Show a loading UI when accepting a dispute is in progress Oct 13, 2023
@Jinksi Jinksi requested a review from a team October 13, 2023 06:03
@Jinksi
Copy link
Member Author

Jinksi commented Oct 13, 2023

FYI, I've requested a review but am yet to change the button text to Accepting... when in progress.

Feel free to review aside from this change.

@Jinksi Jinksi requested a review from nikkivias October 13, 2023 06:06
@Jinksi
Copy link
Member Author

Jinksi commented Oct 13, 2023

@nikkivias I've requested a review from you if you want to take a look. It is probably easiest to review the screen recordings I've added.

FYI, I am yet to change the button text to Accepting... when in progress.

@nikkivias
Copy link

@Jinksi The transition has improved significantly as it no longer jumps into the next screen.

I'm noticing other parts that don't quite match the Figma design though. Particularly the accept button which is outlined with only little side paddings (instead of Text only version), and the size of the modal is quite wide.

image image image image

@Jinksi
Copy link
Member Author

Jinksi commented Oct 23, 2023

Thanks for your feedback @nikkivias.

the accept button which is outlined with only little side paddings (instead of Text only version)

This button uses the text-only tertiary button and I think this matches the component used in the Figma design. It will only show the border when hovered or focused.

Screen.Recording.2023-10-23.at.15.41.02.mov

and the size of the modal is quite wide.

✅ I've fixed the max-width of the modal to match the design (600px).

image

✅ I've also updated the button label to Accepting… when the request is in progress.

@nikkivias
Copy link

Thank you for actioning on those last 2 @Jinksi everything looks good!

@Jinksi
Copy link
Member Author

Jinksi commented Oct 24, 2023

Thanks @nikkivias! Requesting a code review from @Automattic/helix before merging.

Copy link
Contributor

@haszari haszari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and the new UI and submitting progress state is a vast improvement 🚀

I added some comments about renaming and/or refactoring the code to simplify (TLDR – make the code a closer match for what the merchant experiences). None of this is blocking, though renaming isLoading is probably safe and impactful.

}: {
dispute: Dispute;
isLoading: boolean;
} ): AcceptDisputeProps {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAcceptDisputeProps is pretty strange to read and reason about. I think this code would be a lot clearer if it just had a dedicated modal for each case (inquiry vs dispute). AcceptDisputeProps is a domain-specific interface for a semi-generic modal (e.g. modalLines); I don't see a benefit to that over using the React components we have (i.e. a powerful, readable, flexible modal API).

That said, I don't know if it's safe/worthwhile to refactor this for this change. I would be tempted 😁

Mentioning because this PR reads as very decoupled – can't really see what changed. Specifically, the button Submitting… label should be an implementation detail inside a modal component. I find it strange (and complex) that this is delegated to a function (that now needs a new param).

Copy link
Member Author

@Jinksi Jinksi Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree and I began a refactor of this code in #7339, but it became low-priority and out-of-scope due to time constraints.

I think a refactor should be handled in a separate PR, as it is a more significant quantity of work and is separate from this PR's goal.

@@ -346,7 +358,7 @@ const DisputeAwaitingResponseDetails: React.FC< Props > = ( {
'transaction_details',
}
);
setModalOpen( false );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why we didn't use onModalClose before.

Copy link
Member Author

@Jinksi Jinksi Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same! I don't have an answer.

@@ -171,6 +177,9 @@ const DisputeAwaitingResponseDetails: React.FC< Props > = ( {
} = useContext( WCPaySettingsContext );

const onModalClose = () => {
if ( isLoading ) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More renaming suggestions 😁

onModalClose reads strange to me – what is its purpose? Currently the name is based on where it is bound (a kind of coupling?). With the addition of the early return, it's becoming abstract, e.g. onModalClose() { if isLoading don't close }.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed it to handleModalClose in 27e7621 – does this make more sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A tiny bit!

@Jinksi Jinksi added this pull request to the merge queue Oct 25, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 25, 2023
@Jinksi Jinksi added this pull request to the merge queue Oct 25, 2023
Merged via the queue into develop with commit ef6e551 Oct 25, 2023
27 checks passed
@Jinksi Jinksi deleted the fix/7473-accept-dispute-loading-state-ui branch October 25, 2023 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accept Dispute button does not give any feedback when it's clicked – confusing
4 participants