Skip to content

Conversation

@cjc8
Copy link
Contributor

@cjc8 cjc8 commented Jul 27, 2021

No description provided.

@cjc8 cjc8 requested a review from schrockblock July 27, 2021 17:25
}

PlaygroundPage.current.liveView = vc
PlaygroundPage.current.needsIndefiniteExecution = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I can't get the products to show up in the tableview, but I think that's because it's in a playground rather than a project. I'm having a hard time finding any help online (pretty niche situation) but i'll keep at it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Playgrounds do have that issue – have you logged/messed with the view to make sure it's getting displayed?

}

open func productsRequest(_ request: SKProductsRequest, didReceive response: SKProductsResponse) {
print("received response")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For my own memory Remove print statement

return { transaction in
switch transaction.transactionState {
case .failed:
SKPaymentQueue.default().finishTransaction(transaction)
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'm on the fence between finishing the transaction whenever the state is purchased or failed, or providing the function as a safe default in the constructor. I think I'm going to go with the latter, but your input would be nice

import UIKit

public class LUXSubscriptionTableViewCell: UITableViewCell {
@IBOutlet weak var subscriptionNameLabel: UILabel!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, with generalizability in mind, I'm torn between making sure it displays the price or not enforcing it, since displaying the price is something that can be done in just a label or as a button (like we do in Tapper) and I'm having trouble finding a way to make sure they have something that shows the price but not restricting that to a certain UI element

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's assume they'll either use what we have or replace it wholesale – so basically, let's make all the assumptions we want about what's getting displayed, but put in a hook that someone could replace with custom functionality.

}

public func productToPriceString(_ product: SKProduct) -> String {
let currency = product.priceLocale.currencySymbol ?? ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WIP but will clean up in future commits

Copy link
Contributor

@schrockblock schrockblock left a comment

Choose a reason for hiding this comment

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

Cool stuff, very excited for it

}

PlaygroundPage.current.liveView = vc
PlaygroundPage.current.needsIndefiniteExecution = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Playgrounds do have that issue – have you logged/messed with the view to make sure it's getting displayed?


public var cancelBag: Set<AnyCancellable> = []
@Published var products: [SKProduct] = []
public var productsCall: CombineNetCall?
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love to find a way to not have to require a CombineNetCall here; is there a way we could instead either split this into a super and a subclass, one that just provides the functions and nothing else, and the other that has a smart default (using CombineNetCall perhaps)? Could we rely on a part of CombineNetCall, or Fireable, or just the returned data in some way? I'd love this to be flexible enough to use in the ReactiveSwift version of LUX, or generally no matter what the net stack looks like.

import UIKit

public class LUXSubscriptionTableViewCell: UITableViewCell {
@IBOutlet weak var subscriptionNameLabel: UILabel!
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's assume they'll either use what we have or replace it wholesale – so basically, let's make all the assumptions we want about what's getting displayed, but put in a hook that someone could replace with custom functionality.

import LithoOperators
import PlaygroundVCHelpers

public func subscriptionViewController<C: LUXSubscriptionTableViewCell>(styleVC: @escaping (LUXSubscriptionViewController) -> Void, configureCell: @escaping (SKProduct, C) -> Void = configureSubscriptionCell, onTap: @escaping (SKProduct) -> Void = productToPayment >?> addPayment, termsPressed: (() -> Void)? = nil, delegate: LUXSubscriptionDelegate = LUXSubscriptionDelegate()) -> LUXSubscriptionViewController{
Copy link
Contributor

Choose a reason for hiding this comment

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

You might make C just a regular UITableViewCell and just use ~> when you want to assume it's our cell

Copy link
Contributor Author

@cjc8 cjc8 Aug 2, 2021

Choose a reason for hiding this comment

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

Because it has two parameters, optionalCast won't work unless I embed it in the second method from LithoOperators, what I have as a default value is ~second(optionalCast) >>> ~configureSubscriptionCell, which compiles fine

@schrockblock
Copy link
Contributor

Is this still in progress?

@cjc8 cjc8 changed the title WIP: Feature/cjc8/storekit Feature/cjc8/storekit Aug 24, 2021
@schrockblock
Copy link
Contributor

@cjc8 ^^^?

@cjc8 cjc8 force-pushed the feature/cjc8/storekit branch from 81cbca4 to d18d930 Compare September 15, 2021 16:02
@cjc8 cjc8 requested a review from schrockblock September 23, 2021 16:29
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.

3 participants