Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added support for Select fields as combo boxes #2000

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Added support for Select fields as combo boxes #2000

merged 3 commits into from
Dec 6, 2023

Conversation

VagnerNico
Copy link
Contributor

I've decoded two PDFs, one with the combo box and another blank and extracted the binary info necessary to render it and applied to the anchors.add_inputs function.

@VagnerNico
Copy link
Contributor Author

I don't know if overriding the select box in weasyprint.layout.block was the right call. Thanks in advance if you could review this PR and maybe merge it if everything is right 🙏

@liZe
Copy link
Member

liZe commented Nov 7, 2023

Hi!

Thank you for this pull request.

Some elements you insert in the PDF are hardcoded values (you probably found in your PDF), but they should be real data instead (or even not set at all): M, BC, Ff, etc. You can read the PDF specification to know what they mean and to find the required values.

Moreover, could you please provide a HTML sample showing the result? I can’t get an example that’s working for me.

@VagnerNico
Copy link
Contributor Author

VagnerNico commented Nov 8, 2023 •
edited
Loading

Hi!

Thank you for this pull request.

Some elements you insert in the PDF are hardcoded values (you probably found in your PDF), but they should be real data instead (or even not set at all): M, BC, Ff, etc. You can read the PDF specification to know what they mean and to find the required values.

Moreover, could you please provide a HTML sample showing the result? I can’t get an example that’s working for me.

Hi, thank you for reviewing and answering my PR 🙏
I've fixed the elements that you mention adding some explanation that points to the PDF specification.
About the sample I've ran the following code on a Python script and it worked fine:

from weasyprint import HTML

# Create a HTML object from a string with a multiple select field
html = HTML(string='''
  <html>
    <body>
      <label for="select-sample">Select Sample</label>
      <select name="sample">
        <option value="1">One</option>
        <option value="2">Two</option>
        <option value="3">Three</option>
        <option value="4">Four</option>
        <option value="5" selected>Five</option>
      </select>
      <label for="textarea-sample">Textarea Sample</label>
      <textarea name="textarea-sample"></textarea>
      <label for="input-sample">Input Sample</label>
      <input name="input-sample" value="Sample">
    </body>
  </html>''')

# Write the PDF
html.write_pdf('example.pdf', stylesheets=html._ua_stylesheets(True))

I'll attach the example.pdf with a single select and with a multiple one.
To change that and test in your machine just add the multiple parameter on the select tag.
Also, please use any PDF reader but DocumentViewer on Linux -- I don't know why the DocumentViewer can't render list boxes properly.
example_single_select.pdf
example_multiple_select.pdf

@VagnerNico
Copy link
Contributor Author

Actually, DocumentViewer can render a List Box, but the multiselect won't work on it.

@VagnerNico VagnerNico force-pushed the vn/add-support-to-combo-boxes branch 5 times, most recently from 21d25bd to dc037d1 Compare November 22, 2023 21:45
@VagnerNico
Copy link
Contributor Author

Hello @liZe, have you had time to check this PR? Is there anything that you want me to change?

@liZe
Copy link
Member

liZe commented Nov 27, 2023

Hello @liZe, have you had time to check this PR? Is there anything that you want me to change?

I’ll add a comment as soon as I find the time to review it! I’m currently working on other issues.

liZe
liZe reviewed Dec 6, 2023
View reviewed changes
weasyprint/layout/block.py Outdated
@@ -833,6 +833,29 @@ def block_container_layout(context, box, bottom_space, skip_stack,
if next_page['page'] is None:
next_page['page'] = new_box.page_values()[1]

if (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code should be CSS instead (in html5_ua.css). Inputs in browsers have been (and still are?) a mess to style because of awful non-CSS hacks, let’s not do like them! 😄

@liZe
Copy link
Member

liZe commented Dec 6, 2023

I’ll clean a couple of little things (including the styling hack) and let you review before merging. I’ll also manually test in different PDF readers.

(And don’t worry about tests failing with 3.7!)

VagnerNico and others added 3 commits December 6, 2023 16:54
@VagnerNico @liZe
Added support for Select fields as combo boxes
d4e8d4b 
I've decoded two PDFs, one with the combo box and another blank and extracted the binary info necessary to render it and applied to the `anchors.add_inputs` function.
@liZe
Use CSS instead of Python to set select boxes style
7a0bc53
@liZe
Clean and simplify code for select fields
2a7ced5
@liZe liZe force-pushed the vn/add-support-to-combo-boxes branch from dc037d1 to 2a7ced5 Compare December 6, 2023 20:58
@liZe
Copy link
Member

liZe commented Dec 6, 2023

@VagnerNico Feedback is welcome!

VagnerNico
VagnerNico commented Dec 6, 2023
View reviewed changes
Copy link
Contributor Author

@VagnerNico VagnerNico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to add; thanks for fixing my wrong assumptions.
Also, I've tested several HTML where I'm using the lib, and everything works fine.

@liZe liZe merged commit e9b06d1 into Kozea:main Dec 6, 2023
6 checks passed
@liZe liZe added the feature New feature that should be supported label Dec 6, 2023
@liZe liZe added this to the 61.0 milestone Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liZe liZe liZe left review comments

Assignees
No one assigned
Labels
feature New feature that should be supported
Projects
None yet
Milestone
61.0
Development

Successfully merging this pull request may close these issues.
2 participants
@VagnerNico @liZe