search

schibsted / account-sdk-browser

Schibsted Account SDK for browsers
https://schibsted.github.io/account-sdk-browser/
MIT License
16 stars 11 forks source link

Add locale param to login #90

Closed marjuszkiewicz closed 6 years ago

marjuszkiewicz commented 6 years ago

The most important change is https://github.com/schibsted/account-sdk-browser/compare/add-locale-param-to-login?expand=1#diff-b8139375bef5ea9da4706270fef9c716R183 . I believe that it shouldn't have big influence since it will skip only empty string query params.

codecov[bot] commented 6 years ago

Codecov Report

Merging #90 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #90   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files          14       14           
  Lines         456      456           
  Branches      101      101           
=======================================
  Hits          455      455           
  Misses          1        1
Impacted Files Coverage Δ
src/identity.js 99.43% <ø> (ø) :arrow_up:
src/RESTClient.js 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dd5d220...3cf6ac4. Read the comment docs.

marjuszkiewicz commented 6 years ago

I think we should, but new payment flow is not exposed yet in sdk. We can add all of params when we will introduce new payment flow urls

torarvid commented 6 years ago

Sure, just saying many links generated by payment.js go to the BFF, and so they will direct the user to a BFF login flow before jumping onto the payment part. Feels like "whenever new payment flows go live" can be controlled on the backend with (possibly) no JS SDK changes?

Anyway, feel free to merge this PR when/if you want to 👍