Code review comment for lp:~zsombi/ubuntu-ui-toolkit/aplReplacePrimaryPage

Revision history for this message
Cris Dywan (kalikiana) wrote :

+ The property can only hold a Page instance. When changed runtime (not by the

...changed at runtime...

+ be cerated asynchronously and the instance will be reported through

...be created asynchronously...

+ if (source === null) {
+ if (source === undefined) {

These conditionals are hard to make sense of for me even after checking the two places the function can be called from. I'm thinking it might make more sense to dissolve the function so it's easier to understand given there is very little code to be shared.

+ function findPageFromLayoutWithProperty(apl, property, value) {

How about making this objectName only? That'd be more consistent and there's no chance of killing time by retroactively investigating localization-related failures because of labels.
Also, may be worth adding to UbuntuTestCase?

review: Needs Fixing

« Back to merge proposal