search

momijizukamori / bookbinder-js

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

Refactor some book.js logic to be cleaner #90

Closed acestronautical closed 4 months ago

acestronautical commented 4 months ago

There might be bugs in here, but wanted to put this up anyways since I think there are some good ideas.

Edit: it's been cleaned up and fixed now

github-actions[bot] commented 4 months ago

PR Preview Action v1.4.7 :---: Preview removed because the pull request was closed. 2024-02-13 06:17 UTC

momijizukamori commented 4 months ago

So I think there's some good stuff in here, but it looks like this is branched off a bad version of the base code (I'm seeing TypeError: this.inputpagelist is undefined when I try to run it), and I think there's going to be conflicts with the linting branch.

acestronautical commented 4 months ago

@momijizukamori I cleaned this up and rebased to most recent changes.

acestronautical commented 4 months ago

I'm not in a rush to merge this, I think the comments are valid and this might be more risk than benefit. I'll let this sit for a week or so while I think about it.

sithel commented 4 months ago

also, sorry for kinda' spazzing out on that earlier review. That was... a bit much-- I was stressing out about the presentation and surrounding life/weekend stuff and it leaked into the PR review I shouldn't have even been doing Really do appreciate all the improvements you've been chipping away at here