Tabs: Fix style issues

[jzaefferer]

Powered by esformatter! Will do more of these with individual files.

[scottgonzalez]

I think we either need to clarify this in our style guide or change how this is formatted. The current rule is:

When a chain of method calls is too long to fit on one line, there must be one call per line, with the first call on a separate line from the object the methods are called on. If the method changes the context, an extra level of indentation must be used.

So, presumably this should be:

this.anchors = this.tabs
    .map( function() {
        return $( "a", this )[ 0 ];
    } )
        .attr( {
            ...
        } );

[scottgonzalez]

There are other instances of this below.

[jzaefferer]

Updated with a second commit. Addresses several cases where line breaks were missing. If these changes are correct, I can look into update my esformatter-jquery-chain plugin to address those line breaks as well.

[scottgonzalez]

I don't remember where we had the conversation, but I thought we decided that filtering as the first action was allowed on the first line.

/cc @mikesherov

[jzaefferer]

That's pretty random - both find and filter change context.

Can I merge the first commit and open a new PR for the other?

[scottgonzalez]

That's pretty random - both find and filter change context.

And .not() and .children() and lots of other methods. I thought this existing block was valid:

this.tabs.not( this.active ).attr( {
    "aria-selected": "false",
    "aria-expanded": "false",
    tabIndex: -1
} );

Or maybe:

this.tabs.not( this.active )
    .attr( {
        "aria-selected": "false",
        "aria-expanded": "false",
        tabIndex: -1
    } );

But I didn't expect it to be:

this.tabs
    .not( this.active )
        .attr( {
            "aria-selected": "false",
            "aria-expanded": "false",
            tabIndex: -1
        } );

Though I understand that's the rule as written.

[dmethvin]

The first looks best to me as a reader and is still very clear. I'm not sure how the style guide should balance human readability vs absolute rules that can be enforced by a formatter, but it seems sad to lose the former in search of the latter.

[jzaefferer]

My second commit was created by hand, no esformatter involved. Both esformatter and my esformatter-jquery-chain plugin tolerate line breaks as-is. The plugin only fixes indent.

[jzaefferer]

I merged my first commit, created a new PR for the other, #1508