Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix span widths #576

Closed
wants to merge 4 commits into from
Closed

Fix span widths #576

wants to merge 4 commits into from

Conversation

sigmike
Copy link
Contributor

@sigmike sigmike commented Nov 18, 2013

Following my comment on #407 (comment)

To make the spec pass I rewrote the way width is distributed on cells with colspan > 1.
Before that it allocated space evenly, which is a problem when other cells in a table already have allocated space.

For example this table:

| aaa | b |
| c       |

If "aaa" has a width of 150, "b" and "c" a width of 50, and the column widths are set to 150 and 50, then there's room for everything.
Without the patch it would allocate 150 to the first column, and 100 to the second one (total width 200 / colspan 2).

With the patch it allocates widths in 2 passes:

  • first only allocate width for cells with colspan == 1
  • then for cells with colspan > 1: distribute only the width that does not fit in the already allocate widths

I made the same change for height but it probably needs a spec.

I also had to change the way extra space in table is distributed.

@ghost ghost assigned practicingruby Nov 18, 2013
@practicingruby
Copy link
Member

Hi, can you add a comment here with a realistic use case for me to confirm the problem with? A table with all empty cells seems like a pathological case to me.

@sigmike
Copy link
Contributor Author

sigmike commented Nov 18, 2013

Here's the code to illustrate the example given above:

table([
  ["a", "b"],
  [{:content => "c", :colspan => 2}],
], column_widths: [150, 50])

move_down 10

table([
  ["a", "b"],
  ["c", "d"],
], column_widths: [150, 50])

The 2 tables will have different widths without the patch.

@practicingruby
Copy link
Member

@sigmike: that gives me what I need to review further, thanks. Right now there is a massive backlog of pull requests that need to be looked at, but since this addresses a defect hopefully it will be processed soon.

(f * (max - nat)) + nat
widths = (0...column_length).map { |c| natural_column_widths[c] }

loop do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this loop will never exit if all columns have reached their max width.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bummer. Please revise and then we'll re-open the pull request when you are ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants