Code review comment for lp:~canonical-dx-team/unity/unity.fix-685830

Revision history for this message
Neil J. Patel (njpatel) wrote :

It looks like you haven't yet hooked everything up so this merge proposal might be a bit premature. I'll comment on what I see anyway:

 - It should be _panel_style not _panelStyle, same goes for anything else like that

- GetDefault(), if _panelStyle == NULL, then create a new one and return it, but if one is already around: return _panelStyle->Reference(); On the other end, the objects should grab panel style object on construction and then ->UnReference it on destruction.

- To be more C++, you should probably have:

nux::Color& GetBackgroundTop ()

etc, so you can do nux::Color top = style->GetBackgroundTop ();, which is cleaner

- Probably can rid of printfs?

- I think the PanelIndicatorObjectEntryView bits still need to be hooked up

- Missing a "changed" signal so we can react to theme changes?

review: Needs Fixing

« Back to merge proposal