From 640047e65d41311150f106a282f7849eb7086f35 Mon Sep 17 00:00:00 2001 From: Michael Neuwert Date: Thu, 1 Aug 2019 14:01:23 +0200 Subject: [PATCH] Fix for the PR #447 (keep gallery alive) (#465) * Keeping track of individual OCItems in DisplayViewController instances But loading the list of items in the gallery only once and not reacting to any changes (moving, deleting) * Removed query stop call in DisplayHostViewController Since this query is not created there but just passed from the parent view. * Removing more button if the currently viewed file got moved or deleted * Fixed updating UI after renaming current item * Fixed a warning * Fixed issues in the gallery * - Minor fixes --- ios-sdk | 2 +- .../Viewer/DisplayHostViewController.swift | 234 ++++++++++-------- .../Client/Viewer/DisplayViewController.swift | 88 ++++--- 3 files changed, 176 insertions(+), 148 deletions(-) diff --git a/ios-sdk b/ios-sdk index 0fc61d2f8..fcdad1cc9 160000 --- a/ios-sdk +++ b/ios-sdk @@ -1 +1 @@ -Subproject commit 0fc61d2f84bb3a978d54ed2612daf73457066c0c +Subproject commit fcdad1cc968533d1d826c5276f60b4b316cb8dd5 diff --git a/ownCloud/Client/Viewer/DisplayHostViewController.swift b/ownCloud/Client/Viewer/DisplayHostViewController.swift index e5cf8ae5b..95db656bf 100644 --- a/ownCloud/Client/Viewer/DisplayHostViewController.swift +++ b/ownCloud/Client/Viewer/DisplayHostViewController.swift @@ -7,84 +7,34 @@ // /* - * Copyright (C) 2019, ownCloud GmbH. - * - * This code is covered by the GNU Public License Version 3. - * - * For distribution utilizing Apple mechanisms please see https://owncloud.org/contribute/iOS-license-exception/ - * You should have received a copy of this license along with this program. If not, see . - * - */ +* Copyright (C) 2019, ownCloud GmbH. +* +* This code is covered by the GNU Public License Version 3. +* +* For distribution utilizing Apple mechanisms please see https://owncloud.org/contribute/iOS-license-exception/ +* You should have received a copy of this license along with this program. If not, see . +* +*/ import UIKit import ownCloudSDK class DisplayHostViewController: UIPageViewController { + enum PagePosition { + case before, after + } + // MARK: - Constants - let hasChangesAvailableKeyPath: String = "hasChangesAvailable" let imageFilterRegexp: String = "\\A((image/*))" // Filters all the mime types that are images (incluiding gif and svg) // MARK: - Instance Variables weak private var core: OCCore? - private var lastSelectedLocalID: String? - - private var selectedItem: OCItem { - willSet { - // Remember last selected local ID for the case the selected item disappears and reapears again (e.g. due to some failed action) - lastSelectedLocalID = self.selectedItem.localID - } - } + private var initialItem: OCItem + private var displayedIndex: Int? - private var items: [OCItem]? { - willSet { - if let oldItems = self.items, let newItems = newValue { - if newItems.count > 0 { - if oldItems.count != newItems.count { - let previouslySelectedItem = newItems.first(where: { $0.localID == selectedItem.localID }) - - // Handle the case in which selected item disappears (move, delete) - if oldItems.count > newItems.count { - if previouslySelectedItem == nil, let deletedItem = oldItems.first(where: { $0.localID == selectedItem.localID }) { - if let deletedIndex = oldItems.index(of: deletedItem) { - if deletedIndex < newItems.count { - self.selectedItem = newItems[deletedIndex] - } else { - self.selectedItem = newItems.last! - } - } - } - } - - // Handle the case in which selected item does re-appear (e.g. upon failed move operation) - if oldItems.count < newItems.count && lastSelectedLocalID != nil { - if let reappearingItem = newItems.first(where: { $0.localID == lastSelectedLocalID }) { - self.selectedItem = reappearingItem - } - } - - // Update data source in case number of items has changed - OnMainThread { [weak self] in - self?.updateDataSource(animated: true) - } - } - - } else { - // If there is nothing to display, go back to the previous view in the navigation stack - OnMainThread { [weak self] in - self?.navigationController?.popViewController(animated: true) - } - } - - } - } - didSet { - OnMainThread { [weak self] in - self?.configureScrolling() - } - } - } + private var items: [OCItem]? private var query: OCQuery private var queryStarted : Bool = false @@ -96,23 +46,28 @@ class DisplayHostViewController: UIPageViewController { // MARK: - Init & deinit init(core: OCCore, selectedItem: OCItem, query: OCQuery) { self.core = core - self.selectedItem = selectedItem + self.initialItem = selectedItem self.query = query super.init(transitionStyle: .scroll, navigationOrientation: .horizontal, options: nil) if query.state == .stopped { - core.start(query) + self.core?.start(query) queryStarted = true } queryObservation = query.observe(\OCQuery.hasChangesAvailable, options: [.initial, .new]) { [weak self] (query, _) in + //guard self?.items == nil else { return } + query.requestChangeSet(withFlags: .onlyResults) { ( _, changeSet) in guard let changeSet = changeSet else { return } - if let queryResult = changeSet.queryResult, let items = self?.applyImageFilesFilter(items: queryResult) { - Log.log("Presenting items (DisplayHOSTViewController.queryObservation): \(items.description)") + if let queryResult = changeSet.queryResult, let newItems = self?.applyImageFilesFilter(items: queryResult) { + let shallUpdateDatasource = self?.items?.count != newItems.count ? true : false - self?.items = items + self?.items = newItems + if shallUpdateDatasource { + self?.updateDatasource() + } } } } @@ -137,12 +92,19 @@ class DisplayHostViewController: UIPageViewController { // MARK: - ViewController lifecycle override func viewDidLoad() { super.viewDidLoad() - updateDataSource() - } - override func viewWillAppear(_ animated: Bool) { - super.viewWillAppear(animated) - configureScrolling() + self.dataSource = self + self.delegate = self + + if let initialViewController = viewController(for: self.initialItem) { + self.setViewControllers([initialViewController], direction: .forward, animated: false, completion: nil) + + if let displayController = initialViewController as? DisplayViewController, let items = self.items, let initialID = self.initialItem.localID, + let currentIndex = items.firstIndex(where: {$0.localID == initialID}) { + self.displayedIndex = currentIndex + displayController.itemIndex = currentIndex + } + } } override var childForHomeIndicatorAutoHidden : UIViewController? { @@ -188,24 +150,60 @@ class DisplayHostViewController: UIPageViewController { // MARK: - Helper methods - private func updateDataSource(animated:Bool = false) { - // First reset data source, to make sure that when it is again set, the page view controller does actually reload - self.dataSource = nil - self.dataSource = self - self.delegate = self + private func updateDatasource() { + OnMainThread { [weak self] in + self?.dataSource = nil + if let itemCount = self?.items?.count { + if itemCount > 0 { + + if let currentDisplayViewController = self?.viewControllers?.first as? DisplayViewController, + let item = currentDisplayViewController.item, + let index = currentDisplayViewController.itemIndex { - // Display first item - guard let mimeType = self.selectedItem.mimeType else { return } + let foundIndex = self?.items?.firstIndex(where: {$0.localID == item.localID}) + + if foundIndex == nil { + if index < itemCount { + if let newIndex = self?.computeNewIndex(for: index, itemCount: itemCount, position: .after, indexFound: false), + let newViewController = self?.viewControllerAtIndex(index: newIndex) { + self?.setViewControllers([newViewController], direction: .forward, animated: false, completion: nil) + } + } else { + if let newIndex = self?.computeNewIndex(for: index, itemCount: itemCount, position: .before, indexFound: false), + let newViewController = self?.viewControllerAtIndex(index: newIndex) { + self?.setViewControllers([newViewController], direction: .reverse, animated: false, completion: nil) + } + } + } + } - let viewController = self.selectDisplayViewControllerBasedOn(mimeType: mimeType) - let configuration = self.configurationFor(self.selectedItem, viewController: viewController) + self?.dataSource = self + } + } + } + } - viewController.configure(configuration) + func computeNewIndex(for currentIndex:Int, itemCount:Int, position:PagePosition, indexFound:Bool = true) -> Int? { + switch position { + case .after: + if indexFound { + if currentIndex < (itemCount - 1) { + return currentIndex + 1 + } + } else { + // If current index was moved, next element in the list will assume it's position + if currentIndex < itemCount { + return currentIndex + } + } - self.setViewControllers([viewController], direction: .forward, animated: animated, completion: nil) + case .before: + if currentIndex > 0 { + return currentIndex - 1 + } + } - viewController.present(item: self.selectedItem) - viewController.updateNavigationBarItems() + return nil } private func viewControllerAtIndex(index: Int) -> UIViewController? { @@ -215,11 +213,22 @@ class DisplayHostViewController: UIPageViewController { let item = items[index] - let newViewController = selectDisplayViewControllerBasedOn(mimeType: item.mimeType!) + let viewController = self.viewController(for: item) + (viewController as? DisplayViewController)?.itemIndex = index + + return viewController + } + + private func viewController(for item:OCItem) -> UIViewController? { + + guard let mimeType = item.mimeType else { return nil } + + let newViewController = selectDisplayViewControllerBasedOn(mimeType: mimeType) let configuration = configurationFor(item, viewController: newViewController) newViewController.configure(configuration) newViewController.present(item: item) + return newViewController } @@ -236,38 +245,50 @@ class DisplayHostViewController: UIPageViewController { // MARK: - Filters private func applyImageFilesFilter(items: [OCItem]) -> [OCItem] { - if selectedItem.mimeType?.matches(regExp: imageFilterRegexp) ?? false { + if initialItem.mimeType?.matches(regExp: imageFilterRegexp) ?? false { let filteredItems = items.filter({$0.type != .collection && $0.mimeType?.matches(regExp: self.imageFilterRegexp) ?? false}) return filteredItems } else { - let filteredItems = items.filter({$0.type != .collection && $0.fileID == self.selectedItem.fileID}) + let filteredItems = items.filter({$0.type != .collection && $0.fileID == self.initialItem.fileID}) return filteredItems } } } extension DisplayHostViewController: UIPageViewControllerDataSource { - func pageViewController(_ pageViewController: UIPageViewController, viewControllerAfter viewController: UIViewController) -> UIViewController? { - if let displayViewController = viewControllers?.first as? DisplayViewController, - let item = displayViewController.item, - let index = items?.firstIndex(where: {$0.fileID == item.fileID}) { - return viewControllerAtIndex(index: index + 1) - } - return nil + private func vendNewViewController(from viewController:UIViewController, _ position:PagePosition) -> UIViewController? { + guard let displayViewController = viewControllers?.first as? DisplayViewController else { return nil } + guard let item = displayViewController.item, let items = self.items else { return nil } - } + // Is the item assigned to the currently visible view controller still available? + let index = items.firstIndex(where: {$0.localID == item.localID}) - func pageViewController(_ pageViewController: UIPageViewController, viewControllerBefore viewController: UIViewController) -> UIViewController? { + if index != nil { + // If so, then vend view controller with the item next to the current item + if let nextIndex = computeNewIndex(for: index!, itemCount:items.count, position: position) { + return viewControllerAtIndex(index: nextIndex) + } - if let displayViewController = viewControllers?.first as? DisplayViewController, - let item = displayViewController.item, - let index = items?.firstIndex(where: {$0.fileID == item.fileID}) { - return viewControllerAtIndex(index: index - 1) + } else { + // Currently visible item was deleted or moved, use it's old index to find a new one + if let index = displayViewController.itemIndex { + if let nextIndex = computeNewIndex(for: index, itemCount:items.count, position: position, indexFound: false) { + return viewControllerAtIndex(index: nextIndex) + } + } } return nil } + + func pageViewController(_ pageViewController: UIPageViewController, viewControllerAfter viewController: UIViewController) -> UIViewController? { + return vendNewViewController(from: viewController, .after) + } + + func pageViewController(_ pageViewController: UIPageViewController, viewControllerBefore viewController: UIViewController) -> UIViewController? { + return vendNewViewController(from: viewController, .before) + } } extension DisplayHostViewController: UIPageViewControllerDelegate { @@ -292,11 +313,6 @@ extension DisplayHostViewController: UIPageViewControllerDelegate { self.viewControllerToTansition = viewControllerToTransition } } - - private func configureScrolling() { - guard let items = self.items else { return } - self.dataSource = items.count > 1 ? self : nil - } } extension DisplayHostViewController: Themeable { diff --git a/ownCloud/Client/Viewer/DisplayViewController.swift b/ownCloud/Client/Viewer/DisplayViewController.swift index f1a0a788f..f522813eb 100644 --- a/ownCloud/Client/Viewer/DisplayViewController.swift +++ b/ownCloud/Client/Viewer/DisplayViewController.swift @@ -49,6 +49,8 @@ class DisplayViewController: UIViewController, OCQueryDelegate { // MARK: - Configuration var item: OCItem? + var itemIndex: Int? + private var coreConnectionStatusObservation : NSKeyValueObservation? weak var core: OCCore? { willSet { @@ -263,14 +265,23 @@ class DisplayViewController: UIViewController, OCQueryDelegate { self.render() } + override func viewWillAppear(_ animated: Bool) { + super.viewWillAppear(animated) + updateNavigationBarItems() + } + func updateNavigationBarItems() { - if let parent = parent, let itemName = item?.name { + if let parent = parent, let itemName = item?.name, let queryState = query?.state { parent.navigationItem.title = itemName if shallDisplayMoreButtonInToolbar { - let actionsBarButtonItem = UIBarButtonItem(image: UIImage(named: "more-dots"), style: .plain, target: self, action: #selector(optionsBarButtonPressed)) - actionsBarButtonItem.accessibilityLabel = itemName + " " + "Actions".localized - parent.navigationItem.rightBarButtonItem = actionsBarButtonItem + if queryState != .targetRemoved { + let actionsBarButtonItem = UIBarButtonItem(image: UIImage(named: "more-dots"), style: .plain, target: self, action: #selector(optionsBarButtonPressed)) + actionsBarButtonItem.accessibilityLabel = itemName + " " + "Actions".localized + parent.navigationItem.rightBarButtonItem = actionsBarButtonItem + } else { + parent.navigationItem.rightBarButtonItem = nil + } } } } @@ -348,6 +359,7 @@ class DisplayViewController: UIViewController, OCQueryDelegate { self.cancelButton?.isHidden = true self.infoLabel?.isHidden = true self.showPreviewButton?.isHidden = true + } } @@ -367,12 +379,7 @@ class DisplayViewController: UIViewController, OCQueryDelegate { let actionsLocation = OCExtensionLocation(ofType: .action, identifier: .moreItem) let actionContext = ActionContext(viewController: self, core: core, items: [item], location: actionsLocation) - if let moreViewController = Action.cardViewController(for: item, with: actionContext, completionHandler: { [weak self] (action, _) in - - if action is RenameAction { - self?.updateNavigationBarItems() - } - }) { + if let moreViewController = Action.cardViewController(for: item, with: actionContext, completionHandler: nil) { self.present(asCard: moreViewController, animated: true) } } @@ -408,20 +415,24 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } func queryHasChangesAvailable(_ query: OCQuery) { - query.requestChangeSet(withFlags: OCQueryChangeSetRequestFlag(rawValue: 0)) { (query, changeSet) in + query.requestChangeSet(withFlags: .onlyResults) { [weak self] (query, changeSet) in OnMainThread { + Log.log("Presenting item (DisplayViewController.queryHasChangesAvailable): \(changeSet?.queryResult.description ?? "nil") - state: \(String(describing: query.state.rawValue))") + switch query.state { case .idle, .contentsFromCache, .waitingForServerReply: - Log.log("Presenting item (DisplayViewController.queryHasChangesAvailable): \(changeSet?.queryResult.description ?? "nil")") - if let firstItem = changeSet?.queryResult.first { - if (firstItem.syncActivity != .updating) && ((firstItem.itemVersionIdentifier != self.item?.itemVersionIdentifier) || (firstItem.name != self.item?.name)) { - self.present(item: firstItem) + if (firstItem.syncActivity != .updating) && ((firstItem.itemVersionIdentifier != self?.item?.itemVersionIdentifier) || (firstItem.name != self?.item?.name)) { + self?.present(item: firstItem) + } else { + self?.item = firstItem } } - case .targetRemoved: break + self?.updateNavigationBarItems() + case .targetRemoved: + self?.updateNavigationBarItems() default: break } } @@ -429,41 +440,42 @@ class DisplayViewController: UIViewController, OCQueryDelegate { } func present(item: OCItem) { - guard self.view != nil else { + guard self.view != nil, item.removed == false else { return } - self.item = item - - metadataInfoLabel?.text = item.sizeLocalized + " - " + item.lastModifiedLocalized - - Log.log("Presenting item (DisplayViewController.present): \(item.description)") - switch state { case .notSupportedMimeType: break default: - self.stopQuery() + self.item = item + metadataInfoLabel?.text = item.sizeLocalized + " - " + item.lastModifiedLocalized + + Log.log("Presenting item (DisplayViewController.present): \(item.description)") + self.startQuery() - if requiresLocalItemCopy { - if core?.localCopy(of: item) == nil { - self.downloadItem(sender: nil) - } else { - if let core = core, let file = item.file(with: core) { - self.source = file.url + if source == nil { + if requiresLocalItemCopy { + if core?.localCopy(of: item) == nil { + self.downloadItem(sender: nil) + } else { + if let core = core, let file = item.file(with: core) { + self.source = file.url + } } + } else { + core?.provideDirectURL(for: item, allowFileURL: true, completionHandler: { (error, url, authHeaders) in + if error == nil { + self.httpAuthHeaders = authHeaders + self.source = url + } + }) } - } else { - core?.provideDirectURL(for: item, allowFileURL: true, completionHandler: { (error, url, authHeaders) in - if error == nil { - self.httpAuthHeaders = authHeaders - self.source = url - } - }) } - updateNavigationBarItems() + self.updateNavigationBarItems() + } } }