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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support to return the html content #107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good change @paresharma 馃憤
I've got one suggestion about the processor specs if you can take a look
spec/grover/processor_spec.rb
Outdated
HTML | ||
end | ||
|
||
it { expect(convert.gsub(/[[:space:]]+/, '')).to eq url_or_html.gsub(/[[:space:]]+/, '') } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things here.. I'd suggest using Grover::Utils.squish
rather than removing all spaces. Would still give a consistent result whilst being true to the rendered output. Also, it feels like this would be a good opportunity to test that the rendered HTML is returned. ie something like:
context 'when rendering HTML' do
let(:method) { :content }
let(:url_or_html) do
<<-HTML
<html>
<head></head>
<body>
<h1>Hey there</h1>
<h2>Konnichiwa</h2>
<script>document.querySelector('h1').remove();</script>
</body>
</html>
HTML
end
it 'returns the rendered HTML' do
expect(Grover::Utils.squish(convert)).to eq Grover::Utils.squish(<<~HTML)
<html><head></head>
<body>
<h2>Konnichiwa</h2>
<script>document.querySelector('h1').remove();</script>
</body></html>
HTML
end
end
Ie not just what you gave it, but rather the result of it being rendered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch and point, thanks. Updated 馃檶
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one @paresharma
Great thanks. I would appreciate if you could release these changes 馃檶 |
Released in 0.14.2 |
Thank you 馃檶 |
This PR adds support to retrieve the HTML content. I just find it particularly helpful in debugging PDFs.
Let me know what you think about this 馃檶
FYI: puppeteer/puppeteer#419