-
Notifications
You must be signed in to change notification settings - Fork 677
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
Improvements/refactoring matching desings #18
Improvements/refactoring matching desings #18
Conversation
|
||
func setupConstraints() { | ||
|
||
for attribute: NSLayoutAttribute in [.CenterX, .CenterY] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, I understand, why all the hassle with not adding dependencies, but after trying Masonry/Cartography default AutoLayout syntax looks really weird.
Nice idea to keep it away from logic-related code.
On the other hand, may be overcomplicated if file grows. Splitting into 2 separate files i.e. Class.swift + ClassLayout.swift may be an option if there are too much constraints.
@@ -47,21 +47,21 @@ public class ImageGalleryView: UIView { | |||
let view = UIView() | |||
view.setTranslatesAutoresizingMaskIntoConstraints(false) | |||
view.addGestureRecognizer(self.panGestureRecognizer) | |||
view.backgroundColor = self.configuration.backgroundColor | |||
view.backgroundColor = UIColor.blackColor().colorWithAlphaComponent(0.6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool 😎
@hyperoslo/myshop-ios This looks awesome! What do you guys think? |
static let indicatorWidth: CGFloat = 41 | ||
static let indicatorHeight: CGFloat = 8 | ||
} | ||
|
||
lazy public var collectionView: UICollectionView = { [unowned self] in | ||
lazy public var collectionView: UICollectionView = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[unowned self] in
? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fuck! 😄
@RamonGilabert Looks great! |
…ng-desings Improvements/refactoring matching desings
🍶 |
☕ |
@@ -3,9 +3,9 @@ import UIKit | |||
public class ImageStack { | |||
|
|||
public struct Notifications { | |||
public static let imageDidPush = "imageDidPush:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm. This needs to be checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a notification name, not a method. The name of the notification is imageDidPush, not imageDidPush:.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I did few tests to check that it works correctly. Seems that I've just copied selector.
No description provided.