Code review comment for lp:~townsend/unity/unity.refactor-preview

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

I think it's about time to get rid of the background layer. Now that previews are a bit more mature I think we can remove them from the code as they are not actually used.

Can you get rid of:
dash::Preview::details_bg_layer_
dash::Preview::SetupBackground
dash::preview::Style::GetShadowBackgroundEnabled

The reason why I didn't do this before is that some of the previews don't (or will not) use some of those members you have moved into the common class. eg. socialpreview does not use CoverArt (movie preview will not in future), Social does not use info hints, music and social don't use description.
Also the Payment preview which will be coming through soon won't use most of these fields.

444 + virtual void SetupViews() {}

Don't need it to be there or virtual. It's only called from the constructor of the derived class, so there's no need for it in the base class, esspecially since it doesn't do anything. There's no redundancy issue here because it's always overridden.

review: Needs Fixing

« Back to merge proposal