Skip to content

Commit

Permalink
resolves #1170 prevent table alignment from modifying margins of subs…
Browse files Browse the repository at this point in the history
…equent pages (PR #1172)
  • Loading branch information
mojavelinux authored Jul 25, 2019
1 parent 6ac300e commit 159c9f1
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For a detailed view of what has changed, refer to the {uri-repo}/commits/master[
* fix crash when toc is enabled and toc-title attribute is unset
* correctly map legacy Font Awesome icon names when icon set is not specified (#1157)
* coerce color values in theme that contain uppercase letters (#1149)
* prevent table alignment from modifying margins of subsequent pages; only align table if width is less than bounds (#1170)
* ensure base font color is set
* use more robust mechanism to detect an empty page; tare content stream after adding page background color or image
* ignore pdf-themesdir unless pdf-theme is specified (#1167)
Expand Down
19 changes: 18 additions & 1 deletion lib/asciidoctor-pdf/converter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1966,7 +1966,8 @@ def convert_table node

table_settings = {
header: table_header,
position: alignment,
# NOTE position is handled by this method
position: :left,
cell_style: {
# NOTE the border color and style of the outer frame is set later
border_color: table_grid_color,
Expand All @@ -1992,11 +1993,23 @@ def convert_table node

theme_margin :block, :top

left_padding = right_padding = nil
table table_data, table_settings do
# NOTE call width to capture resolved table width
table_width = width
caption_max_width = caption_max_width == 'fit-content' ? table_width : nil
@pdf.layout_table_caption node, alignment, caption_max_width if node.title? && caption_side == :top
# NOTE align using padding instead of bounding_box as prawn-table does
# using a bounding_box across pages mangles the margin box of subsequent pages
if alignment != :left && table_width != (this_bounds = @pdf.bounds).width
if alignment == :center
left_padding = right_padding = (this_bounds.width - width) * 0.5
this_bounds.add_left_padding left_padding
this_bounds.add_right_padding right_padding
else # :right
this_bounds.add_left_padding(left_padding = this_bounds.width - width)
end
end
if grid == 'none' && frame == 'none'
if table_header
rows(0).tap do |r|
Expand Down Expand Up @@ -2055,6 +2068,10 @@ def convert_table node
#end
end
end
if left_padding
bounds.subtract_left_padding left_padding
bounds.subtract_right_padding right_padding if right_padding
end
layout_table_caption node, alignment, caption_max_width, caption_side if node.title? && caption_side == :bottom
theme_margin :block, :bottom
end
Expand Down
98 changes: 98 additions & 0 deletions spec/table_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -521,5 +521,103 @@

(expect cell_right[:x]).to be > cell_left[:x]
end

it 'should allow position of table to be set using align attribute on table' do
pdf = to_pdf <<~'EOS', analyze: true
[cols=3*,width=50%]
|===
|LEFT |B1 |C1
|A2 |B2 |C2
|A3 |B3 |C3
|===
[cols=3*,width=50%,align=right]
|===
|RIGHT |B1 |C1
|A2 |B2 |C2
|A3 |B3 |C3
|===
EOS

cell_right_text = (pdf.find_text 'RIGHT')[0]
cell_left_text = (pdf.find_text 'LEFT')[0]

(expect cell_right_text[:x]).to be > cell_left_text[:x]
end

it 'should not mangle margin box on subsequent pages if table with alignment crosses page boundary' do
blank_line = %(\n\n)

pdf = to_pdf <<~EOS, analyze: true
#{(['filler'] * 25).join blank_line}
[%autowidth,align=right]
|===
|A | B
|A1
|B1
|A2
|B2
|A3
|B3
|===
#{(['filler'] * 22).join blank_line}
#{(['* list item'] * 6).join ?\n}
EOS

page_width = pdf.pages[0][:size][0]
a1_text = (pdf.find_text 'A1')[0]
a3_text = (pdf.find_text 'A3')[0]
(expect a1_text[:x]).to be > (page_width * 0.5)
(expect a1_text[:page_number]).to eql 1
(expect a3_text[:x]).to be > (page_width * 0.5)
(expect a3_text[:page_number]).to eql 2
first_list_item_text = (pdf.find_text string: 'list item', page_number: 2)[0]
last_list_item_text = (pdf.find_text string: 'list item', page_number: 3)[-1]
# NOTE if this is off, the margin box got mangled
(expect last_list_item_text[:x]).to eql first_list_item_text[:x]
end

it 'should set width of aligned table relative to bounds' do
pdf = to_pdf <<~EOS, analyze: true
[width=25%,align=right]
|===
|A | B
|A1
|B1
|A2
|B2
|===
====
****
[width=25%,align=right]
|===
|A | B
|A1
|B1
|A2
|B2
|===
****
====
EOS

page_width = pdf.pages[0][:size][0]
first_a1_text = (pdf.find_text 'A1')[0]
second_a1_text = (pdf.find_text 'A1')[1]
(expect first_a1_text[:x]).to be > (page_width * 0.5)
(expect second_a1_text[:x]).to be > (page_width * 0.5)
(expect first_a1_text[:x]).to be > second_a1_text[:x]
end
end
end

0 comments on commit 159c9f1

Please sign in to comment.