From 0dfdac6150b700e6a6fd3ee063d5adfd68c4c80d Mon Sep 17 00:00:00 2001 From: Felix Schwarz Date: Thu, 25 Apr 2019 15:22:14 +0200 Subject: [PATCH] - Clean up DisplayViewController and remove force-casts - Fix (47) "the non-openable files prompt the download progress bar in the "details" view" in #237 --- .../Resources/en.lproj/Localizable.strings | 2 +- ownCloud/Viewer/DisplayViewController.swift | 146 +++++++++--------- 2 files changed, 72 insertions(+), 76 deletions(-) diff --git a/ownCloud/Resources/en.lproj/Localizable.strings b/ownCloud/Resources/en.lproj/Localizable.strings index c19a234ec..11bc1aa9d 100644 --- a/ownCloud/Resources/en.lproj/Localizable.strings +++ b/ownCloud/Resources/en.lproj/Localizable.strings @@ -247,7 +247,7 @@ /* Preview */ "Open file" = "Open file"; -"There is no network" = "There is no network"; +"Network unavailable" = "Network unavailable"; "Error" = "Error"; "Could not get the picture" = "Could not get the picture"; "Downloading" = "Downloading"; diff --git a/ownCloud/Viewer/DisplayViewController.swift b/ownCloud/Viewer/DisplayViewController.swift index ed1cb6c7b..21d9f4738 100644 --- a/ownCloud/Viewer/DisplayViewController.swift +++ b/ownCloud/Viewer/DisplayViewController.swift @@ -20,8 +20,8 @@ import UIKit import ownCloudSDK struct DisplayViewConfiguration { - var item: OCItem! - weak var core: OCCore! + var item: OCItem? + weak var core: OCCore? let state: DisplayViewState } @@ -47,15 +47,25 @@ class DisplayViewController: UIViewController, OCQueryDelegate { // MARK: - Configuration var item: OCItem? + private var coreConnectionStatusObservation : NSKeyValueObservation? weak var core: OCCore? { willSet { - if let core = core { - core.removeObserver(self, forKeyPath: "connectionStatus") - } + coreConnectionStatusObservation?.invalidate() + coreConnectionStatusObservation = nil } didSet { if let core = core { - core.addObserver(self, forKeyPath: "connectionStatus", options: [.initial, .new], context: nil) + coreConnectionStatusObservation = core.observe(\OCCore.connectionStatus, options: [.initial, .new]) { [weak self] (core, _) in + guard let state = self?.state, case DisplayViewState.notSupportedMimeType = state else { + if core.connectionStatus == .online { + self?.state = .hasNetworkConnection + } else { + self?.state = .noNetworkConnection + } + + return + } + } } } } @@ -63,7 +73,7 @@ class DisplayViewController: UIViewController, OCQueryDelegate { var source: URL? { didSet { OnMainThread(inline: true) { - self.iconImageView.isHidden = true + self.iconImageView?.isHidden = true self.hideItemMetadataUIElements() self.renderSpecificView() } @@ -92,7 +102,7 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } // MARK: - Views - private var iconImageView: UIImageView! + private var iconImageView: UIImageView? private var progressView : UIProgressView? private var cancelButton : ThemeButton? private var metadataInfoLabel: UILabel? @@ -104,12 +114,6 @@ class DisplayViewController: UIViewController, OCQueryDelegate { // MARK: - Init & Deinit required init() { - iconImageView = UIImageView() - metadataInfoLabel = UILabel() - cancelButton = ThemeButton(type: .custom) - showPreviewButton = ThemeButton(type: .custom) - noNetworkLabel = UILabel() - super.init(nibName: nil, bundle: nil) } @@ -118,65 +122,68 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } deinit { + coreConnectionStatusObservation?.invalidate() + Theme.shared.unregister(client: self) self.downloadProgress?.cancel() self.stopQuery() - if let core = core { - core.removeObserver(self, forKeyPath: "connectionStatus") - } } // MARK: - Controller lifecycle override func loadView() { super.loadView() + iconImageView = UIImageView() + metadataInfoLabel = UILabel() + cancelButton = ThemeButton(type: .custom) + showPreviewButton = ThemeButton(type: .custom) + noNetworkLabel = UILabel() + progressView = UIProgressView(progressViewStyle: .bar) + + guard let iconImageView = iconImageView, let metadataInfoLabel = metadataInfoLabel, let progressView = progressView, let cancelButton = cancelButton, let showPreviewButton = showPreviewButton, let noNetworkLabel = noNetworkLabel else { + return + } + iconImageView.translatesAutoresizingMaskIntoConstraints = false iconImageView.contentMode = .scaleAspectFit view.addSubview(iconImageView) - metadataInfoLabel?.translatesAutoresizingMaskIntoConstraints = false - metadataInfoLabel?.isHidden = false - metadataInfoLabel?.textAlignment = .center - metadataInfoLabel?.adjustsFontForContentSizeCategory = true - metadataInfoLabel?.font = UIFont.preferredFont(forTextStyle: .headline) + metadataInfoLabel.translatesAutoresizingMaskIntoConstraints = false + metadataInfoLabel.isHidden = false + metadataInfoLabel.textAlignment = .center + metadataInfoLabel.adjustsFontForContentSizeCategory = true + metadataInfoLabel.font = UIFont.preferredFont(forTextStyle: .headline) - view.addSubview(metadataInfoLabel!) + view.addSubview(metadataInfoLabel) - progressView = UIProgressView(progressViewStyle: .bar) - progressView?.translatesAutoresizingMaskIntoConstraints = false - progressView?.progress = 0 - progressView?.observedProgress = downloadProgress - progressView?.isHidden = (downloadProgress != nil) + progressView.translatesAutoresizingMaskIntoConstraints = false + progressView.progress = 0 + progressView.observedProgress = downloadProgress + progressView.isHidden = (downloadProgress != nil) - view.addSubview(progressView!) + view.addSubview(progressView) - cancelButton?.translatesAutoresizingMaskIntoConstraints = false - cancelButton?.setTitle("Cancel".localized, for: .normal) - cancelButton?.isHidden = (downloadProgress != nil) - cancelButton?.addTarget(self, action: #selector(cancelDownload(sender:)), for: UIControl.Event.touchUpInside) + cancelButton.translatesAutoresizingMaskIntoConstraints = false + cancelButton.setTitle("Cancel".localized, for: .normal) + cancelButton.isHidden = (downloadProgress != nil) + cancelButton.addTarget(self, action: #selector(cancelDownload(sender:)), for: UIControl.Event.touchUpInside) - view.addSubview(cancelButton!) + view.addSubview(cancelButton) - showPreviewButton?.translatesAutoresizingMaskIntoConstraints = false - showPreviewButton?.setTitle("Open file".localized, for: .normal) - showPreviewButton?.isHidden = true - showPreviewButton?.addTarget(self, action: #selector(downloadItem), for: UIControl.Event.touchUpInside) - view.addSubview(showPreviewButton!) + showPreviewButton.translatesAutoresizingMaskIntoConstraints = false + showPreviewButton.setTitle("Open file".localized, for: .normal) + showPreviewButton.isHidden = true + showPreviewButton.addTarget(self, action: #selector(downloadItem), for: UIControl.Event.touchUpInside) + view.addSubview(showPreviewButton) - noNetworkLabel?.translatesAutoresizingMaskIntoConstraints = false - noNetworkLabel?.isHidden = true - noNetworkLabel?.adjustsFontForContentSizeCategory = true - noNetworkLabel?.text = "There is no network".localized - noNetworkLabel?.textAlignment = .center - noNetworkLabel?.font = UIFont.preferredFont(forTextStyle: .headline) - view.addSubview(noNetworkLabel!) - - guard let iconImageView = iconImageView, let metadataInfoLabel = metadataInfoLabel, let progressView = progressView, - let cancelButton = cancelButton, let showPreviewButton = showPreviewButton, let noNetworkLabel = noNetworkLabel - else { - return - } + noNetworkLabel.translatesAutoresizingMaskIntoConstraints = false + noNetworkLabel.isHidden = true + noNetworkLabel.adjustsFontForContentSizeCategory = true + noNetworkLabel.text = "Network unavailable".localized + noNetworkLabel.textAlignment = .center + noNetworkLabel.font = UIFont.preferredFont(forTextStyle: .headline) + view.addSubview(noNetworkLabel) NSLayoutConstraint.activate([ iconImageView.centerXAnchor.constraint(equalTo: view.safeAreaLayoutGuide.centerXAnchor), @@ -211,7 +218,7 @@ class DisplayViewController: UIViewController, OCQueryDelegate { Theme.shared.register(client: self) if let item = item { - iconImageView.image = item.icon(fitInSize:iconImageSize) + iconImageView?.image = item.icon(fitInSize:iconImageSize) if item.thumbnailAvailability != .none { let displayThumbnail = { (thumbnail: OCItemThumbnail?) in @@ -220,8 +227,8 @@ class DisplayViewController: UIViewController, OCQueryDelegate { image != nil, item.itemVersionIdentifier == thumbnail?.itemVersionIdentifier { OnMainThread { - if !self.iconImageView.isHidden { - self.iconImageView.image = image + if self.iconImageView?.isHidden == false { + self.iconImageView?.image = image } } } @@ -237,6 +244,8 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } } } + + self.render() } func updateNavigationBarItems() { @@ -273,8 +282,8 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } return } - self?.item = latestItem - self?.source = file!.url + self?.item = latestItem + self?.source = file?.url }) { self.state = .downloading(progress: downloadProgress) } @@ -285,7 +294,7 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } func hideItemMetadataUIElements() { - iconImageView.isHidden = true + iconImageView?.isHidden = true progressView?.isHidden = true cancelButton?.isHidden = true metadataInfoLabel?.isHidden = true @@ -293,23 +302,6 @@ class DisplayViewController: UIViewController, OCQueryDelegate { noNetworkLabel?.isHidden = true } - // MARK: - KVO observing - // swiftlint:disable block_based_kvo - override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) { - if let newValue = change?[NSKeyValueChangeKey.newKey] as? OCCoreConnectionStatus { - if case DisplayViewState.notSupportedMimeType = self.state { - - } else { - if newValue == .online { - self.state = .hasNetworkConnection - } else { - self.state = .noNetworkConnection - } - } - } - } - // swiftlint:enable block_based_kvo - private func render() { print("LOG --> State changed to \(state)") switch state { @@ -416,6 +408,10 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } func present(item: OCItem) { + guard self.view != nil else { + return + } + self.item = item metadataInfoLabel?.text = item.sizeLocalized + " - " + item.lastModifiedLocalized