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

project: use swiftlint and fix triggered warnings #129

Merged
merged 6 commits into from
Sep 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Lightbox.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
D523B0A61C43AA2A001AD1EC /* Headers */,
D523B0A71C43AA2A001AD1EC /* Resources */,
D54DFCC41C5AAAF600ADEA0E /* Copy frameworks with Carthage */,
5CF8A88D1F50B4EA00C28475 /* ShellScript */,
);
buildRules = (
);
Expand Down Expand Up @@ -200,6 +201,19 @@
/* End PBXResourcesBuildPhase section */

/* Begin PBXShellScriptBuildPhase section */
5CF8A88D1F50B4EA00C28475 /* ShellScript */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
files = (
);
inputPaths = (
);
outputPaths = (
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "if which swiftlint >/dev/null; then\nswiftlint\nelse\necho \"warning: SwiftLint not installed, download from https://github.com/realm/SwiftLint\"\nfi";
};
D54DFCC41C5AAAF600ADEA0E /* Copy frameworks with Carthage */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
Expand Down
4 changes: 2 additions & 2 deletions Source/Library/LightboxTransition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ extension LightboxTransition: UIViewControllerTransitioningDelegate {
}

func animationController(forPresented presented: UIViewController,
presenting: UIViewController,
source: UIViewController) -> UIViewControllerAnimatedTransitioning? {
presenting: UIViewController,
source: UIViewController) -> UIViewControllerAnimatedTransitioning? {
dismissing = false
return self
}
Expand Down
6 changes: 3 additions & 3 deletions Source/LightboxConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ open class LightboxConfig {

NSURLConnection.sendAsynchronousRequest(imageRequest,
queue: OperationQueue.main,
completionHandler: { response, data, error in
completionHandler: { _, data, error in
if let data = data, let image = UIImage(data: data) {
imageView.image = image
}
Expand Down Expand Up @@ -98,8 +98,8 @@ open class LightboxConfig {
public static var minimumScale: CGFloat = 1.0
public static var maximumScale: CGFloat = 3.0
}

public struct LoadingIndicator {
public static var configure: ((UIActivityIndicatorView) -> Void)? = nil
public static var configure: ((UIActivityIndicatorView) -> Void)?
}
}
10 changes: 5 additions & 5 deletions Source/LightboxController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public protocol LightboxControllerDismissalDelegate: class {
}

public protocol LightboxControllerTouchDelegate: class {

func lightboxController(_ controller: LightboxController, didTouch image: LightboxImage, at index: Int)
}

Expand Down Expand Up @@ -203,9 +203,9 @@ open class LightboxController: UIViewController {
override open func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) {
super.viewWillTransition(to: size, with: coordinator)

coordinator.animate(alongsideTransition: { (UIViewControllerTransitionCoordinatorContext) -> Void in
coordinator.animate(alongsideTransition: { _ in
self.configureLayout(size)
}, completion: nil)
}, completion: nil)
}

// MARK: - Configuration
Expand Down Expand Up @@ -366,7 +366,7 @@ extension LightboxController: PageViewDelegate {
guard !pageView.hasZoomed else { return }

imageTouchDelegate?.lightboxController(self, didTouch: images[currentPage], at: currentPage)

let visible = (headerView.alpha == 1.0)
toggleControls(pageView: pageView, visible: !visible)
}
Expand Down Expand Up @@ -421,6 +421,6 @@ extension LightboxController: FooterViewDelegate {
UIView.animate(withDuration: 0.25, animations: {
self.overlayView.alpha = expanded ? 1.0 : 0.0
self.headerView.deleteButton.alpha = expanded ? 0.0 : 1.0
})
})
}
}
4 changes: 2 additions & 2 deletions Source/LightboxImage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ open class LightboxImage {
self.videoURL = videoURL
}

public init(imageURL: URL, text: String = "", videoURL: URL? = nil ) {
public init(imageURL: URL, text: String = "", videoURL: URL? = nil) {
self.imageURL = imageURL
self.text = text
self.videoURL = videoURL
Expand All @@ -26,7 +26,7 @@ open class LightboxImage {
imageView.image = image
completion?(image)
} else if let imageURL = imageURL {
LightboxConfig.loadImage(imageView, imageURL) { [weak self] error, image in
LightboxConfig.loadImage(imageView, imageURL) { [weak self] _, image in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to draw your attention to the error variable here.
I wanted to do something with it, such as printing it or processing it, but I lack of any relevant ideas. Can we do something about it or is it okay to ignore it in this scope?
Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

The error should probably be passed to the completion so that the user has the chance of knowing what went wrong with loading the remote image.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can open an issue to improve the completion of addImageTo completion. It feels beyond scope for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel the same about the outOfScope exception.
I'll open this issue for enhancement soon.
Thanks for your feedback.

self?.image = image
completion?(image)
}
Expand Down
2 changes: 1 addition & 1 deletion Source/Views/HeaderView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ open class HeaderView: UIView {
} else {
button.sizeToFit()
}

button.addTarget(self, action: #selector(deleteButtonDidPress(_:)),
for: .touchUpInside)

Expand Down
4 changes: 2 additions & 2 deletions Source/Views/InfoLabel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ open class InfoLabel: UILabel {
guard numberOfLines(fullText) > numberOfVisibleLines else {
return truncatedText
}

while numberOfLines(truncatedText) > numberOfVisibleLines * 2 {
truncatedText = String(truncatedText.characters.prefix(truncatedText.characters.count / 2))
}
Expand Down Expand Up @@ -123,7 +123,7 @@ open class InfoLabel: UILabel {
return string.boundingRect(
with: CGSize(width: bounds.size.width, height: CGFloat.greatestFiniteMagnitude),
options: [.usesLineFragmentOrigin, .usesFontLeading],
attributes: [NSFontAttributeName : font],
attributes: [NSFontAttributeName: font],
context: nil).height
}

Expand Down