search

pimcore / web-to-print-bundle

Web-to-Print community bundle adds web-to-print features to Pimcore documents.
https://pimcore.com/docs/platform/Web_To_Print/
Other
4 stars 12 forks source link

feat: allow any type of processor classes #50

Closed Cruiser13 closed 9 months ago

Cruiser13 commented 11 months ago

Allows any processor classes as discussed in #44

Tagged services and a service locator would be the better way but overpowered for this use case I think. Let me know your thoughts.

Thank you for considering the PR.

github-actions[bot] commented 11 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Cruiser13 commented 11 months ago

I have read the CLA Document and I hereby sign the CLA

Cruiser13 commented 10 months ago

@kingjia90 I see that a new release has been created without this PR. Are there any changes to be made?

kingjia90 commented 10 months ago

@Cruiser13 Unfortunately this is PR was not being reviewed yet and not in the main pipeline.

The changes looks fine so far, at least as quick fix, but maybe lacks a way to provide the settings by GUI eg. https://github.com/pimcore/web-to-print-bundle/blob/70b7474d17b7b792cd6f52e45100aab58b5a5f7f/public/js/settings.js#L342-L353

Cruiser13 commented 10 months ago

@Cruiser13 Unfortunately this is PR was not being reviewed yet and not in the main pipeline.

The changes looks fine so far, at least as quick fix, but maybe lacks a way to provide the settings by GUI eg.

https://github.com/pimcore/web-to-print-bundle/blob/70b7474d17b7b792cd6f52e45100aab58b5a5f7f/public/js/settings.js#L342-L353

Thanks for letting me know. Since we can not know which settings the custom processor has I don't see a good way to add them here. And you can override the JS in your custom processor anyway. We re-implemented the wkhtmltopdf processor with these changes and simply supplied the web2print options in the class. Works fine.

Cruiser13 commented 9 months ago

@kingjia90 what do you think?