Remove hardcoded bold font for first line in abstract

[nawroth]

The feature could be re-introduced with theming support later on.
As it stands before this change, it limits the styling of abstract blocks too much.

Fixes #378.

[mojavelinux]

There are users that depend on this feature, so we can't remove it outright. The theme setting will need to be part of this change. I would prefer for the default theme to define the bold style for the first line as that is part of the design of the default theme, but I 100% agree it should be possible to control it.

[nawroth]

@mojavelinux I wasn't sure about the naming but went with abstract_firstline_font_style.
It didn't make sense for me to have first in the hierarchy.
Unless we want to have first_line and first_character in the future ... or something!

[mojavelinux]

That's close to the name I had in mind. How about first_line instead of firstline to be more consistent with CSS.

abstract:
  first_line_font_style: bold

We don't need it in the base theme, just the default theme. Since it could be absent, can you write it as the following?

prose_opts = { line_height: @theme.abstract_line_height }
if (first_line_font_style = @theme.abstract_first_line_font_style) && first_line_font_style != font_style
  prose_opts[:first_line_options] = { styles: [font_style, first_line_font_style] }
end

It didn't make sense for me to have first in the hierarchy.

It doesn't matter. That's just a coding styling. All the levels are flattened using an underscore. I tend to keep it flat until there are at least two keys.

To summarize:

• remove from base theme
• rename to first_line
• only pass :first_line_options if key is set in theme

Then we'll be good to merge!

[mojavelinux]

Unless we want to have first_line and first_character in the future ... or something!

That's definitely the long-term plan. As much of core CSS that we can replicate, the easier it is on the designer. But we'll take it a chunk at a time.

[nawroth]

@mojavelinux Pushed. Your code snippet is missing a to_sym !

More than extra options for the first line, I'd like to see borders supported for abstract blocks. Maybe it's just a bug around that?

[mojavelinux]

Using .to_sym in the assignment will throw an exception if this key is not present. Therefore, the .to_sym need to be moved to where first_line_font_style is referenced once the truthy check passes.

[nawroth]

Not having any idea how expensive to_map is, I'll go with a nested if.

[mojavelinux]

Thanks!

The general rule of thumb with Ruby is to avoid method calls if there's another way to do it without one. So the nested if is the right choice in this case (or just call .to_sym both times)!

[mojavelinux]

We have support for putting borders around blocks but, like with all things in Prawn, we have to explicitly add the logic. Borders are a bit tricky though because they can span pages, so the abstraction is still a bit weak atm. Similar to #342, but no dedicated issue exists.

[mojavelinux]

Thanks!