Performance: Add topcoat framework to test suite #45
Conversation
| ], | ||
| button: { | ||
| generator: function( options ) { | ||
| // We need to check whether its a normal button or icon button, as markup and type differs |
There was a problem hiding this comment.
Blank line before comments
| var buttonClass = 'topcoat' + options.type + '-button'+ options.size; | ||
|
|
||
| //Icon Buttons donot support the cta button type | ||
| if( options.type !== '-icon' || options.state !== '--cta' ) { |
|
This whole PR uses the wrong indentation. We use tabs. |
|
|
||
| //Add icon only if it is a icon-button or there is an icon set | ||
| if( options.icon !== "" || options.type === "-icon" ) { | ||
| icon = "<span class='"; |
There was a problem hiding this comment.
This should be combined with line 29
|
Everything i commented on mobile also applies to desktop version |
|
Also please run |
|
All code styling is addressed! Thanks @geekman-rohit! |
|
@geekman-rohit this look good the only last thing is for future PR's take a look at http://contribute.jquery.org/commits-and-pull-requests/#commit-guidelines The first commit conforms and i'm going to squash these all to one commit anyway so ill just land this one. |
|
@arschmitz , I had been through it, but I was not careful enough with minor things like spaces. now I understand the readability the extra spaces is going to give the code. Will be more careful in future PRs. |
Added topcoat framework to the test suite
It is tested for following types of buttons