Closed brampitoyo closed 9 years ago
@andymckay For some reason, the API documentation version that I have is more up to date than what currently exists on master
, so it prevents merge.
More up to date than what exists on master? Do you need to pull down master and rebase?
Bram, on the buy wireframe, can you split the email address flow into a completely separate diagram? As a first pass we only have time to implement the flow beginning with a Firefox Accounts sign-in. Since someone in FxA was asking me to see wireframes I think it would be less confusing to link to a diagram with that one flow. It will still be nice to have the non-account flow so that we can consider implementing it later.
the high level management UX flow shows PayPal as an option alongside credit cards but I don't think we have plans to support PayPal yet. Nevermind, I see that we'll support multiple options in Q3
I had a previous comment about being able to replace all subscriptions at once (instead of one at a time) when you delete a payment method but I don't see that in the mocks. Is that something we could look at adding later?
In regards to the credit card form, I think there will be certain errors that we cannot tie to a specific field. In that case, could you maybe add a label style for how it should look? Perhaps an error window could appear above the Subscribe button? Example: "Internal Error. Please try again later."
@kumar303 @andymckay I’ve managed to fix all of the issues you’ve mentioned. Thanks for the feedback!
I’d like to elaborate a bit on how the “Replace all subscriptions” feature works:
I think this is looking good and we should merge.
I see that the buy flow UX begins with a required sign-in now but it still has some branches after payment completion for the case of not having a Firefox Account. Can you update that since this first iteration will require a sign-in? As an aside, I think the 'save card' feature is something we should focus on (now or later) because there is a privacy aspect that we'll need to get right.
I still see the password prompt at the end of payment completion in the UX pay wireframe too
On the initial repeat-visit buy screen, the word 'Add' seems ambiguous to me. Could we say 'Pay with new card' or something, maybe? Or do you think Add will be obvious enough?
I’d like to elaborate a bit on how the “Replace all subscriptions” feature works:
The behavior you've described here is something I'd like to ensure we get right in the implementation -- could you add this to the document so it's not lost in the PR discussion? I think adding a section for it in the ux.rst
doc would make sense.
it's looking great! Thanks and sorry for the delay in reviewing it. We've been pushing hard this week on finishing the rough draft prototype.
Overall lgtm.
I think we are going to need a more full-screen error for when things go really wrong. E.g. in normal use most users should never see it.
Currently we are processing the login token as the first step before getting the braintree token. If this failed we'd want to show a more dramatic error which allows the user to cancel. This is also tied to mozilla/payments-client#4
Same thing if fetching the braintree token failed for some reason - you won't be able to post the form so a hard fail is needed.
I think we are going to need a more full-screen error for when things go really wrong
For more context, the user will see a spinner while the server tries to validate the access token. If there is an error, we need to display it as a hard stop. There will be nothing they can do. The credit card form will not be visible.
(I forgot to give my commit a good summary. Oops.)
Let's merge and add in smaller issues for anything else raised in here.
r+
45)