Skip to content
This repository has been archived by the owner on Nov 2, 2019. It is now read-only.

Improve UIAlertController APIs in the SwiftCatalog sample app. #87

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cartfile
Original file line number Diff line number Diff line change
@@ -1 +1 @@
github "facebook/facebook-ios-sdk" ~> 4.14
github "facebook/facebook-ios-sdk" ~> 4.16
2 changes: 1 addition & 1 deletion Cartfile.resolved
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
github "BoltsFramework/Bolts-ObjC" "1.8.4"
github "facebook/facebook-ios-sdk" "sdk-version-4.15.0"
github "facebook/facebook-ios-sdk" "sdk-version-4.16.0"
2 changes: 1 addition & 1 deletion Carthage/Checkouts/facebook-ios-sdk
Submodule facebook-ios-sdk updated 138 files
2 changes: 1 addition & 1 deletion FacebookCore.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ Pod::Spec.new do |s|
s.pod_target_xcconfig = { 'ENABLE_TESTABILITY' => 'YES' }

s.ios.dependency 'Bolts', '~> 1.8'
s.ios.dependency 'FBSDKCoreKit', '~> 4.15'
s.ios.dependency 'FBSDKCoreKit', '~> 4.16'
end
4 changes: 2 additions & 2 deletions FacebookLogin.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ Pod::Spec.new do |s|

s.ios.dependency 'FacebookCore', '~> 0.2'
s.ios.dependency 'Bolts', '~> 1.8'
s.ios.dependency 'FBSDKCoreKit', '~> 4.15'
s.ios.dependency 'FBSDKLoginKit', '~> 4.15'
s.ios.dependency 'FBSDKCoreKit', '~> 4.16'
s.ios.dependency 'FBSDKLoginKit', '~> 4.16'
end
4 changes: 2 additions & 2 deletions FacebookShare.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ Pod::Spec.new do |s|

s.ios.dependency 'FacebookCore', '~> 0.2'
s.ios.dependency 'Bolts', '~> 1.8'
s.ios.dependency 'FBSDKCoreKit', '~> 4.15'
s.ios.dependency 'FBSDKShareKit', '~> 4.15'
s.ios.dependency 'FBSDKCoreKit', '~> 4.16'
s.ios.dependency 'FBSDKShareKit', '~> 4.16'
end
94 changes: 88 additions & 6 deletions Samples/Catalog/Sources/AlertController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,94 @@
import Foundation
import UIKit

extension UIAlertAction {

/// Action types most commonly used
public enum actionType{
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Space after actionType and before {


///Ok Option
case ok

/// Default Cancel Option
case cancel

/// Destructive action with custom title
case destructive(String)

/// Custom action with title and style
case custom(String,UIAlertActionStyle)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Space between String and UIAlertActionStyle
Also, we can use labels here... Do you think it would add the nice verbosity to it?


/**
Creates the action instance for UIAlertController
- parameter handler: Call Back function
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Add empty line between description and first parameter.

- returns UIAlertAction Instance
*/
public func action(handler:((String) -> Void)? = nil) -> UIAlertAction {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Space after handler:


//Default value
var actionStyle = UIAlertActionStyle.default
var title = ""

// Action configuration based on the action type
switch self {

case .cancel:
actionStyle = .cancel
title = "Cancel"

case .destructive(let optionTitle):
title = optionTitle
actionStyle = .destructive

case .custom(let optionTitle, let style):
title = optionTitle
actionStyle = style

default:
title = "Ok"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: OK instead of Ok.

}

//Creating UIAlertAction instance
let action = UIAlertAction(title:title,style:actionStyle, handler: {(nativeAction) in
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Reformat using this style title: title, style: actionStyle, ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Since the closure is the last argument, you can use the trailing closure syntax aka

return UIAlertAction(title: title, style: actionStyle) { nativeAction in 
  if let handler = handle {
    handler(title)
  }
}

if (handler != nil){
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be simplified and be more Swifty using the following:

if let handler = handler {
  handler(title)
}

Or even handler.map({ $0() }) (the latter is less preferred version)

handler!(title)
}
})

return action
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't seem to be re-using let action anywhere, please consider just returning it from the method.

}
}
}

extension UIAlertController {
convenience init(title: String, message: String) {
self.init(title: title, message: message, preferredStyle: .alert)
let action = UIAlertAction(title: NSLocalizedString("OK", comment: "OK action"),
style: .default,
handler: nil)
addAction(action)

/**
Creates the alert view controller using the actions specified

- parameter title: Title of the alert.
- parameter message: Alert message body.
- parameter actions: Variable numbre of actions as an Array of actionType values.
- parameter style: UIAlertControllerStyle enum value
- parameter handler: Handler block/closure for the clicked option.

- let alert = UIAlertController.init(title:"Confirm",message:"Do you want to quit sign up and login with other account ?",actions:.custom("Yes,.default),.custom("Cancel",.destructive)){ (buttonTitle) in
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Please line break this line or trim the message argument, to constrain into 140 columns of text.

if buttonTitle == "Yes"{
print("Yes Clicked")
}
}

self.present(alert, animated: true, completion: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this line is relevant, since it suggests presentation from inside a UIViewController. I suggest removing it.


*/
convenience init(title: String, message: String ,actions:[UIAlertAction.actionType] = [.ok],style: UIAlertControllerStyle = .alert,handler:((String) -> Swift.Void)? = nil) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please line break and style as follows:

convenience init(title: String,
                 message: String,
                 actions: [UIAlertAction.actionType] = [.ok],
                 style: UIAlertControllerStyle = .alert,
                 handler: ((String) -> Swift.Void)? = nil) {


//initialize the contoller (self) instance
self.init(title: title, message: message, preferredStyle:style)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: preferredStyle: style


//Fetching actions specidied by the user and adding actions accordingly
for actionType in actions {
addAction(actionType.action(handler: handler))
}
}
}