GNOME Bugzilla – Bug 661236
messageTray: Make Source usable without subclassing
Last modified: 2012-05-17 12:33:27 UTC
Just another code cleanup
Created attachment 198574 [details] [review] messageTray: Make Source usable without subclassing Rather than ask most users of Source to subclass it to simply set their icon, just allow them to create a new instance and add it without any complex magic.
Created attachment 198575 [details] [review] Start using MessageTray.Source directly instead of having to subclass it For most subclasses, this is a direct swap -- a lot of the time, the constructor called its superclass's _init and overrided _setSummaryIcon.
Created attachment 198637 [details] [review] messageTray: Make Source usable without subclassing Rather than ask most users of Source to subclass it to simply set their icon, just allow them to create a new instance and add it without any complex magic. Fixed a few issues -- this is getting a bit dirty.
Created attachment 198638 [details] [review] Start using MessageTray.Source directly instead of having to subclass it For most subclasses, this is a direct swap -- a lot of the time, the constructor called its superclass's _init and overrided _setSummaryIcon.
Created attachment 199588 [details] [review] messageTray: Make Source usable without subclassing Rather than ask most users of Source to subclass it to simply set their icon, just allow them to create a new instance and add it without any complex magic. I forgot the first hunk, which made the patch actually work. -function Source(title) { - this._init(title); +function Source(title, icon_name, icon_type) { + this._init(title, icon_name, icon_type);
Created attachment 213332 [details] [review] messageTray: Make Source usable without subclassing Rather than ask most users of Source to subclass it to simply set their icon, just allow them to create a new instance and add it without any complex magic. Rebased onto HEAD.
Created attachment 213333 [details] [review] Start using MessageTray.Source directly instead of having to subclass it For most subclasses, this is a direct swap -- a lot of the time, the constructor called its superclass's _init and overrided _setSummaryIcon.
Created attachment 213777 [details] [review] messageTray: Make Source usable without subclassing Rather than ask most users of Source to subclass it to simply set their icon, just allow them to create a new instance and add it without any complex magic.
Created attachment 213778 [details] [review] Start using MessageTray.Source directly instead of having to subclass it For most subclasses, this is a direct swap -- a lot of the time, the constructor was a blank class that override createNotificationIcon, and called _setSummaryIcon in _init.
Review of attachment 213777 [details] [review]: Yeah makes sense. ::: js/ui/messageTray.js @@ +1000,3 @@ + // this.createNotificationIcon threw an error. + // This could be normal in the case of notificationDaemon, which + // calls _setSummaryIcon itself. That's ugly but at least there is a comment ...
Review of attachment 213778 [details] [review]: Looks good and is a nice cleanup.
Created attachment 214216 [details] [review] messageTray: Make Source usable without subclassing Rather than ask most users of Source to subclass it to simply set their icon, just allow them to create a new instance and add it without any complex magic. Is this alternative better?
Created attachment 214217 [details] [review] Start using MessageTray.Source directly instead of having to subclass it For most subclasses, this is a direct swap -- a lot of the time, the constructor was a blank class that override createNotificationIcon, and called _setSummaryIcon in _init.
Review of attachment 214216 [details] [review]: Yes that's better.
Review of attachment 214217 [details] [review]: Looks good.
Attachment 214216 [details] pushed as c6fabe5 - messageTray: Make Source usable without subclassing Attachment 214217 [details] pushed as 9e1a2cf - Start using MessageTray.Source directly instead of having to subclass it