Feature/dapp browser tabs #1287
Conversation
| @@ -8,6 +8,7 @@ protocol FileRepositoryProtocol { | |||
| func writeOperation(dataClosure: @escaping () throws -> Data, at path: String) -> BaseOperation<Void> | |||
| func copyOperation(from fromPath: String, to toPath: String) -> BaseOperation<Void> | |||
| func removeOperation(at path: String) -> BaseOperation<Void> | |||
| func removeOperation(at paths: [String]) -> BaseOperation<Void> | |||
There was a problem hiding this comment.
Looks like removeOperation(at path: String) can now move to the protocol extension and implemented via new method added. Don't see necessity to have both methods in the protocol
| let updatedTab = currentTab.updating(with: dApp) | ||
| updateWrapper = tabManager.updateTab(updatedTab) | ||
|
|
||
| dataSource.replace(tab: updatedTab) |
There was a problem hiding this comment.
Why to we skip dataSource update in the .query case?
| transports.first(where: { $0.name == name })?.processConfirmation(response: response) | ||
| } | ||
|
|
||
| func process( | ||
| stateRender: Data, | ||
| tabId: UUID |
There was a problem hiding this comment.
Do we need tabId here as we have current tab in the state?
| render: stateRender | ||
| ) | ||
|
|
||
| execute( |
There was a problem hiding this comment.
I would just use operationQueue.addOperations(wrapper.allOperations, wait: false) if we don't need to handle result
|
|
||
| transports.forEach { $0.stop() } | ||
| completeSetupIfNeeded() | ||
| if case let .dApp(dApp) = newQuery, !currentTab.isBlankPage { |
There was a problem hiding this comment.
Did we confirm the logic: if user selects search string then replace current tab and if user selects dapp then select new tab?
| func updating( | ||
| transportStates: [DAppTransportState]? = nil, | ||
| name: String? = nil, | ||
| lastModified _: Date? = nil, |
There was a problem hiding this comment.
We don't need lastModified here it seems
| final class DAppBrowserTabManager { | ||
| private let cacheBasePath: String | ||
| private let fileRepository: WebViewRenderFilesOperationFactoryProtocol | ||
| private let repository: CoreDataRepository<DAppBrowserTab.PersistenceModel, CDDAppBrowserTab> |
There was a problem hiding this comment.
It should be AnyDataProviderRepository<DAppBrowserTab.PersistenceModel> here
| ) | ||
|
|
||
| let saveRenderWrapper: CompoundOperationWrapper<Void> = if let renderData = tab.stateRender { | ||
| fileRepository.saveRenderOperation(for: tab.uuid, data: { renderData }) |
There was a problem hiding this comment.
So we could serialise renderData from UIImage to Data in the closure instead of doing this in the main thread in the UIViewController
|
|
||
| private var dAppTransportStates: [UUID: [DAppTransportState]] = [:] | ||
| private var tabs: [UUID: DAppBrowserTab] = [:] | ||
| private var observers: [WeakWrapper] = [] |
There was a problem hiding this comment.
I think we completely forgot here about primitives we have to organise reactive code based on the database. I would suggest the following approach:
- Make
PersistentTabLocalSubscriptionFactoryfactory the returnsStreamableProviderfor the entity. There are a lot of examples. For example,DAppLocalSubscriptionFactory - Subscribe to the provider in the TabManager. Now once you update Tab entity the update will automatically arrive to the
TabManager - Make tabs
Observable<[UUID: DAppBrowserTab]> = [:]and update once you receive changes from the provider - Add observers to the state.
That approach avoids a lot of additional logic we have now. Such as fetch tabs and subscribing at the same time (saw in the interactor).
| func removeRenders(for ids: [UUID]) -> CompoundOperationWrapper<Void> | ||
| } | ||
|
|
||
| extension RuntimeFilesOperationFactory: WebViewRenderFilesOperationFactoryProtocol { |
There was a problem hiding this comment.
RuntimeFilesOperationFactory is a separate factory to manage blockchain runtimes. I would create new operation factory for WebViewRenderFilesOperationFactoryProtocol.
- Refactor DAppBrowserTabManager - Add DAppBrowserTabRenderer - Add WebViewRenderImageViewModel
DAppBrowserTabManagerto manage persistence and in-memory state for tabs.WebViewPoolto reuse webviews.WebViewRenderFilesOperationFactoryto manage cache for webview renders.DAppBrowserTabListmoduleDAppBrowserInteractorto deal with tabs instead of dApps or just user queries.