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

Add Windows 10 UWP implementation #1

Merged
merged 8 commits into from
Jan 17, 2017
Merged

Add Windows 10 UWP implementation #1

merged 8 commits into from
Jan 17, 2017

Conversation

robinmanuelthiel
Copy link
Contributor

@robinmanuelthiel robinmanuelthiel commented Jan 11, 2017

Min Supported Version:
Windows 10.0 (Build 10240)

Known issues:

  • Testing Mode currently not available for MvvmCross (needs a platform wide change)
  • Does not support subscriptions (as UWP does not support this, yet)

@jamesmontemagno
Copy link
Owner

Hey there! I have been adjusting the API recently so if you can update based on this new finalized API, should be better. I will start a review on your current and give you some feedback.

Copy link
Owner

@jamesmontemagno jamesmontemagno left a comment

Choose a reason for hiding this comment

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

Ping Ping @robinmanuelthiel let me know if you can make these adjustments

private bool testingMode;

/// <param name="testingMode">UWP offers a way to test in-app purchases with the CurrentAppSimulator class instead of CurrentApp.</param>
public InAppBillingImplementation(bool testingMode = false)
Copy link
Owner

Choose a reason for hiding this comment

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

We can not have a constructor that takes arguments as that creates issues with Bait and Switch.

We should add a bool IsTesting {get;set;} to the Interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, adding IsTesting sounds way better. Will do that.

using System.Threading.Tasks;
using System.Xml;
using Windows.ApplicationModel.Store;

namespace Plugin.InAppBilling
Copy link
Owner

Choose a reason for hiding this comment

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

This file is currently linked with other projects. We will need to remove that or the build will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really get, what you mean here. UWP needs them both to work with IAPs. How could we remove them? Build works for me here. Any advice?

Copy link
Owner

Choose a reason for hiding this comment

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

If you look in the project... essentially the same file is linked to older Windows Phone projects. this means the build will fail. You need to create a blank implementation for those projects.

{
throw new NotImplementedException();
// Get list of products from store or simulator
var listingInformation = testingMode ? await CurrentAppSimulator.LoadListingInformationAsync() : await CurrentApp.LoadListingInformationAsync();
Copy link
Owner

Choose a reason for hiding this comment

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

Can we make this a method for testing mode to return the base of whatever the "CurrentApp" is. That way we don't duplicate that code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea, will do that.

@robinmanuelthiel
Copy link
Contributor Author

Hey @jamesmontemagno, thanks for your review. Sounds good, I will update the PR during the day and give you an update, when it's ready to get reviewed again!

@robinmanuelthiel
Copy link
Contributor Author

robinmanuelthiel commented Jan 17, 2017

Alright, I have updated the PR. Beside failing AppVeyor build. We have to take a look at that together, I guess...

I also took out the testing mode and will send you a separate PR with the IsTestingMode implementation in interface-level.

@jamesmontemagno jamesmontemagno changed the base branch from master to uwp January 17, 2017 18:26
@jamesmontemagno jamesmontemagno merged commit a4c5f80 into jamesmontemagno:uwp Jan 17, 2017
@jamesmontemagno
Copy link
Owner

I created a new UWP branch to look into it and will help out with me trying to get it working. Should be good now :)

jamesmontemagno pushed a commit that referenced this pull request May 18, 2017
Consumption state + merge from forked repo
jamesmontemagno pushed a commit that referenced this pull request Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants