Improve headers method
#66
Conversation
benbarnett
force-pushed
the
nested-headers
branch
from
d09ceda to
7dd9b5a
Compare
| headers.flatten.compact | ||
| end | ||
|
|
||
| private |
There was a problem hiding this comment.
Our style is to indent this at the class level and drop the whitespace line beneath. See https://github.com/alphagov/styleguides/blob/master/ruby.md
There was a problem hiding this comment.
Addressed in guidance-guarantee-programme@84c74e6
|
LGTM. Any thoughts @fofr, @dsingleton? |
| @@ -3,11 +3,45 @@ module Govspeak | |||
|
|
|||
| class HeaderExtractor < Kramdown::Converter::Base | |||
| def convert(doc) | |||
| headers = [] | |||
|
|
|||
| doc.root.children.map do |el| | |||
There was a problem hiding this comment.
Because you're now explicitly pushing things into the headers array, each is more appropriate than map here. map is used to create an array of the items that the iterator block yields, whereas each just executes the block once per item.
benbarnett
force-pushed
the
nested-headers
branch
from
84c74e6 to
1d2582e
Compare
| doc.root.children.map do |el| | ||
| if el.type == :header | ||
| Header.new(el.options[:raw_text], el.options[:level], generate_id(el.options[:raw_text])) | ||
| headers.push(build_header(el)) |
There was a problem hiding this comment.
Personal preference maybe, but I'd prefer headers << build_header(el)
|
Couple of tiny comments, otherwise 👍
You are now 😄 |
benbarnett
force-pushed
the
nested-headers
branch
from
1d2582e to
32febdb
Compare
|
Thanks both, comments should all be addressed now! |
Fixes issues to do with nested headers not being found, and where explicitly assigned ID's were being ignored in favoured of the auto-generated versions.
benbarnett
force-pushed
the
nested-headers
branch
from
32febdb to
1135ada
Compare
|
@boffbowsh @benlovell either of you free to push the button? |
Fixes two issues to do with nested headers not being picked up, and where explicitly assigned ID's were being ignored in favoured of the auto-generated versions.
Please be kind with code review – I'm not a ruby developer 😇
Example:
headers, it was only returning the first heading with an incorrect ID (#headingas opposed to#explicit-id).<h2>Heading two</h2>was not being recognised by the govspeak method, even though it was in the rendered HTML.Follows the kramdown documentation for Specifying a Header ID and HTML Blocks.