Skip to content

Add support for iOS#19

Merged
JlUgia merged 62 commits intomainfrom
ios_support
Apr 6, 2021
Merged

Add support for iOS#19
JlUgia merged 62 commits intomainfrom
ios_support

Conversation

@JlUgia
Copy link
Member

@JlUgia JlUgia commented Mar 29, 2021

  • Add support for iOS with Apple Pay using a PlatformView to draw the button (@jamesblasco)
  • Create both managed and raw buttons to support different integration use cases
  • Separated iOS and Android into two different packages, such that users of the plugin can decide how to create their own plugins
  • Moved the default platform channel implementation to the interface package
  • Added common payment items and types

Closes #6.

JlUgia and others added 30 commits March 9, 2021 12:32
…iness to pay if the platform is not supported
…ion to replace Android and iOS implementations separately
…in-private into ios_support

# Conflicts:
#	pay/lib/pay.dart
#	pay_android/ios/Classes/ApplePayButtonView.swift
#	pay_ios/lib/src/widgets/apple_pay_button.dart
#	pay_mobile/ios/Classes/SwiftPayPlugin.swift
@jamesblasco
Copy link
Collaborator

jamesblasco commented Mar 30, 2021

I would suggest to use a linter for this project. It will help a lot I think. There are some patterns that I haven't seen inside the Flutter Framework and I think there could be a better approach. To highlight the most important ones:

  • Even if Dart allows dynamic values, the Flutter framework is strictly typed and I think it is a good practice to do so with external packages. Right now the payment buttons have all dynamic types, you can build something like the code below without any error during compilation, and lots of them during runtime
 GooglePayButton(
    paymentConfigurationAsset:10,
    paymentItems: 10,
    style: 10,
    type: 10,
    margin: 10,
    onPaymentResult: 10,
    loadingIndicator:10
  )

https://github.com/google-pay/flutter-plugin-private/blob/72496a3527bee3a159183885e6ccaac39f0a1029/pay_platform_interface/lib/pay_channel.dart#L17-L26

@override
Future<Map<String, dynamic>> showPaymentSelector(
  Map<String, dynamic> profile,
  List<PaymentItem> items,
) async {
  final result = await channel.invokeMethod('showPaymentSelector', {
    'payment_profile': jsonEncode(profile),
    'payment_items': items.map((item) => item.toMap()).toList(),
  });
  return jsonDecode(result);
}

There is not good or wrong code about this but I think it improves it by following the style guidelines

@JlUgia
Copy link
Member Author

JlUgia commented Mar 30, 2021

Thank you for the review and comments @Arkangel12, @jamesblasco.
I've added changes and suggestions for all comments.
Feel free to review and resolve conversations as you see fit.

Copy link
Collaborator

@Arkangel12 Arkangel12 left a comment

Choose a reason for hiding this comment

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

minor comment. everything else looks good. I will approve it because it can be like it is, the change is after you.

class _PayButtonState extends State<PayButton> {
late final Future<bool> _userCanPayFuture;

Widget containerizeChildOrShrink({Widget? child, bool isError = false}) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is kind of a hard this to do, naming stuff, haha, but I will something related to the PayButton class maybe something like showChild.

Co-authored-by: Argel Bejarano <argel.bc18@gmail.com>
@JlUgia
Copy link
Member Author

JlUgia commented Apr 5, 2021

All comments addressed.
Merging within the next few hours if there are no objections.

import 'dart:convert';

import 'package:flutter/services.dart';
import 'package:flutter/foundation.dart';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious what you need from flutter/foundation.dart

Copy link
Member Author

Choose a reason for hiding this comment

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

To use the defaultTargetPlatform. See pay_button.dart and pay.dart

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh right. Carry on =)

@JlUgia JlUgia merged commit 2303cd3 into main Apr 6, 2021
@JlUgia JlUgia deleted the ios_support branch May 10, 2021 09:13
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.

Add support for display items to calculate the total price

4 participants