Adds "Share Vault" option to vault detail settings. #424
Conversation
Refactored setupConstraints() to fix function body length violation and applied SwiftFormat auto-formatting.
iammajid
force-pushed
the
feature/hub-sharebutton
branch
from
5c561a1 to
a317f83
Compare
WalkthroughAdds a new ShareVault feature under VaultDetail: four Swift sources (ShareVaultCoordinator, ShareVaultViewController, ShareVaultViewModel, ShareVaultView), project.pbxproj entries/grouping and target source-phase updates, two image assets, and new localization keys. VaultDetailCoordinator gains showShareVault(); VaultDetailViewController handles a new .shareVault action; VaultDetailViewModel adds a shareVault section/button and footer. Also changes Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Cryptomator/VaultDetail/VaultDetailViewModel.swift (1)
115-117: Consider using the disclosure-style button for navigation.Since tapping this row launches the Share Vault flow, it would align with the rest of the settings UI to present it via
ButtonCellViewModel.createDisclosureButton(...), giving the familiar chevron cue alongside rename/move/change password.Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift (1)
123-134: Hide/disable the CTA when no URL is provided
ShareVaultViewModelProtocol.forTeamsURLis optional, yet the button stays tappable when that value is missing, leaving users with a dead control. Please disable (and visually soften) the CTA whenever the view model cannot supply a destination so the UI reflects the available action.let button = UIButton(type: .system) button.setTitle(viewModel.forTeamsButtonTitle, for: .normal) button.backgroundColor = .cryptomatorPrimary button.setTitleColor(.white, for: .normal) button.titleLabel?.font = .preferredFont(forTextStyle: .headline) button.titleLabel?.adjustsFontForContentSizeCategory = true button.layer.cornerRadius = LayoutConstants.cornerRadius button.translatesAutoresizingMaskIntoConstraints = false + let hasTarget = viewModel.forTeamsURL != nil + button.isEnabled = hasTarget + button.alpha = hasTarget ? 1 : 0.5 button.addTarget(self, action: #selector(visitHubButtonTapped), for: .touchUpInside)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
SharedResources/Assets.xcassets/cryptomator-hub-logo.imageset/cryptomator-hub-logo@1x.pngis excluded by!**/*.pngSharedResources/Assets.xcassets/cryptomator-hub-logo.imageset/cryptomator-hub-logo@2x.pngis excluded by!**/*.pngSharedResources/Assets.xcassets/cryptomator-hub-logo.imageset/cryptomator-hub-logo@3x.pngis excluded by!**/*.pngSharedResources/Assets.xcassets/cryptomator-hub.imageset/cryptomator-hub@1x.pngis excluded by!**/*.pngSharedResources/Assets.xcassets/cryptomator-hub.imageset/cryptomator-hub@2x.pngis excluded by!**/*.pngSharedResources/Assets.xcassets/cryptomator-hub.imageset/cryptomator-hub@3x.pngis excluded by!**/*.png
📒 Files selected for processing (10)
Cryptomator.xcodeproj/project.pbxproj(6 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewModel.swift(1 hunks)Cryptomator/VaultDetail/VaultDetailCoordinator.swift(1 hunks)Cryptomator/VaultDetail/VaultDetailViewController.swift(1 hunks)Cryptomator/VaultDetail/VaultDetailViewModel.swift(4 hunks)SharedResources/Assets.xcassets/cryptomator-hub-logo.imageset/Contents.json(1 hunks)SharedResources/Assets.xcassets/cryptomator-hub.imageset/Contents.json(1 hunks)SharedResources/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator.xcodeproj/project.pbxprojCryptomator/VaultDetail/VaultDetailViewController.swiftCryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swiftCryptomator/VaultDetail/VaultDetailCoordinator.swiftCryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift
📚 Learning: 2024-10-14T15:45:13.118Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 386
File: Cryptomator/VaultList/VaultListViewModel.swift:112-112
Timestamp: 2024-10-14T15:45:13.118Z
Learning: In `VaultListViewModel`, use `all(ignoringResult: promises)` instead of `all(promises).then { }` and omit unnecessary logging when refreshing vault lock states.
Applied to files:
Cryptomator/VaultDetail/VaultDetailViewModel.swift
🧬 Code graph analysis (4)
Cryptomator/VaultDetail/VaultDetailViewModel.swift (1)
CryptomatorCommon/Sources/CryptomatorCommonCore/LocalizedString.swift (1)
getValue(12-22)
Cryptomator/VaultDetail/VaultDetailViewController.swift (1)
Cryptomator/VaultDetail/VaultDetailCoordinator.swift (1)
showShareVault(101-106)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (1)
Cryptomator/VaultDetail/VaultDetailCoordinator.swift (1)
start(32-37)
Cryptomator/VaultDetail/VaultDetailCoordinator.swift (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (1)
start(24-34)
🔇 Additional comments (1)
Cryptomator.xcodeproj/project.pbxproj (1)
448-450: ShareVault sources correctly hooked up.Thanks for wiring the new coordinator, view model, and view controller into the Cryptomator target—everything lands in the right build phase with no stray memberships.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new "Share Vault" feature to the Cryptomator iOS app, allowing users to share vaults with team members using Cryptomator Hub. The feature includes UI screens with instructions and links to access the Hub.
Key changes:
- New localized strings for the share vault feature in multiple languages
- Image assets for Cryptomator Hub branding (logo and screenshots)
- New ShareVault module with coordinator, view model, and view controller
- Integration of share vault button in the vault detail screen
Reviewed Changes
Copilot reviewed 10 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| SharedResources/en.lproj/Localizable.strings | Added 13 new localization keys for share vault UI text |
| SharedResources/Assets.xcassets/cryptomator-hub.imageset/* | Added Hub screenshot images at 1x, 2x, and 3x resolutions |
| SharedResources/Assets.xcassets/cryptomator-hub-logo.imageset/* | Added Hub logo images at 1x, 2x, and 3x resolutions |
| Cryptomator/VaultDetail/VaultDetailViewModel.swift | Added share vault section and button action to vault detail screen |
| Cryptomator/VaultDetail/VaultDetailViewController.swift | Added handler for share vault button action |
| Cryptomator/VaultDetail/VaultDetailCoordinator.swift | Added navigation to share vault screen |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tobihagemann
left a comment
tobihagemann
left a comment
There was a problem hiding this comment.
Since we're building new UI here: Was there a reason for using UIKit instead of SwiftUI? Would've thought that SwiftUI could simplify things. I think it should be a long-term goal to migrate to SwiftUI, so we shouldn't pile up our technical debt. 😄 What do you think?
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift (1)
368-382: Based on the verification results, I can now finalize the review. Let me search for any additional context about the codebase's documentation standards.Add documentation for the public HubConfig extension and its methods.
The extension and its methods are now part of the public API (used by ShareVault code at line 44 of ShareVaultCoordinator.swift) but lack documentation. Public declarations require documentation comments, which improve API clarity and enable Xcode Quick Help support.
Apply this diff:
+/// Public helpers for deriving Hub URLs from HubConfig. public extension HubConfig { + /// Returns the API base URL for the Hub instance. + /// + /// If `apiBaseUrl` is set, returns it directly. Otherwise, derives the API base URL + /// by removing the last path component from `devicesResourceUrl`. + /// + /// - Returns: The API base URL, or `nil` if it cannot be determined. func getAPIBaseURL() -> URL? { if let apiBaseUrl { return URL(string: apiBaseUrl) } guard let deviceResourceURL = URL(string: devicesResourceUrl) else { return nil } return deviceResourceURL.deletingLastPathComponent() } + /// Returns the web app URL for the Hub instance. + /// + /// Derives the web app URL by navigating up one level from the API base URL + /// and appending the "app" path component. + /// + /// - Returns: The web app URL, or `nil` if the API base URL cannot be determined. func getWebAppURL() -> URL? { getAPIBaseURL()?.deletingLastPathComponent().appendingPathComponent("app") } }
🧹 Nitpick comments (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (1)
36-45: Consider adding logging for Hub URL extraction failures.The method correctly simplifies the extraction by returning
hubConfig.getWebAppURL()(as per past review feedback). However, thetry?operators silently ignore errors, which could make debugging difficult if URL extraction fails.Consider adding error logging when extraction fails:
private func extractHubVaultURL() -> URL? { - guard let cachedVault = try? VaultDBCache().getCachedVault(withVaultUID: vaultInfo.vaultUID), - let vaultConfigToken = cachedVault.vaultConfigToken, - let vaultConfig = try? UnverifiedVaultConfig(token: vaultConfigToken), - let hubConfig = vaultConfig.allegedHubConfig else { + do { + let cachedVault = try VaultDBCache().getCachedVault(withVaultUID: vaultInfo.vaultUID) + guard let vaultConfigToken = cachedVault.vaultConfigToken, + let vaultConfig = try? UnverifiedVaultConfig(token: vaultConfigToken), + let hubConfig = vaultConfig.allegedHubConfig else { + DDLogWarn("ShareVaultCoordinator: Failed to extract Hub config from vault") + return nil + } + return hubConfig.getWebAppURL() + } catch { + DDLogError("ShareVaultCoordinator: Failed to get cached vault: \(error)") return nil } - - return hubConfig.getWebAppURL() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift(1 hunks)CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift
🧬 Code graph analysis (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (3)
Cryptomator/MainCoordinator.swift (1)
navigationController(92-102)Cryptomator/VaultDetail/VaultDetailCoordinator.swift (1)
start(32-37)CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift (1)
getWebAppURL(379-381)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (freemium)
🔇 Additional comments (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (1)
13-22: Code follows correct memory management pattern.Verification confirms that
ShareVaultViewController.coordinatoris correctly declared asweakat line 13, preventing retain cycles withShareVaultCoordinator. The coordinator setup follows best practices.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift (2)
98-98: Add completion handler for better error handling.The
UIApplication.shared.open(_:)call can fail silently. Consider using the overload with a completion handler to handle potential failures gracefully.Apply this diff:
-.onTapGesture { - UIApplication.shared.open(docsURL) -} +.onTapGesture { + UIApplication.shared.open(docsURL, options: [:]) { success in + if !success { + // Log or handle the error + } + } +}
114-114: Add completion handler for better error handling.Similar to the docs URL, this
UIApplication.shared.open(_:)call should include a completion handler to handle potential failures.Apply this diff:
-action: { - UIApplication.shared.open(url) -}, +action: { + UIApplication.shared.open(url, options: [:]) { success in + if !success { + // Log or handle the error + } + } +},Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (1)
28-31: Consider adding logging for Hub URL extraction failures.While the user-facing error handling is well implemented, adding a log statement when Hub URL extraction fails would help with debugging and troubleshooting in production.
Example addition:
guard let hubURL = extractHubVaultURL() else { DDLogError("Failed to extract Hub URL for vault: \(vaultInfo.vaultUID)") showHubURLExtractionError() return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Cryptomator.xcodeproj/project.pbxproj(7 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewModel.swift(1 hunks)SharedResources/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator.xcodeproj/project.pbxprojCryptomator/VaultDetail/ShareVault/ShareVaultViewController.swiftCryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift
🧬 Code graph analysis (2)
Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift (1)
Cryptomator/VaultDetail/VaultDetailViewController.swift (1)
viewDidLoad(25-41)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (2)
Cryptomator/VaultDetail/VaultDetailCoordinator.swift (1)
start(32-37)CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift (1)
getWebAppURL(379-381)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (freemium)
🔇 Additional comments (5)
Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift (1)
19-46: LGTM!The implementation correctly bridges SwiftUI and UIKit using standard patterns. The weak coordinator reference prevents retain cycles, and the UIHostingController setup follows best practices for embedding SwiftUI views in UIKit.
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (2)
42-50: LGTM!The error handling properly alerts users when Hub URL extraction fails, following the agreed-upon approach from previous discussions. The alert provides clear context and uses localized strings appropriately.
52-61: LGTM!The Hub URL extraction logic is clean and properly handles all failure points with the guard statement chain. The method returns nil on any failure, which is then handled appropriately by the caller.
Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift (1)
30-30: Image asset verified—no issues found.The asset "cryptomator-hub" exists in the project at
SharedResources/Assets.xcassets/cryptomator-hub.imageset/. No runtime issues will occur from the hard-coded image reference.Cryptomator/VaultDetail/ShareVault/ShareVaultViewModel.swift (1)
44-69: Nice encapsulation of localized content.Centralizing the share-vs-hub copy and links behind
ShareVaultTypewill keep the view layer tidy and makes future tweaks to the messaging straightforward. Nicely done.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (2)
46-52: Consider eliminating redundant type check.The method checks
vaultInfo.vaultConfigType == .hubagain, which duplicates the check already performed instart(). Since theShareVaultViewModelalready knows its type (.hubor.normal), consider having the view model indicate the preferred URL opening behavior or storing the type decision made instart()to avoid checking twice.
69-78: Add logging to aid debugging of Hub URL extraction failures.The method uses
try?which silently swallows errors, making it difficult to debug which step failed (cache retrieval, token extraction, config parsing, or Hub config extraction). Consider adding logging at each failure point to improve debugging visibility.For example:
private func extractHubVaultURL() -> URL? { guard let cachedVault = try? VaultDBCache().getCachedVault(withVaultUID: vaultInfo.vaultUID) else { DDLogError("ShareVaultCoordinator: Failed to get cached vault for UID: \(vaultInfo.vaultUID)") return nil } guard let vaultConfigToken = cachedVault.vaultConfigToken else { DDLogError("ShareVaultCoordinator: Vault config token is nil for vault: \(vaultInfo.vaultUID)") return nil } guard let vaultConfig = try? UnverifiedVaultConfig(token: vaultConfigToken) else { DDLogError("ShareVaultCoordinator: Failed to parse vault config for vault: \(vaultInfo.vaultUID)") return nil } guard let hubConfig = vaultConfig.allegedHubConfig else { DDLogError("ShareVaultCoordinator: Hub config is nil for vault: \(vaultInfo.vaultUID)") return nil } guard let webAppURL = hubConfig.getWebAppURL() else { DDLogError("ShareVaultCoordinator: Failed to get web app URL for vault: \(vaultInfo.vaultUID)") return nil } return webAppURL }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift
- Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift
🧬 Code graph analysis (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (2)
Cryptomator/VaultDetail/VaultDetailCoordinator.swift (1)
start(32-37)CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift (1)
getWebAppURL(379-381)
🔇 Additional comments (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift (1)
25-44: Error handling for Hub URL extraction is correct and retain cycles are properly prevented.The implementation correctly addresses previous feedback:
- Error handling for hub URL extraction failures (lines 29-31) presents an alert before returning
- The
coordinatorproperty inShareVaultViewControlleris declared asweak, preventing retain cycles- The
onOpenURLcallback usesweak selfcapture
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Cryptomator/VaultDetail/ShareVault/ShareVaultViewModel.swift (1)
42-66: Consider force-unwrapping hardcoded URLs for early failure detection.The initializer logic correctly configures properties based on vault type. However, the hardcoded URLs at lines 50 and 52 use
URL(string:)which returns an optional. Since these URLs are static and should always be valid, consider force-unwrapping them (URL(string: "...")!) to catch any typos at development time rather than silently producing nil at runtime.Example for line 50:
-self.docsURL = URL(string: "https://docs.cryptomator.org/security/best-practices/#sharing-of-vaults") +self.docsURL = URL(string: "https://docs.cryptomator.org/security/best-practices/#sharing-of-vaults")!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift(1 hunks)Cryptomator/VaultDetail/ShareVault/ShareVaultViewModel.swift(1 hunks)SharedResources/en.lproj/Localizable.strings(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Cryptomator/VaultDetail/ShareVault/ShareVaultCoordinator.swift
- SharedResources/en.lproj/Localizable.strings
- Cryptomator/VaultDetail/ShareVault/ShareVaultView.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (freemium)
🔇 Additional comments (6)
Cryptomator/VaultDetail/ShareVault/ShareVaultViewModel.swift (2)
12-28: LGTM! Clean protocol design.The
ShareVaultTypeenum andShareVaultViewModelProtocolare well-structured. The enum clearly distinguishes between normal and hub-managed vaults, and the protocol provides a clean interface for the view layer.
30-41: LGTM! Immutable view model with proper SwiftUI integration.The use of
letconstants ensures thread safety, andObservableObjectconformance enables reactive SwiftUI updates. The property types appropriately match the protocol requirements.Cryptomator/VaultDetail/ShareVault/ShareVaultViewController.swift (4)
13-21: LGTM! Proper coordinator pattern with retain cycle prevention.The
weakcoordinator reference correctly prevents retain cycles, consistent with the established pattern in the codebase. Theprivateview model ensures proper encapsulation. Based on learnings.
23-31: LGTM! Standard initializer pattern.Dependency injection through the initializer is good practice, and disabling the coder initializer with
fatalErroris the standard pattern for programmatically-created view controllers.
33-37: LGTM! Clean viewDidLoad implementation.The view controller correctly configures its title from the view model and delegates SwiftUI view setup to a dedicated method.
39-46: LGTM! Correct child view controller containment.The SwiftUI bridge implementation follows the proper child view controller containment pattern. The constraint setup using
constraints(equalTo:)assumes a custom extension exists in the codebase for convenience, which is a reasonable abstraction.
New Share Vault section appears in vault detail settings between locking options and vault management. For normal vaults, displays promotional content about Cryptomator Hub with links to documentation and the Hub website. For Hub-managed vaults, shows sharing instructions and provides a direct link to open the vault in Cryptomator Hub.