-
Notifications
You must be signed in to change notification settings - Fork 67
Performance: Add topcoat framework to test suite #45
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
module.exports = { | ||
css: [ | ||
"//cdnjs.cloudflare.com/ajax/libs/topcoat/0.8.0/css/topcoat-mobile-dark.css", | ||
"//cdnjs.cloudflare.com/ajax/libs/topcoat-icons/0.2.0/font/icomatic.css" | ||
], | ||
button: { | ||
generator: function( options ) { | ||
var button = "<button class='"; | ||
var icon = ""; | ||
var buttonText = " Button"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this |
||
var buttonClass = 'topcoat' + options.type + '-button'+ options.size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. single quotes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
//Icon Buttons donot support the cta button type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
if( options.type !== '-icon' || options.state !== '--cta' ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Single quotes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
buttonClass = buttonClass + options.state; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
} | ||
button = button + buttonClass + "'>"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
|
||
//Add icon only if it is a icon-button or there is an icon set | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
if( options.icon !== "" || options.type === "-icon" ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to see all the icon logic grouped together like in my previous example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing also |
||
icon = "<span class='"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be combined with line 29 |
||
iconClass="topcoat-icon"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You never define this above so your creating a global. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
|
||
//If it is an icon button, add size to class and reset buttonText | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All these comments need spaces after the slashes. // If it is an icon button, add size to class and reset buttonText There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
if( options.type === "-icon" ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just check |
||
buttonText = ""; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line can be removed if you change the declaration above. |
||
iconClass = iconClass+options.size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You never define this above so your creating a global. |
||
} | ||
icon = icon + iconClass +" icomatic'>" + options.icon + "</span>"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
} | ||
return button + icon + buttonText + "</button>"; | ||
}, | ||
variations: { | ||
type: [ | ||
"", | ||
"-icon", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove trailing comma |
||
], | ||
size: [ | ||
"", | ||
"--large", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing comma |
||
], | ||
state: [ | ||
"", | ||
"--cta", | ||
"--quiet", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove trailing comma |
||
|
||
], | ||
icon: [ | ||
"", | ||
"location", | ||
"home", | ||
"alert" | ||
] | ||
|
||
} | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove trailing comma |
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
module.exports = { | ||
css: [ | ||
"//cdnjs.cloudflare.com/ajax/libs/topcoat/0.8.0/css/topcoat-desktop-dark.css", | ||
"//cdnjs.cloudflare.com/ajax/libs/topcoat-icons/0.2.0/font/icomatic.css" | ||
], | ||
button: { | ||
generator: function( options ) { | ||
var button = "<button class='"; | ||
var icon = ""; | ||
var buttonText = " Button"; | ||
var buttonClass = 'topcoat' + options.type + '-button'+ options.size; | ||
|
||
//Icon Buttons donot support the cta button type | ||
if( options.type !== '-icon' || options.state !== '--cta' ) { | ||
buttonClass = buttonClass + options.state; | ||
} | ||
button = button + buttonClass + "'>"; | ||
|
||
//Add icon only if it is a icon-button or there is an icon set | ||
if( options.icon !== "" || options.type === "-icon" ) { | ||
icon = "<span class='"; | ||
iconClass="topcoat-icon"; | ||
|
||
//If it is an icon button, add size to class and reset buttonText | ||
if( options.type === "-icon" ) { | ||
buttonText = ""; | ||
iconClass = iconClass+options.size; | ||
} | ||
icon = icon + iconClass +" icomatic'>" + options.icon + "</span>"; | ||
} | ||
return button + icon + buttonText + "</button>"; | ||
}, | ||
variations: { | ||
type: [ | ||
"", | ||
"-icon", | ||
], | ||
size: [ | ||
"", | ||
"--large", | ||
], | ||
state: [ | ||
"", | ||
"--cta", | ||
"--quiet", | ||
|
||
], | ||
icon: [ | ||
"", | ||
"location", | ||
"home", | ||
"alert" | ||
] | ||
|
||
} | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing indentation?