Code review comment for lp:~mandel/unity/fix-static-cairo-text

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

57 + text.SetSetterFunction([this](std::string newText)
58 + {
59 + this->_tooltip_text->SetText(newText);
60 + return true;
61 + }
62 + );c

You can't just return true; otherwise the property changed signal gets emitted every time you call set. It should only return true if the value of the text has changed, otherwise false.

48 + _tooltip_text = new nux::StaticCairoText(TEXT("Unity"), NUX_TRACKER_LOCATION);
271 + EXPECT_NE(new_tip, tooltip->_tooltip_text->GetText());

Don't know why tool-tips are being initialised to TEXT("Unity"). They should be blank by default.
And then you can change the test to:
271 + EXPECT_EQ("", tooltip->_tooltip_text->GetText());

Otherwise, LGTM.
Builds OK for me, runs and passes test.

review: Approve

« Back to merge proposal