Introduce PrawnHtml::Instance class to preserve the context #43

blocknotes · 2022-06-08T06:51:25Z

Introduce a PrawnHtml::Instance to be able to preserve styles. Related to #41

Sample usage:

  pdf = Prawn::Document.new(page_size: 'A4')
  phtml = PrawnHtml::Instance.new(pdf)
  phtml.append(html: '<style>h1 { color: green }</style>')
  phtml.append(html: '<h1>Some HTML before</h1>')
  pdf.text('Some Prawn text') # call any Prawn method external to PrawnHtml
  phtml.append(html: '<h1>Some HTML after</h1>')

spec/units/prawn_html/instance_spec.rb

jrochkind · 2022-06-08T14:05:15Z

One of the use cases for this is in setting <style> tags that can keep applying... currently <style> tags require your HTML to have <html>, <head>, and <body> tags (not otherwise required, really), with the <style> tag in the <head>.

I'm not sure how that all comes together here.... are you expecting something like this?

pdf = Prawn::Document.new(page_size: 'A4')
phtml = PrawnHtml::Instance.new(pdf)

phtml.append(html: '<html><head><style> style: stuff; </style></head><body>')

phtml.append(html: '<h1>some html before</h1>')

# do something in the pdf

phtml.append(html: '<h1>some html after</h1>')

phtml.append(html: "</body></html>")

pdf.render_file('/tmp/test.pdf')

I'm not sure if that will actually work in your implementation? And it's not necessarily super convenient for the caller either?

At any rate, if the use case that motivates this is mine in #41, that requires you to somehow set some CSS, add some HTML, do some other things to the PDF, add some more html....

It would be good to see an example test case that actually does that, to see what the UX is like, and verify that the PR can satisfy the use case!

Thinking more, I wonder if allowing CSS style to be set in another way is actually the answer....

pdf = Prawn::Document.new(page_size: 'A4')
phtml = PrawnHtml::Instance.new(pdf)

phtml.set_css = <<~EOS
  body {   font-size: 40px;  }
  h1 { font-size: 80px; }
EOS

phtml.append(html: '<h1>some html before</h1>')
# do something in the pdf
phtml.append(html: '<p>some html after</p>')
pdf.render_file('/tmp/test.pdf')

I already found it kind of annoying to have to use teh otherwise unnecessary wrapper html/head/body tags to get the CSS in there... although I'm not sure if this particular example will work, will setting CSS on body work if I don't actually have a body? Maybe it could be made to?

Anyway, those are some thoughts, sorry for the stream of consciousness!

blocknotes · 2022-06-09T06:57:30Z

Thanks for the inputs, I think that having an instance could bring some nice extras 👍

At any rate, if the use case that motivates this is mine in #41, that requires you to somehow set some CSS, add some HTML, do some other things to the PDF, add some more html....

The goal of this PR is the same: preserving styles (at this time, I don't know if there could be something else useful to retain) in the instance.

Initially I thought it was enough having an instance, but yesterday I made different tests and found out that the styles are not preserved in the current implementation. So I added a new commit to fix this behaviour.

phtml.set_css = <<~EOS
body { font-size: 40px; }
h1 { font-size: 80px; }
EOS

...

I already found it kind of annoying to have to use teh otherwise unnecessary wrapper html/head/body tags to get the CSS in there... although I'm not sure if this particular example will work, will setting CSS on body work if I don't actually have a body? Maybe it could be made to?

set_css 🤔 yeah, it could be an idea. I'll think about it.

At the moment I like to keep the API as simple as possible, so in my current implementation an example would be:

  pdf = Prawn::Document.new(page_size: 'A4')
  phtml = PrawnHtml::Instance.new(pdf)
  phtml.append(html: '<style>h1 { color: green }</style>') # <= set styles
  phtml.append(html: '<h1>Some HTML before</h1>') # <= green title
  pdf.text('Some Prawn text') # <= call any Prawn method external to PrawnHtml
  phtml.append(html: '<h1>Some HTML after</h1>') # <= green title again

A test output is here: https://github.com/blocknotes/prawn-html/blob/prawn-html-instance/examples/instance.pdf

Styles are only set, so merging is not supported at this point (but it could be a nice point too).

Another option could be to add a different hash key for css, like:

phtml.append(css: 'h1 { color: green }')

🤔

jrochkind · 2022-06-09T11:57:00Z

Ah, ok, does it let you set CSS just with phtml.append(html: '<style>h1 { color: green }</style>')?

That would work then! In present prawn-html, I was unable to get that to work, the style tag needed to be wrapped in a head tag that was wrapped in a html tag, with the content in a body tag. Which was a bit of an annoyance, since those tags were otherwise not needed.

So this is an improvement, that will work! If I want to set styles that apply to the entire document like font-size: 20px... do I still need a <body> tag somehow, so I can set the style for body { font-size: 20px; }? If so, is there a way to supply a body tag with this new method of sending the html in separate segments?

blocknotes · 2022-06-09T20:44:56Z

So this is an improvement, that will work! If I want to set styles that apply to the entire document like font-size: 20px... do I still need a <body> tag somehow, so I can set the style for body { font-size: 20px; }? If so, is there a way to supply a body tag with this new method of sending the html in separate segments?

I think that this is getting out of scope for this PR and the standard scenario for me is using PrawnHtml.append_html method.
Also consider that you could achieve something similar with:

  phtml.append(html: '<style>p { font-size: 20px }</style>')
  phtml.append(html: '<p>The first part</p>')
  phtml.append(html: '<p>The <b>second</b> part</p>')
  phtml.append(html: '<p>The <i>third</i> part</p>')

jrochkind · 2022-06-09T21:12:34Z

Thanks, it's good to see explicitly that you can provide a <style> tag with this new method without it being inside a <head> tag! I think that's a change? A welcome one!

Here's my case for why I think it's in scope for the use case being addressed here.

Normally, prior to this PR, If I want to supply a <style> tag I am forced to supply html/head/body tags. And I can definitely change the font on body, which is a pretty normal CSS thing to do.

html_str = <<~EOS
<html>
<head>
  <style>
      body { font-size: 40px; }
  <style>
</head>
<body>
  <div>something</div>
  <p>something</p>
</body>
</html>
EOS

PrawnHtml.append_html(pdf, html_str)

Works great, cool.

The problem was -- okay, how do I do svg with prawn-svg? With <style> tags?

This PR provides a way to do that, where you can supply the HTML in seperate calls, with svg calls in between them, great!

But... there's no way to do do that while also doing the body CSS which worked fine before. Isn't that the scope?

But ok, nothing's perfect, that's fine.

blocknotes · 2022-06-10T20:09:38Z

The problem was -- okay, how do I do svg with prawn-svg? With <style> tags?
This PR provides a way to do that, where you can supply the HTML in seperate calls, with svg calls in between them, great!
But... there's no way to do do that while also doing the body CSS which worked fine before. Isn't that the scope?

I think that's a tricky task because prawn-svg (or any Prawn method) operates directly on the PDF, while my gem work on its own to parse the styles, the HTML tags and attributes to convert them at the end in Prawn's method calls.

So for now I think that this PR is fine and I'm merging it.
In any case it can be improved later on if something better come up.

blocknotes self-assigned this Jun 8, 2022

github-actions bot reviewed Jun 8, 2022

View reviewed changes

spec/units/prawn_html/instance_spec.rb Outdated Show resolved Hide resolved

Introduce PrawnHtml::Instance class to preserve the context

9a93376

blocknotes force-pushed the prawn-html-instance branch from 5e874a0 to 9a93376 Compare June 8, 2022 06:52

blocknotes mentioned this pull request Jun 8, 2022

svg images? #41

Closed

Preserve parsed styles in the instance

966b423

Support for append css

de3a069

blocknotes marked this pull request as ready for review June 10, 2022 20:03

blocknotes merged commit 014eb28 into main Jun 10, 2022

blocknotes deleted the prawn-html-instance branch June 10, 2022 20:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce PrawnHtml::Instance class to preserve the context #43

Introduce PrawnHtml::Instance class to preserve the context #43

blocknotes commented Jun 8, 2022 •

edited

Loading

jrochkind commented Jun 8, 2022 •

edited

Loading

blocknotes commented Jun 9, 2022 •

edited

Loading

jrochkind commented Jun 9, 2022

blocknotes commented Jun 9, 2022

jrochkind commented Jun 9, 2022 •

edited

Loading

blocknotes commented Jun 10, 2022

Introduce PrawnHtml::Instance class to preserve the context #43

Introduce PrawnHtml::Instance class to preserve the context #43

Conversation

blocknotes commented Jun 8, 2022 • edited Loading

jrochkind commented Jun 8, 2022 • edited Loading

blocknotes commented Jun 9, 2022 • edited Loading

jrochkind commented Jun 9, 2022

blocknotes commented Jun 9, 2022

jrochkind commented Jun 9, 2022 • edited Loading

blocknotes commented Jun 10, 2022

blocknotes commented Jun 8, 2022 •

edited

Loading

jrochkind commented Jun 8, 2022 •

edited

Loading

blocknotes commented Jun 9, 2022 •

edited

Loading

jrochkind commented Jun 9, 2022 •

edited

Loading