-
Notifications
You must be signed in to change notification settings - Fork 21
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
Added track width as optional props #34
Conversation
Thanks for this, I'll take a look shortly! |
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.
Looks good to me overall! Minor change requested, then I'll merge it and publish a new version
@@ -22,7 +22,8 @@ | |||
"postpublish": "PACKAGE_VERSION=$(echo 'const p = require(\"./package.json\"); process.stdout.write(p.version)' | node) && git tag v$PACKAGE_VERSION && git push --tags", | |||
"test": "eslint src/** && prettier --check src/**", | |||
"build": "rm -rf ./dist ./docs && prettier --write src/** docs-src/** && webpack && tsc", | |||
"watch-docs": "webpack serve" | |||
"watch-docs": "webpack serve", | |||
"prepare": "npm run build" |
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.
From reading the docs for prepare
, I see that it runs on install and prepublish. Since there's already a prepublishOnly
script that runs a build
, this prepare
will just add a build
at install time.
However, I don't think consumers of this library generally need to run build
at install time, since the built version should already be included (as part of the prepublishOnly
during publishing the new version). What do you think?
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.
prepare
is a nice addition, doesn't hurt anyone
Thanks for the contribution! |
Published as v0.3.0 at https://www.npmjs.com/package/react-circular-slider-svg |
Needed the ability to set the width of the circle, i.e. track width, via props.
Pls consider including this feature in the next version.