From fe9dc2713d4cc552bbe7ffe602bfcdbb054b95c7 Mon Sep 17 00:00:00 2001 From: Dan Allen Date: Tue, 3 May 2022 18:36:19 -0600 Subject: [PATCH] resolves #2134 use base-border-color as default border color; control appearance of border on border-width value alone * if border-color for element is not set or nil, fall back to value of base-border-color * don't set default value for base_border_color and base_border_width keys on theme object * change fallback value for table border color to base-border-color * change fallback value for thematic break stroke color to base-border-color * don't use base-border-width as fallback width for admonition column rule * interpret nil value in table border color array as transparent * interpret nil value in table border width array as 0 * don't set border colors in base theme so they can be controlled using base-border-color when extending theme * set border width on sidebar in base theme --- CHANGELOG.adoc | 2 ++ data/themes/base-theme.yml | 11 ++----- lib/asciidoctor/pdf/converter.rb | 53 ++++++++++++++++---------------- spec/admonition_spec.rb | 22 +++++-------- spec/converter_spec.rb | 16 ++++++++++ spec/example_spec.rb | 2 +- spec/running_content_spec.rb | 8 ++--- spec/sidebar_spec.rb | 2 +- 8 files changed, 59 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index db9213ca4..9fe53415f 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,8 @@ Enhancements:: * allow theme to configure which end the caption is placed for a block image (#2115) * add `Page#imported` method to mark page as imported (which suppresses running contennt) * add support for `smallcaps` text transform by replacing lowercase letters with small capital variants (#1192) +* use `base-border-color` as default border color; control appearance of border using `border-width` value alone (#2134) +* remove border colors in base theme so all border colors can be controlled using `base-border-color` when extending theme Bug Fixes:: diff --git a/data/themes/base-theme.yml b/data/themes/base-theme.yml index 8da5853bc..fdadb11ba 100644 --- a/data/themes/base-theme.yml +++ b/data/themes/base-theme.yml @@ -13,8 +13,7 @@ base_font_size: 12 base_font_size_min: 6 base_font_style: normal base_line_height: 1.15 -base_border_color: 'EEEEEE' -base_border_width: 0.5 +base_border_color: '000000' role_lead_font_size: 13.5 role_line-through_text_decoration: line-through role_underline_text_decoration: underline @@ -65,43 +64,37 @@ abstract_line_height: 1.4 abstract_padding: 0 abstract_title_align: center abstract_title_font_style: bold -admonition_column_rule_color: 'EEEEEE' admonition_column_rule_width: 0.5 admonition_padding: [4, 12, 4, 12] admonition_label_font_style: bold admonition_label_text_transform: uppercase -quote_border_color: 'EEEEEE' quote_border_left_width: 4 quote_padding: [3, 12, 3, 14] -verse_border_color: 'EEEEEE' verse_border_left_width: 4 verse_padding: [3, 12, 3, 14] code_font_family: Courier code_font_size: 10.8 code_line_height: 1.2 code_padding: 9 -code_border_color: 'EEEEEE' code_border_width: 0.5 conum_line_height: 1.15 conum_glyphs: circled example_background_color: 'FFFFFF' -example_border_color: 'EEEEEE' example_border_width: 0.5 example_padding: 12 image_align: left prose_margin_bottom: 12 sidebar_background_color: 'EEEEEE' +sidebar_border_width: 0.5 sidebar_padding: 12 sidebar_title_align: center sidebar_title_font_style: bold -table_border_color: '000000' table_border_style: solid table_border_width: 0.5 table_cell_padding: 2 table_head_font_style: bold table_head_border_bottom_width: 1.25 table_body_stripe_background_color: 'EEEEEE' -thematic_break_border_color: 'EEEEEE' thematic_break_border_style: solid thematic_break_border_width: 0.5 thematic_break_margin_bottom: 12 diff --git a/lib/asciidoctor/pdf/converter.rb b/lib/asciidoctor/pdf/converter.rb index ce253691a..9c1b8155e 100644 --- a/lib/asciidoctor/pdf/converter.rb +++ b/lib/asciidoctor/pdf/converter.rb @@ -463,8 +463,6 @@ def load_theme doc end def prepare_theme theme - theme.base_border_color ||= '000000' - theme.base_border_width ||= 0 theme.base_font_color ||= '000000' theme.base_font_size ||= 12 theme.base_font_style = theme.base_font_style&.to_sym || :normal @@ -481,8 +479,9 @@ def prepare_theme theme theme.list_item_spacing ||= 0 theme.description_list_term_spacing ||= 0 theme.description_list_description_indent ||= 0 + theme.table_border_color ||= (theme.base_border_color || '000000') theme.table_border_width ||= 0.5 - theme.thematic_break_border_color ||= '000000' + theme.thematic_break_border_color ||= (theme.base_border_color || '000000') theme.image_border_width ||= 0 theme.code_linenum_font_color ||= '999999' theme.callout_list_margin_top_after_code ||= 0 @@ -905,8 +904,8 @@ def convert_admonition node pad_box [0, cpad[1], 0, lpad[3]] do if extent label_height = extent.single_page_height || cursor - if (rule_color = @theme.admonition_column_rule_color) && - (rule_width = @theme.admonition_column_rule_width || @theme.base_border_width) > 0 + if (rule_width = @theme.admonition_column_rule_width || 0) > 0 && + (rule_color = @theme.admonition_column_rule_color || @theme.base_border_color) rule_style = @theme.admonition_column_rule_style&.to_sym || :solid float do extent.each_page do |first_page, last_page| @@ -1060,7 +1059,7 @@ def convert_quote_or_verse node category = node.context == :quote ? :quote : :verse # NOTE: b_width and b_left_width are mutually exclusive if (b_left_width = @theme[%(#{category}_border_left_width)]) && b_left_width > 0 - b_color = @theme[%(#{category}_border_color)] + b_color = @theme[%(#{category}_border_color)] || @theme.base_border_color else b_left_width = nil b_width = nil if (b_width = @theme[%(#{category}_border_width)]) == 0 @@ -1658,7 +1657,7 @@ def convert_image node, opts = {} end def draw_image_border top, w, h, alignment - if (Array @theme.image_border_width).any? {|it| it&.> 0 } && @theme.image_border_color + if (Array @theme.image_border_width).any? {|it| it&.> 0 } && (@theme.image_border_color || @theme.base_border_color) if (@theme.image_border_fit || 'content') == 'auto' bb_width = bounds.width elsif alignment == :center @@ -2194,12 +2193,12 @@ def convert_table node rect_side_names = [:top, :right, :bottom, :left] grid_axis_names = [:rows, :cols] - border_color = (rect_side_names.zip expand_rect_values theme.table_border_color, theme.base_border_color).to_h + border_color = (rect_side_names.zip expand_rect_values theme.table_border_color, 'transparent').to_h border_style = (rect_side_names.zip (expand_rect_values theme.table_border_style, :solid).map(&:to_sym)).to_h - border_width = (rect_side_names.zip expand_rect_values theme.table_border_width, theme.base_border_width).to_h - grid_color = (grid_axis_names.zip expand_grid_values (theme.table_grid_color || [border_color[:top], border_color[:left]]), theme.base_border_color).to_h + border_width = (rect_side_names.zip expand_rect_values theme.table_border_width, 0).to_h + grid_color = (grid_axis_names.zip expand_grid_values (theme.table_grid_color || [border_color[:top], border_color[:left]]), 'transparent').to_h grid_style = (grid_axis_names.zip (expand_grid_values (theme.table_grid_style || [border_style[:top], border_style[:left]]), :solid).map(&:to_sym)).to_h - grid_width = (grid_axis_names.zip expand_grid_values (theme.table_grid_width || [border_width[:top], border_width[:left]]), theme.base_border_width).to_h + grid_width = (grid_axis_names.zip expand_grid_values (theme.table_grid_width || [border_width[:top], border_width[:left]]), 0).to_h if table_header_size head_border_bottom_color = theme.table_head_border_bottom_color || grid_color[:rows] @@ -2355,7 +2354,9 @@ def convert_table node def convert_thematic_break node theme_margin :thematic_break, :top - stroke_horizontal_rule @theme.thematic_break_border_color, line_width: @theme.thematic_break_border_width, line_style: (@theme.thematic_break_border_style&.to_sym || :solid) + stroke_horizontal_rule @theme.thematic_break_border_color, + line_width: @theme.thematic_break_border_width, + line_style: (@theme.thematic_break_border_style&.to_sym || :solid) theme_margin :thematic_break, ((block_next = next_enclosed_block node) ? :bottom : :top), block_next || true end @@ -2997,7 +2998,7 @@ def ink_heading string, opts = {} }.merge(opts) end if h_category && @theme[%(#{h_category}_border_width)] && - @theme[%(#{h_category}_border_color)] && page_number == start_page_number + (@theme[%(#{h_category}_border_color)] || @theme.base_border_color) && page_number == start_page_number float do bounding_box [bounds.left, start_cursor], width: bounds.width, height: start_cursor - cursor do theme_fill_and_stroke_bounds h_category @@ -3457,7 +3458,7 @@ def ink_running_content periphery, doc, skip = [1, 1], body_start_page_number = theme_font periphery do canvas do bounding_box [trim_styles[:content_left][side], trim_styles[:top][side]], width: trim_styles[:content_width][side], height: trim_styles[:height] do - if (trim_column_rule_width = trim_styles[:column_rule_width]) > 0 + if trim_styles[:column_rule_color] && (trim_column_rule_width = trim_styles[:column_rule_width]) > 0 trim_column_rule_spacing = trim_styles[:column_rule_spacing] else trim_column_rule_width = nil @@ -3565,12 +3566,12 @@ def allocate_running_content_layout doc, page, periphery, cache # NOTE: we've already verified this property is set height: (trim_height = @theme[%(#{periphery}_height)]), bg_color: (resolve_theme_color %(#{periphery}_background_color).to_sym), - border_color: (trim_border_color = resolve_theme_color %(#{periphery}_border_color).to_sym), + border_width: (trim_border_width = @theme[%(#{periphery}_border_width)] || 0), + border_color: trim_border_width > 0 ? (resolve_theme_color %(#{periphery}_border_color).to_sym, @theme.base_border_color) : nil, border_style: (@theme[%(#{periphery}_border_style)]&.to_sym || :solid), - border_width: (trim_border_width = trim_border_color ? @theme[%(#{periphery}_border_width)] || @theme.base_border_width : 0), - column_rule_color: (trim_column_rule_color = resolve_theme_color %(#{periphery}_column_rule_color).to_sym), + column_rule_width: (trim_column_rule_width = @theme[%(#{periphery}_column_rule_width)] || 0), + column_rule_color: trim_column_rule_width > 0 ? (resolve_theme_color %(#{periphery}_column_rule_color).to_sym) : nil, column_rule_style: (@theme[%(#{periphery}_column_rule_style)]&.to_sym || :solid), - column_rule_width: (trim_column_rule_color ? @theme[%(#{periphery}_column_rule_width)] || 0 : 0), column_rule_spacing: (@theme[%(#{periphery}_column_rule_spacing)] || 0), valign: valign_offset ? [valign, valign_offset] : valign, img_valign: @theme[%(#{periphery}_image_vertical_align)], @@ -3861,7 +3862,7 @@ def apply_text_decoration styles, category, level = nil end def resolve_text_transform key, use_fallback = true - if (transform = ::Hash === key ? (key.delete :text_transform) : @theme[key.to_s]) + if (transform = ::Hash === key ? (key.delete :text_transform) : @theme[key]) transform == 'none' ? nil : transform elsif use_fallback @text_transform @@ -3870,9 +3871,9 @@ def resolve_text_transform key, use_fallback = true # QUESTION: should we pass a category as an argument? # QUESTION: should we make this a method on the theme ostruct? (e.g., @theme.resolve_color key, fallback) - def resolve_theme_color key, fallback_color = nil - if (color = @theme[key.to_s]) && color != 'transparent' - color + def resolve_theme_color key, fallback_color = nil, transparent_color = fallback_color + if (color = @theme[key]) + color == 'transparent' ? transparent_color : color else fallback_color end @@ -3883,7 +3884,7 @@ def resolve_font_kerning keyword end def theme_fill_and_stroke_bounds category, opts = {} - fill_and_stroke_bounds opts[:background_color], @theme[%(#{category}_border_color)], + fill_and_stroke_bounds opts[:background_color], @theme[%(#{category}_border_color)] || @theme.base_border_color, line_width: @theme[%(#{category}_border_width)], line_style: (@theme[%(#{category}_border_style)]&.to_sym || :solid), radius: @theme[%(#{category}_border_radius)] @@ -3909,11 +3910,9 @@ def theme_fill_and_stroke_block category, extent, opts = {} ink_caption node_with_caption, category: category if node_with_caption return end - if (b_color = @theme[%(#{category}_border_color)]) == 'transparent' - b_color = @page_bg_color - end + b_color = resolve_theme_color %(#{category}_border_color).to_sym, @theme.base_border_color, @page_bg_color b_radius ||= (@theme[%(#{category}_border_radius)] || 0) + (b_width || 0) - if b_width && b_color + if b_width if b_color == @page_bg_color # let page background cut into block background b_gap_color, b_shift = @page_bg_color, (b_width * 0.5) elsif (b_gap_color = bg_color) && b_gap_color != b_color diff --git a/spec/admonition_spec.rb b/spec/admonition_spec.rb index 50042452e..131f77029 100644 --- a/spec/admonition_spec.rb +++ b/spec/admonition_spec.rb @@ -1077,22 +1077,18 @@ end end - it 'should assign default width to column rule if key is not specified' do + it 'should not assign default width to column rule if key is not specified' do pdf_theme = { admonition_column_rule_color: '222222', admonition_column_rule_width: nil, + base_border_width: nil, } pdf = to_pdf <<~'EOS', pdf_theme: pdf_theme, analyze: :line TIP: You can use the theme to customize the color and width of the column rule. EOS lines = pdf.lines - (expect lines).to have_size 1 - column_rule = lines[0] - (expect column_rule[:from][:x]).to eql column_rule[:to][:x] - (expect column_rule[:color]).to eql '222222' - (expect column_rule[:width]).to eql 0.5 - (expect column_rule[:style]).to eql :solid + (expect lines).to be_empty end it 'should not fail if base border width is not set when using original theme' do @@ -1134,7 +1130,7 @@ (expect column_rule2[:from][:x] - column_rule1[:from][:x]).to eql 2.0 end - it 'should use base border width for column rule if column rule width is nil' do + it 'should not use base border width for column rule if column rule width is nil' do pdf_theme = { base_border_width: 2, admonition_column_rule_color: '222222', @@ -1145,11 +1141,7 @@ EOS lines = pdf.lines - (expect lines).to have_size 1 - column_rule = lines[0] - (expect column_rule[:from][:x]).to eql column_rule[:to][:x] - (expect column_rule[:color]).to eql '222222' - (expect column_rule[:width]).to eql 2 + (expect lines).to be_empty end it 'should allow theme to add border', visual: true do @@ -1233,8 +1225,8 @@ (expect (text_left - left).to_f).to eql 72.0 end - it 'should allow theme to disable column rule by setting color to nil' do - pdf_theme = { admonition_column_rule_color: nil } + it 'should allow theme to disable column rule by setting width to 0' do + pdf_theme = { admonition_column_rule_width: 0 } pdf = to_pdf <<~'EOS', pdf_theme: pdf_theme, analyze: :line TIP: You can use the theme to add a background color. EOS diff --git a/spec/converter_spec.rb b/spec/converter_spec.rb index 8ef0a563a..b45bd026e 100644 --- a/spec/converter_spec.rb +++ b/spec/converter_spec.rb @@ -348,6 +348,22 @@ (expect (pdf.page 1).text).to include 'Documentation Chronicles' end + it 'should allow all border colors to be set using base-border-color when extending base theme' do + [ + %(****\ncontent\n****), + %(====\ncontent\n====), + %([cols=2*]\n|===\n|a|b\n|c|d\n|===), + %(____\ncontent\n____), + %([verse]\n____\ncontent\n____), + %(----\ncontent\n----), + '---', + 'NOTE: content', + ].each do |input| + pdf = to_pdf input, pdf_theme: { extends: 'base', base_border_color: '0000EE' }, analyze: :line + (expect pdf.lines.map {|it| it[:color] }.uniq).to eql %w(0000EE) + end + end + it 'should convert background position to options' do converter = Asciidoctor::Converter.create 'pdf' { diff --git a/spec/example_spec.rb b/spec/example_spec.rb index 881c46303..0e077d54e 100644 --- a/spec/example_spec.rb +++ b/spec/example_spec.rb @@ -313,7 +313,7 @@ lines = (to_pdf input, attribute_overrides: { 'pdf-theme' => 'base' }, analyze: :line).lines (expect lines).to have_size 4 - (expect lines.map {|it| it[:color] }.uniq).to eql ['EEEEEE'] + (expect lines.map {|it| it[:color] }.uniq).to eql %w(000000) (expect lines.map {|it| it[:width] }.uniq).to eql [0.5] top, bottom = lines.map {|it| [it[:from][:y], it[:to][:y]] }.flatten.yield_self {|it| [it.max, it.min] } left = lines.map {|it| [it[:from][:x], it[:to][:x]] }.flatten.min diff --git a/spec/running_content_spec.rb b/spec/running_content_spec.rb index 77fef3344..e0502e8ee 100644 --- a/spec/running_content_spec.rb +++ b/spec/running_content_spec.rb @@ -1494,12 +1494,12 @@ (expect to_file).to visually_match 'running-content-border-style.pdf' end - it 'should use base border width and solid style if border width and style are not specified', visual: true do + it 'should use base border color and solid style if border width and style are not specified', visual: true do pdf_theme = { - base_border_width: 1, - footer_border_width: nil, + base_border_color: '000000', + footer_border_color: nil, + footer_border_width: 1, footer_border_style: nil, - footer_border_color: '000000', } to_file = to_pdf_file 'content', 'running-content-border-defaults.pdf', enable_footer: true, pdf_theme: pdf_theme diff --git a/spec/sidebar_spec.rb b/spec/sidebar_spec.rb index 72d767b6e..a64616b31 100644 --- a/spec/sidebar_spec.rb +++ b/spec/sidebar_spec.rb @@ -82,7 +82,7 @@ boundaries = (pdf.extract_graphic_states pdf.pages[0][:raw_content])[0] .select {|l| l.end_with? 'l' } .map {|l| l.split.yield_self {|it| { x: it[0].to_f, y: it[1].to_f } } } - (expect boundaries).to have_size 4 + (expect boundaries).to have_size 8 # border and background top, bottom = boundaries.map {|it| it[:y] }.yield_self {|it| [it.max, it.min] } left = boundaries.map {|it| it[:x] }.min text_top = (pdf.find_unique_text 'first').yield_self {|it| it[:y] + it[:font_size] }