Skip to content

Feature/facebook use case#3

Open
sabrineElbahri wants to merge 29 commits into
3IE:developfrom
sabrineElbahri:feature/FacebookUseCase
Open

Feature/facebook use case#3
sabrineElbahri wants to merge 29 commits into
3IE:developfrom
sabrineElbahri:feature/FacebookUseCase

Conversation

@sabrineElbahri

Copy link
Copy Markdown
Contributor

exemple of facebook login

@BenoitVerdier BenoitVerdier left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

• 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

//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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use "view lifecycle" instead of "VC methods"

if userInfo != nil {
SVProgressHUD.show(withStatus: "Chargement")

self.facebookId = self.logInWithFacebook.getFacebookIdFromUserInfoInTheCompletionHandlerFromLoginFunction(userInfo!)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please change the naming of the method "getUserFirstNameFromUserInfoInTheCompletionHandlerFromLoginFunction" in the socialNetwork framework

self.email = self.logInWithFacebook.getUserEmailFromUserInfoInTheCompletionHandlerFromLoginFunction(userInfo!)

self.performSegue(withIdentifier: "PushFacebookInfo", sender: nil)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please use "if let var1 = var2 as? type" construct to have a more robust code

Comment thread App/FacebookInfoVC.swift

}

//MARK: - Setup view

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

//MARK must not be used everywhere, but only the define zones, like "properties", view life cycle, ...

Comment thread App/FacebookInfoVC.swift
super.viewDidLayoutSubviews()

cropping(imageView: ProfileImageView)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

coding style : useless newlines

Comment thread App/FacebookInfoVC.swift


//MARK: - Treatment on image
func cropping(imageView: UIImageView) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

unused parameter "imageView"; either switch to a static method and keep the arg OR remove the argument

Comment thread App/FacebookInfoVC.swift
@IBOutlet weak var firstNameTextField: UITextField!
@IBOutlet weak var lastNameTextField: UITextField!
@IBOutlet weak var emailTextField: UITextField!
@IBOutlet weak var ProfileImageView: UIImageView!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bad naming, use lower camel case

Comment thread App/FacebookInfoVC.swift
emailTextField.text = email
emailTextField.isEnabled = false

self.loginWithFacebook?.getFacebookProfileImageUrlAsync(self.facebookId!) { image in

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bad naming in the socialNetwork framework: "get" keyword is useless and the method does not get and "ImageUrl" but an "Image"

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.

2 participants