-
-
Notifications
You must be signed in to change notification settings - Fork 7
Add user follow button support. #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Since this would break the former config, it's better to publish with a higher version, which is |
src/plugin.js
Outdated
var size = size || "large"; | ||
var width = width || (size === "large" ? "150" : "100"); | ||
var height = height || (size === "large" ? "30" : "20"); | ||
var count = typeof count === "undefined" ? "true" : "false"; |
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.
We should treat count: true
setting as var count = true
.
Can you change this to following?
var count = typeof count === "boolean" ? String(count) : "false";
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.
This is moved from your code: https://github.com/azu/gitbook-plugin-github-buttons/blob/master/src/plugin.js#L58
and would be applied to both createButton
and createUserButton
. Should I?
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.
This is moved from your code:
Oh, sorry.
would be applied to both createButton and createUserButton. Should I?
Yes, Could you please do it?
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.
BTW, I think var count = typeof count === "boolean" ? count : false;
would be more directly.
One more thing, should we regard non boolean (including undefined
) values as false
or true
?
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.
One more thing, should we regard non boolean (including undefined) values as false or true?
https://ghbtns.com/github-btn.html?user=twbs&repo=bootstrap&type=watch
Default value seem to be false
.
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.
var count = typeof count === "boolean" ? count : false;
I agree. (also need to update README.md)
"buttons": [{
"user": "azu",
"repo": "JavaScript-Plugin-Architecture",
"type": "star",
"size": "large"
}, {
"user": "azu",
"type": "follow",
"width": "230",
- "count": "false"
+ "count": false
}]
b4d81ff
to
f746e82
Compare
Fixed and updated. |
@kxxoling Thanks! |
Preview:

And my settings: