> I can't claim it's better but I thought you might like to see another
> way to use call chaining in showError:
>
> this.fieldNode
> .append(Y.Node.create('<div />')
> .addClass('message')
> .set("text", error))
> .ancestor('tr')
> .addClass('error');
>
> To my eyes the above makes the structure of what's happening a little
> clearer; not that it was particularly obscure to start with. The
> structural subscription JS uses this pattern to good effect.
I agree, it's more compact this way.
> Line 233 of the diff is indented by one space too many. Oh, and you
> don't really need the "check" variable.
>
> check_and_submit: function() {
> if (this.validate()) {
> this.submit();
> }
> }
Fixed.
> The join in testValidateFails surprised me. Is there a reason string
> concatenation won't work there?
>
> Assert.areEqual(
> "The distroseries has no architectures selected to "
> + "build architecture independent binaries.",
> this.widget.fieldNode.one('.message').get('text'));
> I can't claim it's better but I thought you might like to see another Y.Node. create( '<div />') 'message' )
> way to use call chaining in showError:
>
> this.fieldNode
> .append(
> .addClass(
> .set("text", error))
> .ancestor('tr')
> .addClass('error');
>
> To my eyes the above makes the structure of what's happening a little
> clearer; not that it was particularly obscure to start with. The
> structural subscription JS uses this pattern to good effect.
I agree, it's more compact this way.
> Line 233 of the diff is indented by one space too many. Oh, and you
> don't really need the "check" variable.
>
> check_and_submit: function() {
> if (this.validate()) {
> this.submit();
> }
> }
Fixed.
> The join in testValidateFails surprised me. Is there a reason string fieldNode. one('.message' ).get(' text')) ;
> concatenation won't work there?
>
> Assert.areEqual(
> "The distroseries has no architectures selected to "
> + "build architecture independent binaries.",
> this.widget.
Force of habit I guess ;) I've changed that.
Thanks for the review.