Skip to content
This repository was archived by the owner on Dec 11, 2021. It is now read-only.

Performance: Add topcoat framework to test suite #45

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions performance/frameworks/topcoat-mobile.js
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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing indentation?

"//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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this buttonText = options.type ? "" : " button",

var buttonClass = 'topcoat' + options.type + '-button'+ options.size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single quotes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


//Icon Buttons donot support the cta button type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing // Icon Buttons donot support the cta button type

if( options.type !== '-icon' || options.state !== '--cta' ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single quotes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing if (

buttonClass = buttonClass + options.state;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use +=

}
button = button + buttonClass + "'>";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use += instead of button = button +


//Add icon only if it is a icon-button or there is an icon set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing

if( options.icon !== "" || options.type === "-icon" ) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also just do options.icon and options.type because "" == false and "foo" == true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing also if (

icon = "<span class='";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be combined with line 29

iconClass="topcoat-icon";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You never define this above so your creating a global.

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing

if( options.type === "-icon" ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just check options.icon

buttonText = "";
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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>";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+= here as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing

}
return button + icon + buttonText + "</button>";
},
variations: {
type: [
"",
"-icon",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing comma

],
size: [
"",
"--large",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing comma

],
state: [
"",
"--cta",
"--quiet",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing comma


],
icon: [
"",
"location",
"home",
"alert"
]

}
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing comma

};
57 changes: 57 additions & 0 deletions performance/frameworks/topcoat.js
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"
]

}
},
};
8 changes: 6 additions & 2 deletions tasks/options/perfjankie.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,12 @@ module.exports = {
"http://localhost:4200/framework/jquery-ui-1-12/component/button/count/1000/" +
"jquery-ui-1-12:button",
"http://localhost:4200/framework/bootstrap/component/button/count/1000/" +
"bootstrap:button"
"bootstrap:button",
"http://localhost:4200/framework/topcoat/component/button/count/1000/" +
"topcoat:button",
"http://localhost:4200/framework/topcoat-mobile/component/button/count/1000/" +
"topcoat-mobile:button"
]
}
}
}
}