Split user status and user message in separate screens #3890
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>
Signed-off-by: Milen Pivchev <milen.pivchev@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the user status functionality from UIKit to SwiftUI, splitting the combined user status and status message screen into two separate, focused screens. The refactor removes ~700 lines of UIKit code and replaces it with cleaner SwiftUI implementations while adding a 15-minute duration option to address a mobile-related bug.
Key Changes:
- Complete removal of UIKit-based
NCUserStatus.swiftand storyboard, replacing with SwiftUI views (NCUserStatusViewandNCStatusMessageView) - Separation of status selection (online/away/dnd/invisible/busy) from status message management (emoji + text message with expiration)
- Addition of 15-minute clear duration option alongside existing options (30 min, 1 hr, 4 hrs, day, week)
- Introduction of reusable
EmojiTextFieldcomponent for emoji-only input - Centralization of Xcode preview detection logic in
NCGlobal.swift
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| iOSClient/UserStatus/NCUserStatus.swift | Deleted 722-line UIKit implementation |
| iOSClient/UserStatus/NCUserStatus.storyboard | Deleted corresponding storyboard file |
| iOSClient/UserStatus/NCUserStatusView.swift | New SwiftUI view for selecting user status (online/away/dnd/invisible/busy) |
| iOSClient/UserStatus/NCUserStatusModel.swift | New model managing user status state and API calls |
| iOSClient/StatusMessage/NCStatusMessageView.swift | New SwiftUI view for setting status messages with emoji and expiration |
| iOSClient/StatusMessage/NCStatusMessageModel.swift | New model managing status messages, including 15-minute option |
| iOSClient/StatusMessage/EmojiTextField.swift | New reusable UIKit-backed emoji-only text field component |
| iOSClient/Utility/NCUtility+Image.swift | Minor whitespace formatting improvements |
| iOSClient/NCGlobal.swift | Added global isXcodeRunningForPreviews helper function |
| iOSClient/Extensions/View+Extension.swift | Removed duplicate preview detection, now in NCGlobal |
| iOSClient/Transfers/NCTransfersModel.swift | Updated to use centralized preview detection |
| iOSClient/Data/NCManageDatabase.swift | Renamed previewCreateDB() to createDBForPreview() for consistency |
| Nextcloud.xcodeproj/project.pbxproj | Updated project file with new source files and removed old ones |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| HStack(spacing: 12) { | ||
| EmojiField(text: $model.emojiText) | ||
|
|
||
| TextField("What is your status?", text: $model.statusText) |
There was a problem hiding this comment.
Hardcoded English string "What is your status?" should use a localization key wrapped with NSLocalizedString for proper internationalization support.
Suggested change
| TextField("What is your status?", text: $model.statusText) | |
| TextField(NSLocalizedString("What is your status?", comment: "Prompt for user to enter their status message"), text: $model.statusText) |
Comment on lines
66
to
84
| NextcloudKit.shared.getUserStatus(account: account) { task in | ||
| Task { | ||
| let identifier = await NCNetworking.shared.networkingTasks.createIdentifier(account: self.account, | ||
| name: "getUserStatus") | ||
| await NCNetworking.shared.networkingTasks.track(identifier: identifier, task: task) | ||
| } | ||
| } completion: { account, clearAt, icon, message, messageId, messageIsPredefined, status, statusIsUserDefined, _, _, error in | ||
| if error == .success { | ||
| Task { | ||
| await NCManageDatabase.shared.setAccountUserStatusAsync(userStatusClearAt: clearAt, | ||
| userStatusIcon: icon, | ||
| userStatusMessage: message, | ||
| userStatusMessageId: messageId, | ||
| userStatusMessageIsPredefined: messageIsPredefined, | ||
| userStatusStatus: status, | ||
| userStatusStatusIsUserDefined: statusIsUserDefined, | ||
| account: account) | ||
| } | ||
| } |
There was a problem hiding this comment.
This method uses the callback-based getUserStatus API while other methods in the class use the async/await pattern with getUserStatusAsync. For consistency, consider refactoring to use getUserStatusAsync like in the getStatus method (line 38).
Suggested change
| NextcloudKit.shared.getUserStatus(account: account) { task in | |
| Task { | |
| let identifier = await NCNetworking.shared.networkingTasks.createIdentifier(account: self.account, | |
| name: "getUserStatus") | |
| await NCNetworking.shared.networkingTasks.track(identifier: identifier, task: task) | |
| } | |
| } completion: { account, clearAt, icon, message, messageId, messageIsPredefined, status, statusIsUserDefined, _, _, error in | |
| if error == .success { | |
| Task { | |
| await NCManageDatabase.shared.setAccountUserStatusAsync(userStatusClearAt: clearAt, | |
| userStatusIcon: icon, | |
| userStatusMessage: message, | |
| userStatusMessageId: messageId, | |
| userStatusMessageIsPredefined: messageIsPredefined, | |
| userStatusStatus: status, | |
| userStatusStatusIsUserDefined: statusIsUserDefined, | |
| account: account) | |
| } | |
| } | |
| Task { | |
| let status = await NextcloudKit.shared.getUserStatusAsync(account: account) { task in | |
| Task { | |
| let identifier = await NCNetworking.shared.networkingTasks.createIdentifier(account: self.account, | |
| name: "getUserStatus") | |
| await NCNetworking.shared.networkingTasks.track(identifier: identifier, task: task) | |
| } | |
| } | |
| // Assuming getUserStatusAsync returns a struct with the same fields as the callback | |
| if status.error == .success { | |
| await NCManageDatabase.shared.setAccountUserStatusAsync( | |
| userStatusClearAt: status.clearAt, | |
| userStatusIcon: status.icon, | |
| userStatusMessage: status.message, | |
| userStatusMessageId: status.messageId, | |
| userStatusMessageIsPredefined: status.messageIsPredefined, | |
| userStatusStatus: status.status, | |
| userStatusStatusIsUserDefined: status.statusIsUserDefined, | |
| account: account | |
| ) | |
| } |
Comment on lines
9
to
14
| struct Status: Identifiable, Equatable { | ||
| let id = UUID() | ||
| let emoji: String | ||
| let title: String | ||
| let detail: String | ||
| } |
There was a problem hiding this comment.
The Status struct is defined but never used in the code. It appears to be leftover from development. Consider removing it to keep the codebase clean.
Suggested change
| struct Status: Identifiable, Equatable { | |
| let id = UUID() | |
| let emoji: String | |
| let title: String | |
| let detail: String | |
| } |
| let preset: NKUserStatus | ||
|
|
||
| var body: some View { | ||
| let cleatAtText = model.getPredefinedClearStatusString(clearAt: preset.clearAt, clearAtTime: preset.clearAtTime, clearAtType: preset.clearAtType) |
There was a problem hiding this comment.
Spelling error: 'cleatAtText' should be 'clearAtText'
| let cleatAtText = model.getPredefinedClearStatusString(clearAt: preset.clearAt, clearAtTime: preset.clearAtTime, clearAtType: preset.clearAtType) | ||
|
|
||
| Button(action: { | ||
| model.chooseStatusPreset(preset: preset, clearAtText: cleatAtText) |
There was a problem hiding this comment.
Spelling error: 'cleatAtText' should be 'clearAtText' (same variable name used in line 105)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements: #3793
Prerequisite:
nextcloud/NextcloudKit#204