Polymer / shop

The Shop app
https://shop.polymer-project.org/
992 stars 494 forks source link

Adds Credential API and Payment API #91

Closed agektmr closed 7 years ago

agektmr commented 8 years ago

Adding Credential API and Payment API a la my demo introduced at Polymer Summit session. This is a continuation work of #70 .

ppshein commented 8 years ago

shop-account.html:148 Uncaught TypeError: Cannot read property 'gSignIn' of undefined

Encountered above error when click close browser button of google signIn authentication pop-up and then persistently occurred above error whenever click on google signIn button.

agektmr commented 8 years ago

@ppshein Odd. There shouldn't be need for manually closing a pop up window opened for Google Sign-In. Can you describe the flow step by step? Make sure to remove all cache from DevTools.
"Application" > "Clear storage" and press "Clear site storage".

ppshein commented 8 years ago

@agektmr I just clicked google signIn button, then popup is displayed. At that time, click close button of browser instead of no click on Deny and Allow of that popup. After that, popup is closed and spinner was displaying permanently as never disappeared. That's why refresh browser and click on Google SignIn button then show error message as I mentioned above.

https://i.imgsafe.org/d9a58b9c4c.png

agektmr commented 8 years ago

@ppshein Did you put client_secrets.json as described in README.md?

ppshein commented 8 years ago

@agektmr I just used & tested website link you mentioned above only https://polykart-credential-payment.appspot.com

agektmr commented 8 years ago

@ppshein I just tried with my other gmail account and had no problem. Asked my neighbor to try as well but succeeded. Can you open the page with an incognito mode and try again?

ppshein commented 8 years ago

@agektmr Same as what I tried. Can you try to click on close button of browser without clicking anything on popup box?

agektmr commented 8 years ago

OK, I see what you mean. Issue reproduced. Will look into this.

agektmr commented 8 years ago

@ppshein Can you go back to root page "/" then reload, go back to "/account" and try sign-in?

ppshein commented 8 years ago

@agektmr everything is working correctly when I click "Allow" button of that popup as signing in. Problem is when I click on close button of browser and NOW "deny" button of that popup without going root page and reloading it.

agektmr commented 8 years ago

@ppshein Just going directly to this url will reproduce the issue.
This seems to be caused by async bootstrap of shop-account-data and obtaining it via iron-meta. Thinking about ways to solve this issue...

agektmr commented 8 years ago

@ppshein The issue should have been fixed. Can you try again?

ppshein commented 8 years ago

@agektmr same as before in Chrome. Btw, I've deleted all cookie & storage of google chrome.

shop-account.html:148 Uncaught TypeError: Cannot read property 'gSignIn' of undefined(…)_onGSignIn @ shop-account.html:148s @ shop-app.html:48

when open in safari, I've encountered following error in root page.

TypeError: undefined is not an object (evaluating 'this.accountData.autoSignIn(!0).then')

agektmr commented 8 years ago

How about this now?

ppshein commented 8 years ago

@agektmr spinner should be disappeared when I click on close button of popup as should be calling same method of clicking on "Deny" button of it. :)

agektmr commented 8 years ago

@ppshein Hmm, Google Sign-In library doesn't omit reject response. I'll do an investigation but there seems no way to catch closing window as far as I see it now.

FluorescentHallucinogen commented 8 years ago

@blasten, PTAL.

FluorescentHallucinogen commented 7 years ago

Any progress?

agektmr commented 7 years ago

I was busy with Chrome Dev Summit. Will resume the work.

agektmr commented 7 years ago

OK, I've removed server side code and created a service worker based mock server (Phew). Can you have another look through? @blasten

blasten commented 7 years ago

@agektmr Thanks for the update! I left a few comments, but let me know if you would like me to help. If you add permissions to your copy, I could push a few changes.

agektmr commented 7 years ago

@blasten I've addressed all feedback. One remaining bit I have is if I should add a Promise polyfill or not. Without promises, there will be a major rewrite. What is the general policy on promise in Polymer team?

blasten commented 7 years ago

@agektmr Thanks for taking the time and sorry for the long time in getting this PR merged! We wanted to make sure that it won't have any perf regression. In that spirit, I have created the branch credential-payment-api. Would it be possible to send a PR against that branch? We will conduct a few tests and possibly make small changes. We will keep you in the loop and merge it to master as soon as possible! Probably after the holidays. Thank you!

agektmr commented 7 years ago

@blasten That is great to hear. I'll send a separate PR. In the meanwhile, I was giving a deep dive into google-signin component and would like to propose a major change in the design. That will allow google-signin to be a part of this PR. I will detail the change in an issue I will raise in google-signin repository.

agektmr commented 7 years ago

FYI, https://github.com/GoogleWebComponents/google-signin/issues/156

blasten commented 7 years ago

PR merged! cc @frankiefu

FluorescentHallucinogen commented 7 years ago

@frankiefu @blasten @agektmr Any plans to merge it to the master branch?

googlebot commented 7 years ago

So there's good news and bad news.

:thumbsup: The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

:confused: The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

agektmr commented 7 years ago

My understanding is this PR is no longer relevant. Please close if that's ok.

FluorescentHallucinogen commented 7 years ago

@agektmr

My understanding is this PR is no longer relevant. Please close if that's ok.

On 20 Dec 2016 your pull request #104 was merged by @blasten to credential-payment-api branch and closed, but for some reasons it was not merged to the master branch. After that the project was rewritten in Polymer 2 with ES6 classes. This pull request was never merged at all and now has conflicts with master branch that must be resolved. Could you please resolve the conflicts?

keanulee commented 7 years ago

From my understanding this branch/PR was made as a proof-of-concept of how Web Payments can be integrated into Shop, which is not necessarily part of Shop's primary goal of showing how to build a fast front-end PWA with Polymer. Feel free to examine this PR to get a sense of what is required, but otherwise I'm going to close this PR.