-
Notifications
You must be signed in to change notification settings - Fork 19
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
PPF-560 add pushbutton field, possibility to trasform it in image field #744
base: master
Are you sure you want to change the base?
Conversation
Hey, thanks for contributing. I read your last comment in the issue thread. While I appreciate your effort into making these changes, those reasons still do not validate the changes. Bit 17 of field flag does differentiate push button from checkbox and radio buttons. However that only makes a button widget a push button, not an image field. I have explained the key behavioral difference. Image fields, when clicked using a lot of PDF editors, will pop up and let you insert an image. A push button does not fall under that behavior. For example in the test you modified ( Finally, if you really need to fill an image onto a push button, you always have the option of simply drawing on it. |
After reading your comment I went another way: images can only be set for Image Fields, PushButtons are recognized as such and basically nothing can be done on them right now, except that they can be converted to Image Fields (and then an image is appliable). Would this approach be okay? |
Hey, sorry for a rather late response. I was on vacation last week and did not have my computer with me. Anyway, I checked out your changes in this PR and gave this snippet a try: from PyPDFForm import PdfWrapper
new_form = PdfWrapper("pdf_samples/scenario/existed/ds11_pdf.pdf")
new_form.pushbutton_field_to_image_field("Clear")
with open("temp/output.pdf", "wb+") as output:
output.write(new_form.read()) I downloaded the generated However, when I actually attempted to insert an image to For an actual image field, the image was successfully inserted. So I believe while On top of the above, there are couple other reasons why I'm not a big fan of these changes:
Again, much appreciated effort as I could tell you studied the project's code structure and made as much appropriate changes as you can. But I still cannot merge them for the above reasons. Let me know if you have more questions. |
All Submissions:
New Feature Submissions:
(I run pylint locally)
Changes to Core Features:
(I don't really know where to add an explanation for this, recognizing all pushbuttons as such instead of recognizing only a subset as image fields)
(I don't know if there's a need for new tests, I modified some of them to correctly run with these changes)
Basically this PR is based on my last comment on the issue , it removes the special image field and treats all pushbuttons equally, with the possibility to set an image to them