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

Add collection view for menu view #25

Merged
merged 3 commits into from Mar 26, 2019
Merged

Add collection view for menu view #25

merged 3 commits into from Mar 26, 2019

Conversation

jaewonsim
Copy link
Contributor

Resolves #8

  • Add IGListKit-based menu view
  • Navigation controller to be implemented


// MARK: Constraint setup
private func setUpConstraints() {
let leadingOffset: CGFloat = 24
Copy link
Member

Choose a reason for hiding this comment

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

might want to put constants at the top along with view vars

@doctor3w doctor3w requested review from kevinchan159 and removed request for doctor3w March 26, 2019 20:15
@doctor3w
Copy link
Contributor

Merged with master and updated design

@appdev-danger-bot
Copy link

appdev-danger-bot commented Mar 26, 2019

1 Warning
⚠️ SwiftLint not installed, download from https://github.com/realm/SwiftLint

SwiftLint found issues

Warnings

File Line Reason
MenuViewController.swift 63 TODOs should be resolved (get menu from endpoint). (todo)

Generated by 🚫 Danger

}

private func loadMenu() {
// TODO: get menu from endpoint
Copy link

Choose a reason for hiding this comment

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

⚠️ TODOs should be resolved (get menu from endpoint).
todo MenuViewController.swift:63

kevinchan159
kevinchan159 previously approved these changes Mar 26, 2019
Copy link
Contributor

@kevinchan159 kevinchan159 left a comment

Choose a reason for hiding this comment

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

lgtm

var bottomSeparator: UIView!

// var additionalPricingLabel: UILabel!
// var selectButton: 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 if unnecessary

// selectButton.setTitle("Select", for: .normal)
// selectButton.backgroundColor = UIColor(red: 1.0, green: 156.0 / 255.0, blue: 29.0 / 255.0, alpha: 1.0)
// selectButton.tintColor = .white
// contentView.addSubview(selectButton)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove if unnecessary

let imageViewWidthHeight: CGFloat = 64
let imageViewStackViewHorizontalSpacing: CGFloat = 24
let bottomSeparatorHeight: CGFloat = 1.0
// let stackViewSelectButtonHorizontalSpacing: CGFloat = 17
Copy link
Contributor

Choose a reason for hiding this comment

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

remove if unnecessary

make.leading.equalTo(imageView.snp.trailing).offset(imageViewStackViewHorizontalSpacing)
make.centerY.equalToSuperview()
make.trailing.equalToSuperview().inset(trailingInset)
// make.trailing.equalTo(selectButton.snp.leading).offset(-stackViewSelectButtonHorizontalSpacing)
Copy link
Contributor

Choose a reason for hiding this comment

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

same

// selectButton.snp.makeConstraints { make in
// make.trailing.equalToSuperview().inset(trailingInset)
// make.width.equalTo(selectButtonWidth)
// make.centerY.equalToSuperview()
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@doctor3w doctor3w merged commit 45492ba into master Mar 26, 2019
@doctor3w doctor3w deleted the jaewon/menu-view branch March 26, 2019 20:56
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.

None yet

5 participants