feat(tray): move logout and remove to settings menu#10165
Conversation
Signed-off-by: Rello <github@scherello.de>
Rello
requested review from
camilasan,
i2h3,
mgallien and
nilsding
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4608a9ef94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Rello <github@scherello.de>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aceda087d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (_accountState->isConnected()) { | ||
| userModel->logout(userId); | ||
| } else { | ||
| userModel->login(userId); |
There was a problem hiding this comment.
When an account is in a non-connected but still signed-in state such as NetworkError, ServiceUnavailable, or NeedToSignTermsOfService, this branch calls login() because isConnected() is false. AccountState::signIn() only transitions from SignedOut, so the new settings button becomes a no-op in those states instead of letting the user log out and clear credentials; please preserve the old isSignedOut() distinction for choosing login versus logout.
Useful? React with 👍 / 👎.
…tead of isConnected() Signed-off-by: Rello <github@scherello.de>
|
Artifact containing the AppImage: nextcloud-appimage-pr-10165.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
| const auto userModel = UserModel::instance(); | ||
| const auto userId = userModel->findUserIdForAccount(_accountState); | ||
| if (userId >= 0) { | ||
| if (_accountState->isSignedOut()) { | ||
| userModel->login(userId); | ||
| } else { | ||
| userModel->logout(userId); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
why does this need to go through UserModel? User::logout and User::login (called by UserModel) do the same thing as the if below here already
There was a problem hiding this comment.
This change makes a lot of sense! 😊 As I already rallied for it, those actions are rather rare for the majority of users and only clutter up the hot paths in the user interface.
Though, the visual difference between the regular ("Log out" appears embossed) and destructive button ("Remove account" appears completely flat) needs to be resolved.
| const auto destructiveTextColor = Theme::instance()->darkMode() ? QColor(0xffdad6) : QColor(0xba1a1a); | ||
| const auto destructiveBorderColor = Theme::instance()->darkMode() ? QColor(0xffb4ab) : QColor(0xba1a1a); | ||
| const auto destructiveBackgroundColor = Theme::instance()->darkMode() ? QColor(0x2d1514) : QColor(0xfffff4f2); | ||
| const auto destructiveHoverBackgroundColor = Theme::instance()->darkMode() ? QColor(0x3b1b1a) : QColor(0xffffe4df); | ||
| const auto destructivePressedBackgroundColor = Theme::instance()->darkMode() ? QColor(0x4a221f) : QColor(0xffffd6cf); |
There was a problem hiding this comment.
In place definition in account settings code of color value constants? I guess this should be moved to an independent and global scope, most likely the Theme itself.
3 participants
the user logout and remove are moved from the tray window user menu into the user settings