Opened 11 years ago

Closed 10 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:
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 10 years ago.
0001-Changed-notification-button-to-a-checkbox.patch​ (4.3 KB ) - added by Secris 10 years ago.
Patch with all changes included.
0001-Changed-enable-notification-button-to-a-checkb.patch​ (4.5 KB ) - added by Secris 10 years ago.
Final patch? Fixes a potential issue with launching the Notification Server twice.

Download all attachments as: .zip

Change History (10)

comment:1 by pulkomandy, 10 years ago

Keywords: easy added

comment:2 by pulkomandy, 10 years ago

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

by Secris, 10 years ago

Attachment: 0001-Changed-Enable-notifications-button-to-checkbox.patch​ added

comment:3 by Secris, 10 years ago

patch: 0 → 1

comment:4 by pulkomandy, 10 years ago

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 by Secris, 10 years ago

I will continue working on this.

comment:6 by Secris, 10 years ago

Cc: secris1@… added

by Secris, 10 years ago

Attachment: 0001-Changed-notification-button-to-a-checkbox.patch​ added

Patch with all changes included.

by Secris, 10 years ago

Attachment: 0001-Changed-enable-notification-button-to-a-checkb.patch​ added

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

comment:7 by pulkomandy, 10 years ago

Resolution: → fixed
Status: new → closed

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

Note: See TracTickets for help on using tickets.