search

momijizukamori / bookbinder-js

A JS application to format PDFs for bookbinding.
Mozilla Public License 2.0
99 stars 26 forks source link

#57 adds marking for sewing #113

Closed MikDal002 closed 2 months ago

MikDal002 commented 2 months ago

I think it solves: #57

UI: image Example output: image

Works on folio image

quatro image

octavio image

Link to excalidraw image: https://excalidraw.com/#json=0MM-vkb65hcBYfuZSDCYA,Z5gNvdXsQkJbhEmimFlcyg

momijizukamori commented 2 months ago

I probably won't be able to get to a review until later this week, but thank you for taking a stab at this!

acestronautical commented 2 months ago

I think "french link" need not be mentioned. Kettle and french link use the same hole layout AFAIK, so we can probably just generically say "stitching holes" or "sewing marks" or something of that nature.

For the inputs we could maybe do "number of tapes" and "size of tapes" and assume that the spacing is always evenly distributed.

MikDal002 commented 2 months ago

I think "french link" need not be mentioned. Kettle and french link use the same hole layout AFAIK, so we can probably just generically say "stitching holes" or "sewing marks" or something of that nature.

For the inputs we could maybe do "number of tapes" and "size of tapes" and assume that the spacing is always evenly distributed.

I've changed the naming and adjusted images in PR's description

MikDal002 commented 2 months ago

I probably won't be able to get to a review until later this week, but thank you for taking a stab at this!

Please consider, if functionality to print only on most inner page should be introduced? Because actually, points are printed on every page :).

sithel commented 2 months ago

🤔 know why the Preview isn't working? a compile error? https://github.com/momijizukamori/bookbinder-js/actions/runs/8693335860/job/23858049531?pr=113#step:5:16 (I've not yet checked out branch, just sat down at computer and have forgotten how to check out branches 🤦‍♀️ )

sithel commented 2 months ago

Please consider, if functionality to print only on most inner page should be introduced? Because actually, points are printed on every page :).

🙌 I think if it was just inner, that'd be great! You should totally slap isSigMiddle in that object returned by Signatures.booklet! (maybe inner & outer? Or let user select either/both? Don't let me feature creep your PR, this is great and am thrilled that you added it! I like the UX you've gone with)

sithel commented 2 months ago

Ah! could you bump the version number? https://github.com/momijizukamori/bookbinder-js/blob/main/package.json#L3

MikDal002 commented 2 months ago

Please consider, if functionality to print only on most inner page should be introduced? Because actually, points are printed on every page :).

🙌 I think if it was just inner, that'd be great! You should totally slap isSigMiddle in that object returned by Signatures.booklet! (maybe inner & outer? Or let user select either/both? Don't let me feature creep your PR, this is great and am thrilled that you added it! I like the UX you've gone with)

I actually understand what you meant by feature creep :D. As I started adding new settings, then everything got a little messy. What to make it clearer, I would need to introduce new layout for this feature. If you think it is okay, then I can proceed :).

image

MikDal002 commented 2 months ago

I have enabled actions on my fork to speed up resolution of problems. https://github.com/MikDal002/bookbinder-js/pull/1 Requiring Linux-like line endings makes problems on a Windows machine :D.

momijizukamori commented 2 months ago

I have enabled actions on my fork to speed up resolution of problems. MikDal002#1 Requiring Linux-like line endings makes problems on a Windows machine :D.

I think you can configure git to auto-convert line endings - https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings

Sorry about the mess with the preview action - I think it's security-related, but I didn't write the action, I'm just using it :c

MikDal002 commented 2 months ago

I have enabled actions on my fork to speed up resolution of problems. MikDal002#1 Requiring Linux-like line endings makes problems on a Windows machine :D.

I think you can configure git to auto-convert line endings - https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings

Sorry about the mess with the preview action - I think it's security-related, but I didn't write the action, I'm just using it :c

No problem, I've read the docs, and noticed that it is not working for forks :D. I am using autoconvert, but it is converting to \r\n on Windows machine where I start linter (which breaks because of line endings), then it gets converted just to \n when commit :). Co it keeps it consisted in repository :).

MikDal002 commented 2 months ago

To make it clear, I've finished although you would like to see a proposal UI instead of that implemented :)

sithel commented 2 months ago

What you have is good! I think I feel good w/ the code, but want to run it/test it before I 👍 unfortunately I can NOT figure out how to check out this PR (because forked) and the preview isn't working 😩 it'll happen, this will work.. just... need a bit more time...

MikDal002 commented 2 months ago

What you have is good! I think I feel good w/ the code, but want to run it/test it before I 👍 unfortunately I can NOT figure out how to check out this PR (because forked) and the preview isn't working 😩 it'll happen, this will work.. just... need a bit more time...

Nice :) You can always check it on my branch here: https://mikdal002.github.io/bookbinder-js/pr-preview/pr-1/ and here https://github.com/MikDal002/bookbinder-js/pull/1. Notice SHAs of commits that they are the same here and in my PR :). Maybe it will help a little.

sithel commented 2 months ago

@MikDal002 - very excited to have this! Was there any follow-up/next work you had in mind?

I think I'm going to try and build atop this in the next couple days to get my spine order PR in & have a pulled out "PDF markup section" -- heads up in case of conflicting plans?

MikDal002 commented 2 months ago

@MikDal002 - very excited to have this!

Was there any follow-up/next work you had in mind?

I think I'm going to try and build atop this in the next couple days to get my spine order PR in & have a pulled out "PDF markup section" -- heads up in case of conflicting plans?

Actually those changes are sufficient for me, already sawed two books thanks to your app and those changes, and next awaits :-). I could work on the ui part I've presented above, but from my understanding you are taking this idea, then great 😊. So I don't have anything to add from my side, although I can help in this project somehow?