-
-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[trello.com/c/F7EV0pKq] nodes are not enabled alert #643
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
alert.addAction(showListAction) | ||
alert.addAction(cancelAction) | ||
|
||
self.present(alert, animated: true, completion: nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ChatDialogManager
. Let's not overcomplicate this huge ChatViewController
@@ -783,6 +784,37 @@ private extension ChatViewController { | |||
} | |||
} | |||
|
|||
func checkNodesEnabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too little logics, let's remove this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Uncle Bob said
If the functions are small it will be easier to comprehend the code. IDK I think that's ideal
@@ -985,6 +991,9 @@ | |||
/* Shared alert 'Delete' button. Used anywhere. */ | |||
"Shared.Delete" = "Удалить"; | |||
|
|||
/* Shared alert 'Review Node List' button. Used on Chat screen. */ | |||
"Shared.ReviewNodeList" = "К списку узлов ADM"; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add strings for all the other languages as well (use english)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check the comments
@@ -199,6 +201,26 @@ private extension ChatDialogManager { | |||
dialogService.present(alert, animated: true, completion: nil) | |||
} | |||
|
|||
func showNoEnabledNodesAlert(showAction: @escaping ()->Void) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@escaping () -> Void
@@ -17,6 +17,7 @@ enum ChatDialog { | |||
case warning(String) | |||
case richError(Error) | |||
case freeTokenAlert | |||
case noEnabledNodesAlert(()->Void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
showNodesListAction: () -> Void
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move it to ChatLocalization
, because this alert is used on the chat screen only
@@ -161,6 +161,7 @@ final class ChatViewController: MessagesViewController { | |||
defer { viewAppeared = true } | |||
inputBar.isUserInteractionEnabled = true | |||
chatMessagesCollectionView.fixedBottomOffset = nil | |||
checkNodesEnabled() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not show the alert again on the same chat screen. If we close this chat screen and open the same chat screen — it’s counted as another chat screen.
@@ -199,6 +201,26 @@ private extension ChatDialogManager { | |||
dialogService.present(alert, animated: true, completion: nil) | |||
} | |||
|
|||
func showNodeListAlert(showAction: @escaping () -> Void) { | |||
let alert = UIAlertController( | |||
title: String.adamant.chat.admNoActiveNodesTitle, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.adamant.chat.admNoActiveNodesTitle
Purpose of the task is to notify user with alert which appears when entering the chat screen if all nodes are switched off.