launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing code

7	/**
8	- * Reset the widget to a blank state.
9	+ * Reset the widget to a blank state and fires
10	+ * 'CodeReviewComment.updated' event.
11	*

Wouldn't it be better to fire the event in insert_comment_HTML, between the appendChild and green_flash? The event name also doesn't make a whole lot of sense.

139	+ setter: function(navigator) {
140	+ var self = this;
141	+ var container = self.get('srcNode').one('.diff-navigator')
142	+ container.append(navigator)
143	+ navigator.on('change', function() {
144	+ self._showPreviewDiff(this.get('value'), false);
145	+ });

I don't think the self/this bit does what you intend. self is only useful inside the callback, but you use self outside and this inside.

248	+ if (!Y.Lang.isValue(navigator)) {
249	+ // DiffNav was not properly initialized.
250	+ return
251	+ 

Missing semicolon.


-- 
https://code.launchpad.net/~cprov/launchpad/ic-ui-stab-two/+merge/213950
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.