resolves asciidoctor#2134 use base-border-color as default border col…

…or; 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

@@ -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::
 

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

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

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

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'
         {

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

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

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] }