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

Display page level error and disable checkout button when no available payment methods #2873

Merged

Conversation

ankurvr
Copy link
Member

@ankurvr ankurvr commented Nov 25, 2020

Fixed issue #2869 Assigned by @sirugh

Description

Display page level error and disable checkout button when no available payment methods

Related Issue

Closes #2869.

Acceptance

Verification Stakeholders

  1. @sirugh

Specification

Verification Steps

  1. Add item to cart
  2. go to checkout and complete shipping steps
  3. Make sure you have payment methods enabled other than Braintree

Screenshots / Screen Captures (if appropriate)

pwa-docker localhost_8080_checkout (2)

pwa-docker localhost_8080_checkout (1)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

…ll available payment methods while fetching cart data on checkout page
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Nov 25, 2020

Messages
📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against a09f4d8

@ankurvr
Copy link
Member Author

ankurvr commented Dec 11, 2020

@sirugh Can you please review this PR and let me know if i am missing anything from mentioned requirements in the assigned issue #2869 .

Copy link
Contributor

@revanth0212 revanth0212 left a comment

Choose a reason for hiding this comment

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

Hey @ankurvr, thanks for the contribution. Nice work.

I have outlined a couple of minor changes but the overall code looks good.

Please do update the merge conflicts.

@sirugh
Copy link
Contributor

sirugh commented Feb 23, 2021

I'm looking into the first issue Dev posted.

Comment on lines 179 to 182
// If we don't have an implementation for a method type, ignore it.
const isPaymentAvailable = !!availablePaymentMethods.find(({ code }) =>
paymentMethods.includes(code)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

If zero total checkout is enabled, and a user or coupon causes the total to be $0, then code will be free. As we don't have a payment implementation view for this, we just need to allow the code free to mean that payment is available. Making the below changes allows checkout to proceed when a user has entered a coupon/gc that causes the total to be zero.

Suggested change
// If we don't have an implementation for a method type, ignore it.
const isPaymentAvailable = !!availablePaymentMethods.find(({ code }) =>
paymentMethods.includes(code)
);
// If we have an implementation, or if this is a "zero" checkout,
// we can allow checkout to proceed.
const isPaymentAvailable = !!availablePaymentMethods.find(
({ code }) => code === 'free' || paymentMethods.includes(code)
);

Copy link
Contributor

Choose a reason for hiding this comment

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

However, in addition to this change, we may want to change the copy here. free will only be the code when a GC/coupon is entered causing the total to be zero.

That means if a user gets to checkout page before adding a gift card or coupon they will first see the error messaging. So we might want to update the message to explain this somehow. cc @schensley

supernova-at
supernova-at previously approved these changes Feb 24, 2021
@ankurvr
Copy link
Member Author

ankurvr commented Feb 25, 2021

@ankurvr @sirugh

  1. If multiple errors then instead of stacking one after other currently they are displayed incorrectly.

image

@dpatil-magento Please suggest right veriant from below.
Payment error messages have 1rem padding and stock message have 0.625rem.
Please suggest final one.

  1. Padding is different in both kind of messages

Screenshot 2021-02-25 at 1 52 59 PM

  1. Made padding same for both the messages

Screenshot 2021-02-25 at 2 15 27 PM

@sirugh
Copy link
Contributor

sirugh commented Feb 25, 2021

@ankurvr Good catch on the difference. This is because there are now two errors being rendered that look similar but are different components. Please update the StockStatusMessage component css to use FormError's padding, 1rem.

@ankurvr
Copy link
Member Author

ankurvr commented Feb 25, 2021

@sirugh Noted. I will do needful.

Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Thank you for fixing the styling!

@dpatil-magento
Copy link
Contributor

Thanks @ankurvr @sirugh , QA Approved.

@dpatil-magento dpatil-magento merged commit ec4e97d into magento:develop Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:peregrine pkg:venia-ui Progress: done version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature]: Display page level error and disable checkout button when no available payment methods
6 participants