Opened 6 years ago

Closed 5 years ago

#9426 closed bug (fixed)

Change "Enable notifications" button to checkbox. (easy)

Reported by: mmadia Owned by: pulkomandy
Priority: normal Milestone: R1
Component: Preferences/Notifications Version: R1/Development
Keywords: Cc: secris1@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

hrev45129

The enable/disable button should really be a checkbox "[_] Enable notifications"

Attachments (3)

0001-Changed-Enable-notifications-button-to-checkbox.patch (3.6 KB) - added by Secris 5 years ago.
0001-Changed-notification-button-to-a-checkbox.patch (4.3 KB) - added by Secris 5 years ago.
Patch with all changes included.
0001-Changed-enable-notification-button-to-a-checkb.patch (4.5 KB) - added by Secris 5 years ago.
Final patch? Fixes a potential issue with launching the Notification Server twice.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by pulkomandy

Keywords: easy added

comment:2 Changed 5 years ago by pulkomandy

Keywords: easy removed
Summary: Change "Enable notifications" button to checkbox.Change "Enable notifications" button to checkbox. (easy)

comment:3 Changed 5 years ago by Secris

Has a Patch: set

comment:4 Changed 5 years ago by pulkomandy

There are some more improvements to do:

  • kServer and fServer are not good names. kToggleNotifications and fNotificationsEnabled may be better (or something like this)
  • in GenralView::MessageReceived, the action to take depends on _IsServerRunning() only. There is a risk of the checkbox being out of sync. It should decide the action depending on the checkbox state (you can check using the "be:value" field of the message, or using fServer->Value()). If the action is "enable notifications", it should check that the server is not running. If the action is "disable notifications", the current code will show an alert, which sonds right.

comment:5 Changed 5 years ago by Secris

I will continue working on this.

comment:6 Changed 5 years ago by Secris

Cc: secris1@… added

Changed 5 years ago by Secris

Patch with all changes included.

Changed 5 years ago by Secris

Final patch? Fixes a potential issue with launching the Notification Server twice.

comment:7 Changed 5 years ago by pulkomandy

Resolution: fixed
Status: newclosed

Applied with some extra style fixes in hrev46979. Thanks for working on this!

Note: See TracTickets for help on using tickets.