Unity

Merge lp:~townsend/unity/unity.refactor-preview into lp:unity

  1. unity.refactor-preview
  2. Merge into trunk
Proposed by Christopher Townsend
Status: Merged
Approved by: Brandon Schaefer
Approved revision: no longer in the source branch.
Merged at revision: 3006
Proposed branch: lp:~townsend/unity/unity.refactor-preview
Merge into: lp:unity
Diff against target: 567 lines (+36/-152)
12 files modified
dash/previews/ApplicationPreview.cpp (+0/-8)
dash/previews/ApplicationPreview.h (+1/-21)
dash/previews/GenericPreview.cpp (+0/-8)
dash/previews/GenericPreview.h (+1/-24)
dash/previews/MoviePreview.cpp (+0/-8)
dash/previews/MoviePreview.h (+1/-22)
dash/previews/MusicPreview.cpp (+0/-10)
dash/previews/MusicPreview.h (+1/-22)
dash/previews/Preview.cpp (+10/-0)
dash/previews/Preview.h (+21/-1)
dash/previews/SocialPreview.cpp (+0/-8)
dash/previews/SocialPreview.h (+1/-20)
To merge this branch: bzr merge lp:~townsend/unity/unity.refactor-preview
Reviewer Review Type Date Requested Status
Nick Dedekind (community) Needs Fixing
Brandon Schaefer (community) Approve
PS Jenkins bot continuous-integration Pending
Review via email: mp+140976@code.launchpad.net

Commit message

Refactored the derived preview classes to put the common members into the base Preview class. This should help alleviate code redundancy, code readability, and make the creation of any future new derived preview classes easier.

Description of the change

= Issue =
The derived preview classes(ApplicationPreview, GenericPreview, etc.) have a bunch of redundancy and common members that can be moved into the base Preview class.

= Fix =
This moves the common members into the base Preview class. This should alleviate code redundancy, code readability, and make it easier to create any future new derived preview classes.

= Test =
This does not change any functionality, so the current tests should be sufficient.

To post a comment you must log in.
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Awesome! Nice clean up, everything looks good, compiles, and things still work :).

review: Approve
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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'dash/previews/ApplicationPreview.cpp'
2--- dash/previews/ApplicationPreview.cpp 2012-12-14 12:14:34 +0000
3+++ dash/previews/ApplicationPreview.cpp 2012-12-20 19:09:20 +0000
4@@ -24,7 +24,6 @@
5 #include "unity-shared/PreviewStyle.h"
6 #include "unity-shared/CoverArt.h"
7 #include "unity-shared/IconTexture.h"
8-#include "unity-shared/StaticCairoText.h"
9 #include "unity-shared/PlacesOverlayVScrollBar.h"
10 #include <UnityCore/ApplicationPreview.h>
11 #include <NuxCore/Logger.h>
12@@ -38,7 +37,6 @@
13
14 #include "ApplicationPreview.h"
15 #include "ActionButton.h"
16-#include "PreviewInfoHintWidget.h"
17 #include "PreviewRatingsWidget.h"
18
19 namespace unity
20@@ -64,7 +62,6 @@
21
22 ApplicationPreview::ApplicationPreview(dash::Preview::Ptr preview_model)
23 : Preview(preview_model)
24-, full_data_layout_(nullptr)
25 {
26 SetupBackground();
27 SetupViews();
28@@ -133,11 +130,6 @@
29 Preview::AddProperties(builder);
30 }
31
32-void ApplicationPreview::SetupBackground()
33-{
34- details_bg_layer_.reset(dash::previews::Style::Instance().GetBackgroundLayer());
35-}
36-
37 void ApplicationPreview::SetupViews()
38 {
39 dash::ApplicationPreview* app_preview_model = dynamic_cast<dash::ApplicationPreview*>(preview_model_.get());
40
41=== modified file 'dash/previews/ApplicationPreview.h'
42--- dash/previews/ApplicationPreview.h 2012-12-14 12:14:34 +0000
43+++ dash/previews/ApplicationPreview.h 2012-12-20 19:09:20 +0000
44@@ -24,14 +24,6 @@
45 #define APPLICATIONPREVIEW_H
46
47 #include "Preview.h"
48-#include <Nux/Nux.h>
49-
50-namespace nux
51-{
52-class AbstractPaintLayer;
53-class VLayout;
54-class StaticCairoText;
55-}
56
57 namespace unity
58 {
59@@ -41,9 +33,7 @@
60 {
61 namespace previews
62 {
63-class CoverArt;
64 class PreviewRatingsWidget;
65-class PreviewInfoHintWidget;
66
67 class ApplicationPreview : public Preview
68 {
69@@ -63,26 +53,16 @@
70 virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw);
71 virtual void PreLayoutManagement();
72
73- void SetupBackground();
74- void SetupViews();
75+ virtual void SetupViews();
76
77 protected:
78- nux::VLayout* full_data_layout_;
79 nux::VLayout* title_subtitle_layout_;
80
81- nux::ObjectPtr<CoverArt> image_;
82 nux::ObjectPtr<IconTexture> app_icon_;
83 nux::ObjectPtr<PreviewRatingsWidget> app_rating_;
84- nux::ObjectPtr<StaticCairoText> title_;
85- nux::ObjectPtr<StaticCairoText> subtitle_;
86 nux::ObjectPtr<StaticCairoText> license_;
87 nux::ObjectPtr<StaticCairoText> last_update_;
88 nux::ObjectPtr<StaticCairoText> copywrite_;
89- nux::ObjectPtr<StaticCairoText> description_;
90- nux::ObjectPtr<PreviewInfoHintWidget> preview_info_hints_;
91-
92- typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
93- LayerPtr details_bg_layer_;
94 };
95
96 }
97
98=== modified file 'dash/previews/GenericPreview.cpp'
99--- dash/previews/GenericPreview.cpp 2012-12-14 12:14:34 +0000
100+++ dash/previews/GenericPreview.cpp 2012-12-20 19:09:20 +0000
101@@ -23,7 +23,6 @@
102 #include "unity-shared/IntrospectableWrappers.h"
103 #include "unity-shared/PreviewStyle.h"
104 #include "unity-shared/CoverArt.h"
105-#include "unity-shared/StaticCairoText.h"
106 #include "unity-shared/PlacesOverlayVScrollBar.h"
107 #include <NuxCore/Logger.h>
108 #include <Nux/HLayout.h>
109@@ -33,7 +32,6 @@
110 #include <Nux/AbstractButton.h>
111
112 #include "GenericPreview.h"
113-#include "PreviewInfoHintWidget.h"
114
115 namespace unity
116 {
117@@ -58,7 +56,6 @@
118
119 GenericPreview::GenericPreview(dash::Preview::Ptr preview_model)
120 : Preview(preview_model)
121-, full_data_layout_(nullptr)
122 {
123 SetupBackground();
124 SetupViews();
125@@ -127,11 +124,6 @@
126 Preview::AddProperties(builder);
127 }
128
129-void GenericPreview::SetupBackground()
130-{
131- details_bg_layer_.reset(dash::previews::Style::Instance().GetBackgroundLayer());
132-}
133-
134 void GenericPreview::SetupViews()
135 {
136 if (!preview_model_)
137
138=== modified file 'dash/previews/GenericPreview.h'
139--- dash/previews/GenericPreview.h 2012-12-14 12:14:34 +0000
140+++ dash/previews/GenericPreview.h 2012-12-20 19:09:20 +0000
141@@ -24,24 +24,14 @@
142 #define GENERICPREVIEW_H
143
144 #include "Preview.h"
145-#include <Nux/Nux.h>
146 #include <UnityCore/GenericPreview.h>
147
148-namespace nux
149-{
150-class AbstractPaintLayer;
151-class StaticCairoText;
152-class VLayout;
153-}
154-
155 namespace unity
156 {
157 namespace dash
158 {
159 namespace previews
160 {
161-class CoverArt;
162-class PreviewInfoHintWidget;
163
164 class GenericPreview : public Preview
165 {
166@@ -61,20 +51,7 @@
167 virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw);
168 virtual void PreLayoutManagement();
169
170- void SetupBackground();
171- void SetupViews();
172-
173-protected:
174- nux::VLayout* full_data_layout_;
175-
176- nux::ObjectPtr<CoverArt> image_;
177- nux::ObjectPtr<StaticCairoText> title_;
178- nux::ObjectPtr<StaticCairoText> subtitle_;
179- nux::ObjectPtr<StaticCairoText> description_;
180- nux::ObjectPtr<PreviewInfoHintWidget> preview_info_hints_;
181-
182- typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
183- LayerPtr details_bg_layer_;
184+ virtual void SetupViews();
185 };
186
187 }
188
189=== modified file 'dash/previews/MoviePreview.cpp'
190--- dash/previews/MoviePreview.cpp 2012-12-14 12:14:34 +0000
191+++ dash/previews/MoviePreview.cpp 2012-12-20 19:09:20 +0000
192@@ -23,7 +23,6 @@
193 #include "unity-shared/IntrospectableWrappers.h"
194 #include "unity-shared/PreviewStyle.h"
195 #include "unity-shared/CoverArt.h"
196-#include "unity-shared/StaticCairoText.h"
197 #include "unity-shared/PlacesOverlayVScrollBar.h"
198 #include <UnityCore/MoviePreview.h>
199 #include <NuxCore/Logger.h>
200@@ -33,7 +32,6 @@
201 #include <Nux/AbstractButton.h>
202
203 #include "MoviePreview.h"
204-#include "PreviewInfoHintWidget.h"
205 #include "PreviewRatingsWidget.h"
206
207 namespace unity
208@@ -59,7 +57,6 @@
209
210 MoviePreview::MoviePreview(dash::Preview::Ptr preview_model)
211 : Preview(preview_model)
212-, full_data_layout_(nullptr)
213 {
214 SetupBackground();
215 SetupView();
216@@ -136,11 +133,6 @@
217 {
218 }
219
220-void MoviePreview::SetupBackground()
221-{
222- details_bg_layer_.reset(dash::previews::Style::Instance().GetBackgroundLayer());
223-}
224-
225 void MoviePreview::SetupView()
226 {
227 dash::MoviePreview* movie_preview_model = dynamic_cast<dash::MoviePreview*>(preview_model_.get());
228
229=== modified file 'dash/previews/MoviePreview.h'
230--- dash/previews/MoviePreview.h 2012-12-14 12:14:34 +0000
231+++ dash/previews/MoviePreview.h 2012-12-20 19:09:20 +0000
232@@ -24,14 +24,6 @@
233 #define MOVIEPREVIEW_H
234
235 #include "Preview.h"
236-#include <Nux/Nux.h>
237-
238-namespace nux
239-{
240-class AbstractPaintLayer;
241-class StaticCairoText;
242-class VLayout;
243-}
244
245 namespace unity
246 {
247@@ -39,9 +31,7 @@
248 {
249 namespace previews
250 {
251-class CoverArt;
252 class PreviewRatingsWidget;
253-class PreviewInfoHintWidget;
254
255 class MoviePreview : public Preview
256 {
257@@ -64,21 +54,10 @@
258 virtual void OnNavigateOut();
259 virtual void OnNavigateInComplete();
260
261- void SetupBackground();
262- void SetupView();
263+ virtual void SetupView();
264
265 protected:
266- nux::VLayout* full_data_layout_;
267-
268- nux::ObjectPtr<CoverArt> image_;
269 nux::ObjectPtr<PreviewRatingsWidget> rating_;
270- nux::ObjectPtr<StaticCairoText> title_;
271- nux::ObjectPtr<StaticCairoText> subtitle_;
272- nux::ObjectPtr<StaticCairoText> description_;
273- nux::ObjectPtr<PreviewInfoHintWidget> preview_info_hints_;
274-
275- typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
276- LayerPtr details_bg_layer_;
277 };
278
279 }
280
281=== modified file 'dash/previews/MusicPreview.cpp'
282--- dash/previews/MusicPreview.cpp 2012-12-14 12:14:34 +0000
283+++ dash/previews/MusicPreview.cpp 2012-12-20 19:09:20 +0000
284@@ -24,7 +24,6 @@
285 #include "unity-shared/PreviewStyle.h"
286 #include "unity-shared/CoverArt.h"
287 #include "unity-shared/IconTexture.h"
288-#include "unity-shared/StaticCairoText.h"
289 #include <UnityCore/MusicPreview.h>
290 #include <NuxCore/Logger.h>
291 #include <Nux/HLayout.h>
292@@ -33,7 +32,6 @@
293
294 #include "MusicPreview.h"
295 #include "ActionButton.h"
296-#include "PreviewInfoHintWidget.h"
297 #include "Tracks.h"
298
299 namespace unity
300@@ -48,9 +46,6 @@
301
302 MusicPreview::MusicPreview(dash::Preview::Ptr preview_model)
303 : Preview(preview_model)
304-, image_(nullptr)
305-, title_(nullptr)
306-, subtitle_(nullptr)
307 {
308 SetupBackground();
309 SetupViews();
310@@ -119,11 +114,6 @@
311 Preview::AddProperties(builder);
312 }
313
314-void MusicPreview::SetupBackground()
315-{
316- details_bg_layer_.reset(dash::previews::Style::Instance().GetBackgroundLayer());
317-}
318-
319 void MusicPreview::SetupViews()
320 {
321 dash::MusicPreview* music_preview_model = dynamic_cast<dash::MusicPreview*>(preview_model_.get());
322
323=== modified file 'dash/previews/MusicPreview.h'
324--- dash/previews/MusicPreview.h 2012-12-14 12:14:34 +0000
325+++ dash/previews/MusicPreview.h 2012-12-20 19:09:20 +0000
326@@ -24,15 +24,6 @@
327 #define MUSICPREVIEW_H
328
329 #include "Preview.h"
330-#include <Nux/Nux.h>
331-#include <Nux/StaticText.h>
332-
333-namespace nux
334-{
335-class AbstractPaintLayer;
336-class StaticCairoText;
337-class VLayout;
338-}
339
340 namespace unity
341 {
342@@ -40,8 +31,6 @@
343 {
344 namespace previews
345 {
346-class CoverArt;
347-class PreviewInfoHintWidget;
348 class Tracks;
349
350 class MusicPreview : public Preview
351@@ -62,23 +51,13 @@
352 virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw);
353 virtual void PreLayoutManagement();
354
355- void SetupBackground();
356- void SetupViews();
357+ virtual void SetupViews();
358
359 void OnPlayTrack(std::string const& uri);
360 void OnPauseTrack(std::string const& uri);
361
362 protected:
363- nux::VLayout* full_data_layout_;
364-
365- nux::ObjectPtr<CoverArt> image_;
366- nux::ObjectPtr<StaticCairoText> title_;
367- nux::ObjectPtr<StaticCairoText> subtitle_;
368 nux::ObjectPtr<Tracks> tracks_;
369- nux::ObjectPtr<PreviewInfoHintWidget> preview_info_hints_;
370-
371- typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
372- LayerPtr details_bg_layer_;
373 };
374
375 }
376
377=== modified file 'dash/previews/Preview.cpp'
378--- dash/previews/Preview.cpp 2012-11-08 09:12:24 +0000
379+++ dash/previews/Preview.cpp 2012-12-20 19:09:20 +0000
380@@ -33,6 +33,7 @@
381 #include "MusicPreview.h"
382 #include "MoviePreview.h"
383 #include "SocialPreview.h"
384+#include "PreviewInfoHintWidget.h"
385
386 namespace unity
387 {
388@@ -219,6 +220,10 @@
389 : View(NUX_TRACKER_LOCATION)
390 , preview_model_(preview_model)
391 , tab_iterator_(new TabIterator())
392+ , full_data_layout_(nullptr)
393+ , image_(nullptr)
394+ , title_(nullptr)
395+ , subtitle_(nullptr)
396 {
397 }
398
399@@ -363,6 +368,11 @@
400 nux::GetWindowCompositor().SetKeyFocusArea(default_focus);
401 }
402
403+void Preview::SetupBackground()
404+{
405+ details_bg_layer_.reset(dash::previews::Style::Instance().GetBackgroundLayer());
406+}
407+
408 }
409 }
410 }
411
412=== modified file 'dash/previews/Preview.h'
413--- dash/previews/Preview.h 2012-09-18 09:05:50 +0000
414+++ dash/previews/Preview.h 2012-12-20 19:09:20 +0000
415@@ -28,11 +28,16 @@
416 #include <UnityCore/Preview.h>
417 #include "unity-shared/Introspectable.h"
418 #include "unity-shared/PreviewStyle.h"
419+#include "unity-shared/StaticCairoText.h"
420+#include "PreviewInfoHintWidget.h"
421
422 namespace nux
423 {
424 class AbstractButton;
425+class AbstractPaintLayer;
426 class Layout;
427+class VLayout;
428+class StaticCairoText;
429 }
430
431 namespace unity
432@@ -45,6 +50,7 @@
433 {
434 class CoverArt;
435 class TabIterator;
436+class PreviewInfoHintWidget;
437
438 class Preview : public nux::View, public debug::Introspectable
439 {
440@@ -80,9 +86,12 @@
441
442 virtual bool AcceptKeyNavFocus() { return false; }
443
444+ virtual void SetupViews() {}
445+ void SetupBackground();
446+
447 nux::Layout* BuildGridActionsLayout(dash::Preview::ActionPtrList actions, std::list<nux::AbstractButton*>& buttons);
448 nux::Layout* BuildVerticalActionsLayout(dash::Preview::ActionPtrList actions, std::list<nux::AbstractButton*>& buttons);
449-
450+
451 void UpdateCoverArtImage(CoverArt* cover_art);
452
453 protected:
454@@ -90,6 +99,17 @@
455 std::list<nux::AbstractButton*> action_buttons_;
456 TabIterator* tab_iterator_;
457
458+ nux::VLayout* full_data_layout_;
459+
460+ nux::ObjectPtr<CoverArt> image_;
461+ nux::ObjectPtr<StaticCairoText> title_;
462+ nux::ObjectPtr<StaticCairoText> subtitle_;
463+ nux::ObjectPtr<StaticCairoText> description_;
464+ PreviewInfoHintWidget::Ptr preview_info_hints_;
465+
466+ typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
467+ LayerPtr details_bg_layer_;
468+
469 friend class PreviewContent;
470 };
471
472
473=== modified file 'dash/previews/SocialPreview.cpp'
474--- dash/previews/SocialPreview.cpp 2012-12-17 21:30:27 +0000
475+++ dash/previews/SocialPreview.cpp 2012-12-20 19:09:20 +0000
476@@ -24,7 +24,6 @@
477 #include "unity-shared/PreviewStyle.h"
478 #include "unity-shared/CoverArt.h"
479 #include "unity-shared/IconTexture.h"
480-#include "unity-shared/StaticCairoText.h"
481 #include "unity-shared/PlacesOverlayVScrollBar.h"
482 #include <UnityCore/SocialPreview.h>
483 #include <NuxCore/Logger.h>
484@@ -40,7 +39,6 @@
485 #include "SocialPreviewContent.h"
486 #include "SocialPreviewComments.h"
487 #include "ActionButton.h"
488-#include "PreviewInfoHintWidget.h"
489
490 namespace unity
491 {
492@@ -65,7 +63,6 @@
493
494 SocialPreview::SocialPreview(dash::Preview::Ptr preview_model)
495 : Preview(preview_model)
496-, full_data_layout_(nullptr)
497 {
498 SetupBackground();
499 SetupViews();
500@@ -134,11 +131,6 @@
501 Preview::AddProperties(builder);
502 }
503
504-void SocialPreview::SetupBackground()
505-{
506- details_bg_layer_.reset(dash::previews::Style::Instance().GetBackgroundLayer());
507-}
508-
509 void SocialPreview::SetupViews()
510 {
511 dash::SocialPreview* social_preview_model = dynamic_cast<dash::SocialPreview*>(preview_model_.get());
512
513=== modified file 'dash/previews/SocialPreview.h'
514--- dash/previews/SocialPreview.h 2012-12-14 12:14:34 +0000
515+++ dash/previews/SocialPreview.h 2012-12-20 19:09:20 +0000
516@@ -24,14 +24,6 @@
517 #define SOCIALPREVIEW_H
518
519 #include "Preview.h"
520-#include <Nux/Nux.h>
521-
522-namespace nux
523-{
524-class AbstractPaintLayer;
525-class VLayout;
526-class StaticCairoText;
527-}
528
529 namespace unity
530 {
531@@ -41,9 +33,7 @@
532 {
533 namespace previews
534 {
535-class CoverArt;
536 class PreviewLikesWidget;
537-class PreviewInfoHintWidget;
538 class SocialPreviewContent;
539 class SocialPreviewComments;
540
541@@ -65,25 +55,16 @@
542 virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw);
543 virtual void PreLayoutManagement();
544
545- void SetupBackground();
546- void SetupViews();
547+ virtual void SetupViews();
548
549 protected:
550- nux::VLayout* full_data_layout_;
551 nux::VLayout* sender_layout_;
552 nux::VLayout* title_layout_;
553
554 nux::ObjectPtr<IconTexture> avatar_;
555- nux::ObjectPtr<CoverArt> image_;
556 nux::ObjectPtr<SocialPreviewContent> content_;
557 nux::ObjectPtr<SocialPreviewComments> comments_;
558- nux::ObjectPtr<StaticCairoText> title_;
559- nux::ObjectPtr<StaticCairoText> subtitle_;
560 nux::ObjectPtr<StaticCairoText> comments_hint_;
561- nux::ObjectPtr<PreviewInfoHintWidget> preview_info_hints_;
562-
563- typedef std::unique_ptr<nux::AbstractPaintLayer> LayerPtr;
564- LayerPtr details_bg_layer_;
565 };
566
567 }