> You have some text conflicts in dash/DashView.cpp & UnityCore/Preview.cpp
> 
> 27      + * Copyright (C) 2011-2012 Canonical Ltd
> 
> 41      + * Authored by: Neil Jagdish Patel <email address hidden>
> 42      + *              Michal Hruby <email address hidden>
> 
> Please update all headers to include 2013 and your contact details
> 
Sorry, I yy those lines, fixed.

> 277      {
> 278     +
> 279        if (!preview_displaying_)
> 
> extra line.

No problem.

> 
> 407     +const std::string MusicPaymentPreview::DATA_INFOHINT_ID =
> "album_purchase_preview";
> 408     +const std::string MusicPaymentPreview::DATA_PASSWORD_KEY =
> "password";
> 409     +const std::string MusicPaymentPreview::CHANGE_PAYMENT_ACTION =
> "change_payment_method";
> 410     +const std::string MusicPaymentPreview::FORGOT_PASSWORD_ACTION =
> "forgot_password";
> 411     +const std::string MusicPaymentPreview::CANCEL_PURCHASE_ACTION =
> "cancel_purchase";
> 412     +const std::string MusicPaymentPreview::PURCHASE_ALBUM_ACTION =
> "purchase_album";
> 
> Is there a reason why these are scoped to MusicPaymentPreview class? Can you
> put them in global namespace?
> 

Is there a reason for that,I mean, it can be done but it will be nice to be able to access them for the tests.

> 936     +  std::map<std::string, nux::ObjectPtr<nux::AbstractButton>>
> sorted_buttons_;
> 
> What is sorted about a map ? ;)
> 
> 701     +  actions_layout->AddView(
> 702     +
> sorted_buttons_[MusicPaymentPreview::CHANGE_PAYMENT_ACTION].GetPointer(),
> 703     +          1, nux::MINOR_POSITION_START, nux::MINOR_SIZE_FULL,
> 704     +      100.0f, nux::NUX_LAYOUT_END);
> 705     +  actions_layout->AddView(
> 706     +
> sorted_buttons_[MusicPaymentPreview::FORGOT_PASSWORD_ACTION].GetPointer(),
> 707     +          1, nux::MINOR_POSITION_START, nux::MINOR_SIZE_FULL,
> 708     +      100.0f, nux::NUX_LAYOUT_END);
> 
> 728     +  buttons_data_layout->AddView(sorted_buttons_[MusicPaymentPreview::C
> ANCEL_PURCHASE_ACTION].GetPointer(),
> 729     +          1, nux::MINOR_POSITION_CENTER, nux::MINOR_SIZE_FULL,
> 100.0f,
> 730     +          nux::NUX_LAYOUT_END);
> 731     +  buttons_data_layout->AddView(sorted_buttons_[MusicPaymentPreview::P
> URCHASE_ALBUM_ACTION].GetPointer(),
> 732     +          1, nux::MINOR_POSITION_CENTER, nux::MINOR_SIZE_FULL,
> 100.0f,
> 733     +          nux::NUX_LAYOUT_END);
> 
> should check if sorted_buttons_ contains the actions before adding them.
>

No problem,
 
> 784     +  dash::Preview::InfoHintPtrList hints =
> preview_model_->GetInfoHints();
> 785     +  GVariant *preview_data = NULL;
> 786     +  dash::Preview::InfoHintPtr data_info_hint_ = NULL;
> 787     +  if (!hints.empty())
> 788     +  {
> 789     +    for (dash::Preview::InfoHintPtr info_hint : hints)
> 790     +    {
> 791     +      if (info_hint->id == MusicPaymentPreview::DATA_INFOHINT_ID)
> 792     +      {
> 793     +        preview_data = info_hint->value;
> 794     +      }
> 795     +      if (preview_data != NULL)
> 796     +      {
> 797     +        error_message_ = GetErrorMessage(preview_data);
> 798     +      }
> 799     +    }
> 800     +  }
> 
> 1. data_info_hint_ doesn't seem to be used.

Deleted

> 2. Don't need to check if the hints are empty if you're iterating over them.

True 

> 3. dash::Preview::InfoHintPtr const& info_hint. even though it's a pointer,
> you're still constructing the wrapper.

Well since is no needed this problem does not longer exist.

> 4. Not sure what this is doing, but i might be missing something. looping over
> the info hints. if it is a speicifc id, then set the preview data and get an
> error message; otherwise continue looping, but dont update the data, even
> though you are continuing to do a GetErrorMessage for it?
> 
> 
> 765     +  // set the tab ordering
> 766     +  SetFirstInTabOrder(password_entry_->text_entry());
> 767     +  SetLastInTabOrder(sorted_buttons_[MusicPaymentPreview::CANCEL_PURCH
> ASE_ACTION].GetPointer());
> 768     +  SetLastInTabOrder(sorted_buttons_[MusicPaymentPreview::PURCHASE_ALB
> UM_ACTION].GetPointer());
> 769     +  SetLastInTabOrder(sorted_buttons_[MusicPaymentPreview::CHANGE_PAYME
> NT_ACTION].GetPointer());
> 770     +  SetLastInTabOrder(sorted_buttons_[MusicPaymentPreview::FORGOT_PASSW
> ORD_ACTION].GetPointer());
> 
> Why are you setting the tab order in inside PreLayoutManagement? Shouldn't it
> be done only once when you create the views?
> 
> 1431    +  virtual void PreLayoutManagement() = 0;
> 

We don't want any other developer to be able to create an instance of the PaymentPreview.

> This is a virtual method in nux::Area I think. Why are you making it pure
> virtual in PaymentPreview?
> 
> 1735    +
> 1736    +
> 
> extra lines
> 
> 1778    +using namespace testing;
> 
> can you move this to after the includes please.

Easy fixes.