-
Notifications
You must be signed in to change notification settings - Fork 3
Consolidate calculator logic #10
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
fix input labels
Hi @agrahamg, |
I can certainly add semicolons in. If there are any other parts that are confusing or seem odd I can try to clarify them as well. You might be a novice at javascript but you seem to know a thing or two about sewing. I think the idea behind the site is great and hope you can continue making patterns. |
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.
Thank you for adding the line endings. I found a functional issue in getNumberFromField using parseInt. Added a comment.
Also a small readability thing for the roll top page, commented. The roll top page was the latest generator and pulled the seam allowance based on units radio selection. Earlier pattern generators had the text in html. My thought for future was to allow people to set a seam allowance, but suspected it would lead to confusion so I had not updated older pages.
Thanks again @agrahamg ! |
@agrahamg If you're interested in making one of my premium patterns, hit me up via dm or email and I'll shoot you a complimentary download credit. |
Moved common logic from pattern generators into util file to remove some of the boilerplate code.