Feature/facebook use case#3
Conversation
…ature/FacebookUseCase # Conflicts: # App.xcodeproj/project.pbxproj # App/AppDelegate.swift # Cartfile # Cartfile.resolved
BenoitVerdier
left a comment
There was a problem hiding this comment.
• FacebookInfoVC is not fully displayable on iphone 5
• the app must explain how to configure the socialnetwork (instead of providing one of our own facebookId in the plist)
| logOutWithFacebook.logOut() | ||
| } | ||
|
|
||
| //MARK: - Login |
There was a problem hiding this comment.
//MARK must not be used everywhere, but only the define zones, like "properties", view life cycle, ...
| var logInWithFacebook: LogInWithFacebook = LogInWithFacebook.init() | ||
| var logOutWithFacebook: LogOutWithFacebook = LogOutWithFacebook.init() | ||
|
|
||
| //MARK: - VC methods |
There was a problem hiding this comment.
use "view lifecycle" instead of "VC methods"
| if userInfo != nil { | ||
| SVProgressHUD.show(withStatus: "Chargement") | ||
|
|
||
| self.facebookId = self.logInWithFacebook.getFacebookIdFromUserInfoInTheCompletionHandlerFromLoginFunction(userInfo!) |
There was a problem hiding this comment.
please change the naming of the method "getUserFirstNameFromUserInfoInTheCompletionHandlerFromLoginFunction" in the socialNetwork framework
| self.email = self.logInWithFacebook.getUserEmailFromUserInfoInTheCompletionHandlerFromLoginFunction(userInfo!) | ||
|
|
||
| self.performSegue(withIdentifier: "PushFacebookInfo", sender: nil) | ||
|
|
There was a problem hiding this comment.
coding style: unnecessary new line
| //MARK: - Navigation | ||
| override func prepare(for segue: UIStoryboardSegue, sender: Any?) { | ||
| if segue.identifier == "PushFacebookInfo" { | ||
| let controller: FacebookInfoVC = segue.destination as! FacebookInfoVC |
There was a problem hiding this comment.
please use "if let var1 = var2 as? type" construct to have a more robust code
|
|
||
| } | ||
|
|
||
| //MARK: - Setup view |
There was a problem hiding this comment.
//MARK must not be used everywhere, but only the define zones, like "properties", view life cycle, ...
| super.viewDidLayoutSubviews() | ||
|
|
||
| cropping(imageView: ProfileImageView) | ||
|
|
|
|
||
|
|
||
| //MARK: - Treatment on image | ||
| func cropping(imageView: UIImageView) { |
There was a problem hiding this comment.
unused parameter "imageView"; either switch to a static method and keep the arg OR remove the argument
| @IBOutlet weak var firstNameTextField: UITextField! | ||
| @IBOutlet weak var lastNameTextField: UITextField! | ||
| @IBOutlet weak var emailTextField: UITextField! | ||
| @IBOutlet weak var ProfileImageView: UIImageView! |
There was a problem hiding this comment.
bad naming, use lower camel case
| emailTextField.text = email | ||
| emailTextField.isEnabled = false | ||
|
|
||
| self.loginWithFacebook?.getFacebookProfileImageUrlAsync(self.facebookId!) { image in |
There was a problem hiding this comment.
bad naming in the socialNetwork framework: "get" keyword is useless and the method does not get and "ImageUrl" but an "Image"
exemple of facebook login