D28740: [Notifications] Don't show do not disturb end date beyond 100 days

This revision was automatically updated to reflect the committed changes. Closed by commit R120:a5c9e000b9c9: [Notifications] Dont show do not disturb end date beyond 100 days (authored by broulik). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. All right let's do that on the stable branch to fix the bug and then change the string in master to be consistent. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik added a comment. I find "for a year" odd. Also, this is a bug fix, so can we get this in or not. Changing strings can then be done in master, if we want to. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D28740 To: broulik, #plasma Cc: ngraham,

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

ngraham added a comment. I know, but that's what the menu item says. I'm saying we should make then consistent somehow: either both should say "until turned off" (even if that's a lie) or they should both say "for a year", or "for three months", or whatever. REPOSITORY R120 Plasma

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik added a comment. Technically we don't have a "until turned off" state, just a date. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D28740 To: broulik, #plasma Cc: ngraham, bport, pino, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

ngraham added a comment. Alternative proposal: since showing any date in the UI will be weird, how about we say "until turned off" instead? That would mirror the text in the menu item. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D28740 To: broulik,

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik added a comment. > Why not change what the option do ? so how would you reflect a "forever" as a date? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D28740 To: broulik, #plasma Cc: bport, pino, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev,

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

bport added a comment. In D28740#652419 , @broulik wrote: > This is for displaying whether do not disturb is on. The "until disabled" option just sets it to one year in the future. But it's odd to show "until 2021", so we don't show the date

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik added a comment. This is for displaying whether do not disturb is on. The "until disabled" option just sets it to one year in the future. But it's odd to show "until 2021", so we don't show the date when it's too far in the future. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

bport added a comment. According to settings.h documentation When invalid or in the past, do not disturb mode should be considered disabled. So why not setting date to invalid date or one year in the past and do a test if date is in the future REPOSITORY R120 Plasma Workspace

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik updated this revision to Diff 79818. broulik added a comment. - Also change tooltip Note to future me: put this logic in a shared location REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28740?vs=79816=79818 REVISION DETAIL

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik added a comment. That is exactly what it should do and it did originally but I just noticed it still showed a date, presumable for leap year reasons. With this patch no date is shown again when "until turned off" is chosen. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

pino added a comment. Wouldn't be better instead to not display a date in case the "until turned off" option is chosen? I personally found it confusing the first time I chose this option, and yet a date was shown. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D28740: [Notifications] Don't show do not disturb end date beyond 100 days

broulik created this revision. broulik added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY When setting "until turned off" it effectively sets it to today in a year. Now with leap year