Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

don't breaks the words in the table cells #727

Closed
wants to merge 1 commit into from

4 participants

@piccio

Table cell text unnecessarily wraps in the middle of a word.
With no_breaks_words option table cells wrap their content without breaking the words.

table = pdf.table(data, cell_style: { no_breaks_words: true })
@practicingruby

Just a heads up, this won't make the Prawn 1.1 release, but still is open for consideration for a future release. I need @hbrandl to review it though, as he's the defacto maintainer of the table code.

@hbrandl
Collaborator

I'm sorry that I've been unavailable for the last days / weeks.
I'll look through it tomorrow. I love the feature, but since you're touching very complicated code that is finally bug free I want to make sure we really cover all possible scenarios with the test cases.

@hbrandl
Collaborator

This will lead to lots of CannotFit errors if the content for the tables is dynamically generated. I do think that this is ok, but we would need to explicitly tell users to catch this error and adjust accordingly (bigger table, bigger columns or breaking words) in the documentation.

I'm missing one test case (sorry if I overlooked it).
A user specifies a column width that is too small for a word and the new option to not break words is set. This will raise a CannotFit Error (if I read the code correctly).

Once this is added I'll merge the pull request.

Also I'd love to document this feature before adding it to the code base. Any chance you'd update the manual too? (not a show stopper)

@hbrandl hbrandl self-assigned this
@practicingruby

@hbrandl: We'd only experience those errors when this particular option was set, right?

It'd also be nice to come up with something a bit more natural sounding than no_breaks_words, though I don't have an idea on the top of my head.

:+1: for documentation before merging.

@packetmonkey

How does naming the option "break_words" defaulting it to true for backwards compatibility sound?. That would make the call look like this

table = pdf.table(data, cell_style: { break_words: false })

In InDesign there is a similar feature for hyphenating words and you can uncheck the hyphenation on a chunk of text, but we don't actually inject a hyphen I believe. Otherwise {hyphenate: false} would read well.

@hbrandl
Collaborator

@sandal Yes.
If for example a user sets the column width and wants to ensure that the words don't break, this will lead to a CannotFit, or if multiple column widths are set and the remaining space for a longer than expected word won't fit, we're gonna have another CannotFit.
I do think this is ok, since the only alternative would be to ignore the no_breaks_words option if column_widths don't match. Which seems kind of unexpected to me.
And I wouldn't want to miss out on a good feature just because you have to be a little more careful when using it.

To ensure we don't get too many bug reports with this feature I do think it's vital to document it and document, that this error must be catched and resolved some how.

I don't like the wording of the feature too. break_words sounds a lot better. If someone has another idea, keep them coming.

@practicingruby

break_words: true as a default seems fine, then to get @piccio's behavior: break_words: false

@practicingruby

I'm closing this pull request because the gem extraction of Prawn::Table will happen soon, but when ready, please submit another request to https://github.com/prawnpdf/prawn-table

(or @hbrandl, feel free to move this over yourself)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 23, 2014
  1. optionally no breaks the words in the table cells

    Roberto Piccini authored
This page is out of date. Refresh to see the latest.
View
21 lib/prawn/table/cell/text.rb
@@ -16,8 +16,8 @@ class Cell
class Text < Cell
TextOptions = [:inline_format, :kerning, :size, :align, :valign,
- :rotate, :rotate_around, :leading, :single_line, :skip_encoding,
- :overflow, :min_font_size]
+ :rotate, :rotate_around, :leading, :single_line, :skip_encoding,
+ :overflow, :min_font_size, :no_breaks_words]
TextOptions.each do |option|
define_method("#{option}=") { |v| @text_options[option] = v }
@@ -76,11 +76,20 @@ def draw_content
end
def set_width_constraints
- # Sets a reasonable minimum width. If the cell has any content, make
- # sure we have enough width to be at least one character wide. This is
- # a bit of a hack, but it should work well enough.
+ # Sets a reasonable minimum width.
+ # If there is 'no_breaks_words' option then the minimum width is the size of the longer word in the cell's content,
+ # otherwise make sure we have enough width to be at least one character wide.
+ # This is a bit of a hack, but it should work well enough.
unless defined?(@min_width) && @min_width
- min_content_width = [natural_content_width, styled_width_of_single_character].min
+ if @text_options[:no_breaks_words].is_a?(TrueClass) and ! @content.empty?
+ string_widths = @content.split(/[\s-]+/).collect { |string| styled_width_of(string) }
+ min_string_width = string_widths.max
+ else
+ min_string_width = styled_width_of_single_character
+ end
+
+ min_content_width = [natural_content_width, min_string_width].min
+
@min_width = padding_left + padding_right + min_content_width
super
end
View
3  lib/prawn/text/formatted/box.rb
@@ -341,7 +341,8 @@ def valid_options
:document,
:direction,
:fallback_fonts,
- :draw_text_callback]
+ :draw_text_callback,
+ :no_breaks_words]
end
private
View
84 spec/table_spec.rb
@@ -1233,6 +1233,90 @@
end
+ describe "The columns of a table with cell_style's no_breaks_words option set" do
+
+ before(:each) do
+ @pdf = Prawn::Document.new
+ @data = [ ['foobar bar', 'foo foobar', 'foo bar foobar'] ]
+ @pad = 10
+ @fs = 10
+ @min_col0_width = @pdf.width_of(@data[0][0].split.max, size: @fs) + 2 * @pad
+ @min_col1_width = @pdf.width_of(@data[0][1].split.max, size: @fs) + 2 * @pad
+ @min_col2_width = @pdf.width_of(@data[0][2].split.max, size: @fs) + 2 * @pad
+ end
+
+ it "should have the width greater than the length of the largest word" do
+ table = @pdf.table(@data, cell_style: { no_breaks_words: true, size: @fs, padding: @pad })
+
+ table.width.should >= @min_col0_width + @min_col1_width + @min_col2_width
+ table.column(0).width.should >= @min_col0_width
+ table.column(1).width.should >= @min_col1_width
+ table.column(2).width.should >= @min_col2_width
+ end
+
+ it "should have the width equal to the length of the largest word if the table width's set equal to " +
+ "the sum of the min columns size" do
+ table_width = @min_col0_width + @min_col1_width + @min_col2_width
+
+ table = @pdf.table(@data, width: table_width,
+ cell_style: { no_breaks_words: true, size: @fs, padding: @pad })
+
+ table.width.should == @min_col0_width + @min_col1_width + @min_col2_width
+ table.column(0).width.should == @min_col0_width
+ table.column(1).width.should == @min_col1_width
+ table.column(2).width.should == @min_col2_width
+ end
+
+ it "should have the width greater than the length of the largest word if table_width's set to " +
+ "have enough space" do
+ table_width = 500
+
+ table = @pdf.table(@data, width: table_width, cell_style: { no_breaks_words: true, size: @fs, padding: @pad })
+
+ table.width.should >= @min_col0_width + @min_col1_width + @min_col2_width
+ table.column(0).width.should >= @min_col0_width
+ table.column(1).width.should >= @min_col1_width
+ table.column(2).width.should >= @min_col2_width
+ end
+
+ it "shouldn't fit its content if the table_width's not set to have enough space" do
+ table_width = @min_col0_width + @min_col1_width + @min_col2_width -1
+
+ lambda do
+ @pdf.table(@data, width: table_width,
+ cell_style: { no_breaks_words: true, size: @fs, padding: @pad })
+ end.should raise_error(Prawn::Errors::CannotFit)
+ end
+
+ it "should have the width greater than the length of the largest word if one column width's set to " +
+ "the table width minus the sum of the others min columns size" do
+ table_width = 500
+ col2_width = table_width - @min_col0_width - @min_col1_width
+
+ table = @pdf.table(@data, width: table_width, cell_style: { no_breaks_words: true, size: @fs, padding: @pad }) do
+ style column(2), width: col2_width
+ end
+
+ table.width.should >= @min_col1_width + @min_col1_width + col2_width
+ table.column(0).width.should >= @min_col0_width
+ table.column(1).width.should >= @min_col1_width
+ table.column(2).width.should >= col2_width
+ end
+
+ it "shouldn't fit its content if one column width's set greater than the table width minus the sum of the " +
+ "others min columns size" do
+ table_width = 500
+ col2_width = table_width - @min_col0_width - @min_col1_width + 1
+
+ lambda do
+ @pdf.table(@data, width: table_width, cell_style: { no_breaks_words: true, size: @fs, padding: @pad }) do
+ style column(2), width: col2_width
+ end
+ end.should raise_error(Prawn::Errors::CannotFit)
+ end
+
+ end
+
end
describe "colspan / rowspan" do
Something went wrong with that request. Please try again.