-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat(message-row): Add initial support for replies chat #467
base: main
Are you sure you want to change the base?
feat(message-row): Add initial support for replies chat #467
Conversation
abd3744
to
0e04336
Compare
de27f19
to
ef13cb4
Compare
src/tdlib/chat.rs
Outdated
@@ -481,6 +481,10 @@ impl Chat { | |||
self.type_().user() == Some(&self.session().me()) | |||
} | |||
|
|||
pub(crate) fn is_replies_chat(&self) -> bool { | |||
self.id() == 1271266957 || self.id() == 708513 |
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.
Does it have two ids?
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.
The second id is there because the second id is of the test backend bot.
This is also the same way Telegram Desktop implements it (with the exception that it also checks the current backend, but imo it's not needed): https://github.com/telegramdesktop/tdesktop/blob/41d9a9fcbd0c809c60ddbd9350791b1436aff7d9/Telegram/SourceFiles/data/data_peer.cpp#L891
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.
@alissonlauffer Could one of the ids collide with a regular chat? I would suggest to use this
pub(crate) fn is_replies_chat(&self) -> bool {
(self.session_().client_().database_info().0.use_test_dc && self.id() == 708513)
|| self.id() == 1271266957
}
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.
Well, actually, Marco suggested to handle UpdateOption
instead (which includes a replies_bot_chat_id
option), but I'm not sure how to correctly add support to tdlib options, since keys are just strings, and values can be of any type. Tdlib options are a nice thing to have in the session object, but it could be implemented in another PR, which could be merged before this one.
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.
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.
Yes, but we would need too much "boilerplate code" to do that, in my opinion.
But we can add tdlib options on demand also, there's no need to add all of them right away.
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.
we can add tdlib options on demand also.
What do you mean?
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.
I mean, right now we would only require replies_bot_chat_id
, but in the future, if we need more options, we could add one by one. But yeah, this could become an excuse for not using them.
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.
Yes, at Some point we should start using them. 😆
I think using this option is the correct future-proof way, as we never know whether telegram might suddenly change such a magic id.
177eec4
to
1d6994b
Compare
4b97196
to
ce30f16
Compare
ce30f16
to
f16e0ba
Compare
f16e0ba
to
91b84d0
Compare
This PR adds basic support for the "Replies" chat.
Things like
showing replied messages anda button to view the original message should be addressed in other issues.Closes #465.