Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profile View #18

Merged
merged 16 commits into from Nov 30, 2016
Merged

Profile View #18

merged 16 commits into from Nov 30, 2016

Conversation

doctor3w
Copy link
Contributor

Created the profile view (for tab bar and as a profile detail). Still needs backend integration and once we figure out how our local data is stored will need an update. Design is apparently still working designs so it will need some color updates. Finally the Favorites tableviewcell has not been finalized and will need to be swapped out from the current DiscoverTableViewCell.

natashaarmbrust and others added 5 commits November 6, 2016 15:50
changes

removed the pods folder

trying image view

fixed pods

made header view file

worked more

finished activity cell, added useractivity model

finished cell, started header

updated with colors

changed colors

started new designs for profile

Started animation work on the header view.

more animations done and the follow button completed

Finished follow button, finished possible view animation

Basically done with profile, needs cleanup

almost finished. Cleaning up and changing cell to EpisodeTableViewCell

Finished profile (for the most part). Will be changing the favorites cell

ready to rebase

// Main NavigationController initialization
let firstVC = FBSDKAccessToken.current() != nil ? tabBarController : loginVC
// let firstVC = tabBarController
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete comment

ctx.strokePath()
ctx.restoreGState()

titleLabel?.textColor = (isSelected == true) ? UIColor.podcastWhite : UIColor.podcastBlack
Copy link
Contributor

Choose a reason for hiding this comment

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

only need isSelected instead of isSelected == true; also no UIColor


override func draw(_ rect: CGRect) {
let ctx:CGContext = UIGraphicsGetCurrentContext()!;
let strokeColor: CGColor = self.tintColor.cgColor
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for self here and in this file in general since without it, it would default to self

ctx.setFillColor(fillColor)
ctx.setStrokeColor(strokeColor)
ctx.saveGState()
let lineWidth: CGFloat = 1.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

no semicolon

}

override func draw(_ rect: CGRect) {
let ctx:CGContext = UIGraphicsGetCurrentContext()!;
Copy link
Contributor

Choose a reason for hiding this comment

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

space after colon and no semicolon


titleLabel?.textColor = (isSelected == true) ? UIColor.podcastWhite : UIColor.podcastBlack

if (self.isSelected == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if !isSelected


if (self.isSelected == false) {
ctx.saveGState()
let fillPath: UIBezierPath = UIBezierPath(roundedRect: self.bounds.insetBy(dx: lineWidth, dy: lineWidth), byRoundingCorners: UIRectCorner.allCorners, cornerRadii: CGSize(width: self.bounds.size.height/2, height: self.bounds.size.height/2))
Copy link
Contributor

Choose a reason for hiding this comment

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

no UIRectCorner; generally for enums, it's better to not include the prefix to keep it concise :)

if let activity = activity {
let dateFormatter = DateFormatter()
dateFormatter.dateFormat = "eee, MMM dd"
// dateFormatter.timeStyle = .none
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove all unnecessary comments in this file

var actText = NSMutableAttributedString()
switch (activity.action) {
case .Liked:
actText = NSMutableAttributedString(string: "Liked "+podcastName)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would store the string "Liked" in a separate variable and use it to get the location 6 for the NSRange in line 46, since it's subject to change

Copy link
Contributor

Choose a reason for hiding this comment

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

Also string interpolation is your friend :) "Liked \(podcastName)"

break
case .Listened:
actText = NSMutableAttributedString(string: "Listened to "+podcastName)
actText.addAttributes([NSForegroundColorAttributeName: UIColor.podcastPink, NSFontAttributeName: UIFont.boldSystemFont(ofSize: 14)], range: NSRange(location: 12, length:podcastName.characters.count))
Copy link
Contributor

Choose a reason for hiding this comment

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

same as line 45-46

super.init(style: style, reuseIdentifier: reuseIdentifier)

frame.size.height = height+2*contentPaddingY
backgroundColor = UIColor.podcastWhite
Copy link
Contributor

Choose a reason for hiding this comment

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

remove UIColor and CGRect and other enum prefixes in this file

containerView.frame = CGRect(x: contentPaddingX, y: contentPaddingY, width: cellSize.width-2*contentPaddingX, height: height)

let containSize = containerView.frame.size
let cellPadding:CGFloat = 12.0
Copy link
Contributor

Choose a reason for hiding this comment

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

generally it's good to have a space after : when declaring types of variables for better readability

// Currently: Followers, Following (Used to include subscriptions)
var bottomButton1: UIButton!
var bottomButton2: UIButton!
// var bottomButton3: UIButton!
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove comments


// Currently: Followers, Following (Used to include subscriptions)
var bottomButton1: UIButton!
var bottomButton2: UIButton!
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 rename bottomButton1 and bottomButton2 to names that are more representative or their action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I only did this because the design team hadn't decided on exactly what the buttons would be. They kept changing it so rather than renaming over and over again I just did this. But I can put it back now.


profileImage.image = user.image
nameLabel.text = user.name
usernameLabel.text = "@"+user.username
Copy link
Contributor

Choose a reason for hiding this comment

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

"@ (user.username)"

override init(frame: CGRect) {
super.init(frame: frame)

self.backgroundColor = UIColor.podcastGrayDark
Copy link
Contributor

Choose a reason for hiding this comment

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

no self and enum prefixes in this file

let proImgY: CGFloat = 28.5
let padding: CGFloat = 12
let labelHeight: CGFloat = 19
let labelSpacing: CGFloat = 2.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep all constants at the top of the file

profileImage.layer.cornerRadius = proImgWidth/2.0
nameLabel.frame = CGRect(x: padding, y: proImgY+proImgWidth+padding, width: labelWidth, height: labelHeight)
usernameLabel.frame = CGRect(x: padding, y: usernameY, width: labelWidth, height: labelHeight)
followButton.frame = CGRect(x: (self.frame.size.width-150)/2, y: usernameY+labelHeight+padding, width: 150, height: 29)
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like 150 and 29 should be declared as contants


@objc func animateByYOffset(_ yOffset: CGFloat) {
let usernameY = proImgY+proImgWidth+padding+labelHeight+labelSpacing
print(yOffset)
Copy link
Contributor

Choose a reason for hiding this comment

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

no print statement

print(yOffset)
if yOffset <= 0 {
profileImage.alpha = 1
} else if yOffset >= (proImgY+proImgWidth/2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

(proImgY+proImgWidth/2) is also used in line 195, so let's store it in a variable with a specific name so we know what this offset refers to

paragraphStyle.alignment = .center

let numText = String(format: "%d", num)
let title = NSMutableAttributedString(string: text+"\n"+numText)
Copy link
Contributor

Choose a reason for hiding this comment

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

string interpolation

}

func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
print("Series pressed...")
Copy link
Contributor

Choose a reason for hiding this comment

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

no print

guard let user = user else { return }
profileImage.image = user.image
nameLabel.text = user.name
usernameLabel.text = "@"+user.username
Copy link
Contributor

Choose a reason for hiding this comment

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

string interpolation

}
}

override init(frame: CGRect) {
Copy link
Contributor

Choose a reason for hiding this comment

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

no enum prefixes in file or self


let profileSectionHeaderViewHeight: CGFloat = 37

class ProfileSectionHeaderView: UIView {
Copy link
Contributor

Choose a reason for hiding this comment

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

no enum prefixes and self

profileTableView.reloadData()
}

override func didReceiveMemoryWarning() {
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this, so let's get rid of it for now

miniHeader.setTopOpacity(0)
} else if yOffset >= PHVConstants.bottomBarDist-PHVConstants.miniHeight {
miniHeader.setTopOpacity(1)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

condense lines 151 - 154:
let belowThreshold = (yOffset <= (PHVConstants.bottomBarDist-PHVConstants.miniHeight))
miniHeader.setTopOpacity(belowThreshold ? 0 : 1)

return cell
}

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

not using storyboards, so don't need this

super.init(frame: frame)
backgroundColor = UIColor.white

seriesImageView = UIImageView(frame: CGRect(x: 0, y: 0, width: frame.size.width, height: frame.size.height))
Copy link
Contributor

Choose a reason for hiding this comment

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

don't need size for swift 3 anymore :) frame.width and frame.height should do

@anniexcheng
Copy link
Contributor

Made some comments! When you're done making those changes, commit them and I'll take another look :)

Drew Dunne added 4 commits November 30, 2016 08:59
changes

removed the pods folder

trying image view

fixed pods

made header view file

worked more

finished activity cell, added useractivity model

finished cell, started header

updated with colors

changed colors

started new designs for profile

Started animation work on the header view.

more animations done and the follow button completed

Finished follow button, finished possible view animation

Basically done with profile, needs cleanup

almost finished. Cleaning up and changing cell to EpisodeTableViewCell

Finished profile (for the most part). Will be changing the favorites cell

ready to rebase
@anniexcheng
Copy link
Contributor

good to merge!

@doctor3w doctor3w merged commit b6305ac into master Nov 30, 2016
@anniexcheng
Copy link
Contributor

Merged in b6305ac

@anniexcheng anniexcheng deleted the drew/profile branch December 1, 2016 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants