Menu for picking mute time for specific chat #3678
Conversation
|
Would be nice to have "mute for X hours" in notifications, but I'm not sure of it'd fit. |
|
|
||
| setDimensions(st::boxWidth, y); | ||
| } | ||
| // vi: ts=4 tw=80 |
There was a problem hiding this comment.
Settings for vi(m) so tab and line folding works correctly.
|
@mymedia2 Thanks for your work. This one looks really nice. |
| #include "ui/widgets/checkbox.h" | ||
| #include "ui/widgets/labels.h" | ||
|
|
||
| void MuteSettingsBox::prepare() |
There was a problem hiding this comment.
Please follow the coding style of the rest of the source code, placing the opening curly on the same line that the method name.
| /* This class implements a dialog-box with radio-buttons for pick duration of | ||
| * turning off notifications from a chat. The widget is opened by a context menu | ||
| * in the left list of dialogues. */ | ||
| class MuteSettingsBox |
There was a problem hiding this comment.
Please follow the coding style of the rest of the source code, placing the parent class name and the opening curly on the same line that the class name.
| : public BoxContent | ||
| { | ||
| Q_OBJECT | ||
| public: |
There was a problem hiding this comment.
Coding style: Please place a newline before "public:".
| public: | ||
| MuteSettingsBox(QWidget* parent, PeerData* peer) | ||
| : _peer(peer) | ||
| { |
There was a problem hiding this comment.
Coding style: Please place an open curly on the same line as the last member initialization.
| { | ||
| Q_OBJECT | ||
| public: | ||
| MuteSettingsBox(QWidget* parent, PeerData* peer) |
There was a problem hiding this comment.
Please use "gsl::not_null<PeerData*>" here, because "nullptr" value is not allowed here.
| protected: | ||
| void prepare() override; | ||
| private: | ||
| PeerData* _peer; |
There was a problem hiding this comment.
Please use "gsl::not_null<PeerData*>" here, because "nullptr" value is not allowed here. Also please place a newline before the closing curly of the class definition.
| } | ||
| protected: | ||
| void prepare() override; | ||
| private: |
There was a problem hiding this comment.
Coding style: Please place a newline before "private:".
| : _peer(peer) | ||
| { | ||
| } | ||
| protected: |
There was a problem hiding this comment.
Coding style: Please place a newline before "protected:".
| private: | ||
| PeerData* _peer; | ||
| }; | ||
| // vi: ts=4 tw=80 |
There was a problem hiding this comment.
Contributing policy in general limits pull requests that add new UI elements and / or change the app logic.
But this one looks really nice and really well done, so I'll happily make an exception when some minor changes will be introduced.
I'm not sure that the logic will stay that way in the future (for myself I almost always, 99% of times, disable notifications forever - so I loose a way to do that in two clicks from the context menu and get one more transitional box), but currently this clearly adds value to the user experience.
|
This is modeline[1] for Vim. It says to Vim to treat a tab as four
spaces and sets max line width to 80 characters. Really, very long lines
are difficult to read.
By the way, GitHub also understands some of these options. For example,
you can append `?ts=4` to URL on GitHub to set width of tabs.
[1]: http://vimdoc.sourceforge.net/htmldoc/usr_21.html#21.6
|
Closes: telegramdesktop#3153 Signed-off-by: Nicholas Guriev <[email protected]> (github: mymedia2)
|
Looks good, but why 18 hours? That seems a bit arbitrary. But then again, the Android client's options (1 hour, 8 hours, 2 days) are also quite arbitrary. Would be nice if you could specify a more precise time (at least X hours), although I also rarely mute notifications only temporarily. |
| // the icon is always higher than this chat title | ||
| y += icon->height() + st::boxMediumSkip; | ||
|
|
||
| const int FOREVER = 8760; // in fact, this is mute only for 1 year |
There was a problem hiding this comment.
So that's why muting notifications resets after a year. That's a bit misleading. Is this an API limitation?
There was a problem hiding this comment.
Yes, it is. As far as I can see, there is no true way to mute a chat for ever. And one year was previous behaviour.
See https://core.telegram.org/type/PeerNotifySettings
There was a problem hiding this comment.
Nobody said you can't use INT_MAX, as I suggested. But somehow telegram decided to hardcore 1 year in most clients.
There was a problem hiding this comment.
Interesting. Not sure why they don't use INT_MAX, but then people would see chats unmuted in 2038 if they still use Telegram... MTProto will need some Y2K38-proofing there eventually. ;)
There was a problem hiding this comment.
@nyuszika7h 2038 is better than 2018 :)
|
Thanks for your work! |
| @@ -2244,9 +2245,16 @@ void MainWidget::fillPeerMenu(PeerData *peer, base::lambda<QAction*(const QStrin | |||
| Ui::showPeerProfile(peer); | |||
| }); | |||
| auto muteSubscription = MakeShared<base::Subscription>(); | |||
| auto muteAction = callback(lang(peer->isMuted() ? lng_enable_notifications_from_tray : lng_disable_notifications_from_tray), [peer, muteSubscription] { | |||
There was a problem hiding this comment.
I've accidentally discovered that you've removed "muteSubscription" from the capture list of this handler lambda and that broke the realtime update of that button =(
There was a problem hiding this comment.
I thought the muteSubscription variable wasn't used in that lambda. Okay, I'll make PR to fix it.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Now, an user can choose silent time for a specific chat using Telegram Desktop, like on mobile platforms. The pop-up window may be caused by context menu of dialog list, and it is as follows.