Code review comment for lp:~samuel-buffet/entertainer/reactive_menus
Revision history for this message
| Matt Layman (mblayman) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
35a32c5
(Get the code!)
Review comment of the day:
image_menu.py:
* I need to raise a point that will be contentious. I have to disagree with putting specific logic for videos/albums/clips embedded into a widget. IMO, the only async_add method should be async_add, not async_add_*. I know that this puts more burden on the screen objects, but I strongly believe that classes should serve only the purpose that they were intended for. In the case of the ImageMenu, I think it should only know how to display images that are provided to it. I think that the easy solution would be to move the asyn specific method back into the screen/tab that it came from.
I have a decent idea for how to keep the async_add functionality in ImageMenu, but keep the specific logic out. We can talk about it on IRC. It involves concrete factories as input parameters to async_add (if that makes any sense to you right now, then our conversation on IRC will be a short one).