mike-edel / ID-MultiPageImporter

Script for automating the placing (import) of PDF and InDesign files inside Adobe InDesign
119 stars 24 forks source link

Update MultiPageImporter.jsx #28

Closed noamsmadja closed 2 years ago

noamsmadja commented 2 years ago

Adding a "skip every X pages" in turn, allows to add the pdf pages to only even or odd pages

NO VALIDATION ON INPUT NOT TESTED WHEN ADDING NEW PAGES TO INDD

noamsmadja commented 2 years ago

Oh sorry,

To my understanding, this line deletes the temporary frame style applied during the script execution. I commented it so I would end up with a style for all the frames.

I find that keeping the temporary style is more beneficial than deleting it. If any styling is needed after script execution it is impossible to change all of the frames at once, unless we keep the style.

In addition, it is a good practice to style objects of the same nature with a style, even if it is not used or needed at first. Since all the frames come from the same PDF, giving them all the same style also makes sense :) you could code a style name to correspond with the file name to make it prettier, but a temp style name is a good option, user can then rename it to w hatever.

Would like to apologize if my code was less than organized. I didn’t think it will work at first and when it did I thought I could forward you the idea and you could implement it better than the mess I created!!

From: Mike @.> Sent: Sunday, August 28, 2022 11:16 PM To: mike-edel/ID-MultiPageImporter @.> Cc: Noam Smadja @.>; Author @.> Subject: Re: [mike-edel/ID-MultiPageImporter] Update MultiPageImporter.jsx (PR #28)

@mike-edel commented on this pull request.

Thanks for the feature! I will need some time to test this but much appreciated! Only saw one change that I don't understand yet (see comment in review).


In MultiPageImporter.jsxhttps://github.com/mike-edel/ID-MultiPageImporter/pull/28#discussion_r956776053:

}

// Kill the Object style

-tempObjStyle.remove();

+//tempObjStyle.remove();

I wonder why you removed this one? I don't really see the connection with the feature you implemented.

— Reply to this email directly, view it on GitHubhttps://github.com/mike-edel/ID-MultiPageImporter/pull/28#pullrequestreview-1087907911, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADV2347EV4NKYY2KDKDOKZLV3PCIBANCNFSM57IEFEZQ. You are receiving this because you authored the thread.Message ID: @.**@.>>

mike-edel commented 2 years ago

Oh sorry, To my understanding, this line deletes the temporary frame style applied during the script execution. I commented it so I would end up with a style for all the frames. I find that keeping the temporary style is more beneficial than deleting it. If any styling is needed after script execution it is impossible to change all of the frames at once, unless we keep the style. In addition, it is a good practice to style objects of the same nature with a style, even if it is not used or needed at first. Since all the frames come from the same PDF, giving them all the same style also makes sense :) you could code a style name to correspond with the file name to make it prettier, but a temp style name is a good option, user can then rename it to w hatever. Would like to apologize if my code was less than organized. I didn’t think it will work at first and when it did I thought I could forward you the idea and you could implement it better than the mess I created!!

No worries and good point about the object style actually. From a development perspective it would make sense to split those into 2 features/code commits to separate them. I would therefore first only merge the "skip every x pages" feature and add the object style separately.

On the object style there are a few options to implement this:

  1. the way you did (random name and keep it instead of deleting at the end)
  2. "assign object style" checkbox + name field; possibly use the pdf name if field is left empty
mike-edel commented 2 years ago

Sorry, messed the merge up a bit (shouldn't have done it this late really). Wanted to have it in a dedicated branch for testing it first before going to the main branch - hence the revert for now. Should be there now in https://github.com/mike-edel/ID-MultiPageImporter/tree/skip-every-x-pages Thanks again and I'll see how to best add the object style.

noamsmadja commented 2 years ago

Yes i agree! 2 commits would make more sense!

Maybe still need to think about the best wording for the skip field. Maybe make it more obvious that it skips pages in the indesign file, and not skipping pages in the pdf file.

Also, I have not implemented validation as i didn’t have the time to study the whole code and figure what problems could arise by user input. Might want to check user input is an integer? Typing zero or a negative number in that field will result in an infinite loop.

Implementing your second suggestion for the object style will be much better, of course! I resorted to just using the temporary one because I'm not familiar with indesign scripting and this was the quickest solution for my task. Thank you in advance if you do decide to spend the time on such an implementation!!

On 30 Aug 2022, at 22:44, Mike @.***> wrote:



Oh sorry, To my understanding, this line deletes the temporary frame style applied during the script execution. I commented it so I would end up with a style for all the frames. I find that keeping the temporary style is more beneficial than deleting it. If any styling is needed after script execution it is impossible to change all of the frames at once, unless we keep the style. In addition, it is a good practice to style objects of the same nature with a style, even if it is not used or needed at first. Since all the frames come from the same PDF, giving them all the same style also makes sense :) you could code a style name to correspond with the file name to make it prettier, but a temp style name is a good option, user can then rename it to w hatever. Would like to apologize if my code was less than organized. I didn’t think it will work at first and when it did I thought I could forward you the idea and you could implement it better than the mess I created!!

No worries and good point about the object style actually. From a development perspective it would make sense to split those into 2 features/code commits to separate them. I would therefore first only merge the "skip every x pages" feature and add the object style separately.

On the object style there are a few options to implement this:

  1. the way you did (random name and keep it instead of deleting at the end)
  2. "assign object style" checkbox + name field; possibly use the pdf name if field is left empty

— Reply to this email directly, view it on GitHubhttps://github.com/mike-edel/ID-MultiPageImporter/pull/28#issuecomment-1232090818, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADV23456HYDXB6XOINOLC5LV3ZQBJANCNFSM57IEFEZQ. You are receiving this because you authored the thread.Message ID: @.***>