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

Change the table builder to permit looser intermediate table heads #77

Merged
merged 1 commit into from
Sep 3, 2020

Conversation

despresc
Copy link
Contributor

@despresc despresc commented Sep 1, 2020

Right now the table builder enforces the RowHeadColumns boundary in the intermediate head of a TableBody. That means that a TableBody is currently supposed to look like

+-----------------------+--------------+
|  Head above row head  | Rest of head |
+-----------------------+--------------+
|        Row head       |    Row body  |
+-----------------------+--------------+

so if there were a cell in the top left whose width exceeded the RowHeadColumns, it would be shrunk to fit. I think this is a leftover of the original design of the new Table that had no intermediate tables at all; in that case the model above is actually how tables normally behave. But tables with intermediate sub-tables frequently have headings that stretch across the entire table (an example, which is encoded with two tbody elements). So what's done now is probably too aggressive.

This change makes the TableBody model look like

+---------------------+
|  Intermediate head  |
+----------+----------+
| Row head | Row body |
+----------+----------+

The only behavioural change is that table will not enforce the RowHeadColumns boundary in intermediate heads in a TableBody. It would still do that in the intermediate body, to ensure that the row head has a consistent column width.

This would mean that writers could not confidently put a vertical rule down the entire TableBody, but I'm not sure that any of them want that guarantee, especially compared to the more common case of wanting a table-width intermediate heading.

The table builder (and the normalizeTableBody function) now permit
cells in the intermediate head of a TableBody to extend past the
RowHeadColumns. This allows for intermediate tables to have
subheadings that extend across the entire table.

Formerly the table builder would treat the intermediate head like the
intermediate body, and clip or drop cells that extended past the row
head.
@jgm
Copy link
Owner

jgm commented Sep 3, 2020

yes, this makes a lot of sense!

@jgm jgm merged commit 8e9ca37 into jgm:master Sep 3, 2020
@sergiocorreia
Copy link

Not sure if this is the best place, but I'm a bit confused about what is RowHeadColumns exactly.

The comment right above the definition is:

The number of columns taken up by the row head of each row of a -- 'TableBody'. The row body takes up the remaining columns.

Just to be sure:

  • Each table body then has an intermediate header and an intermediate body, which are just lists of rows.
  • Within the intermediate body, RowHeadColumns is a purely aesthetical attribute that indicates how many cells on the left should have a special formatting?
  • If someone wanted to add something similar to the cells on the right (e.g. a "Total" column) then they would need to modify the attributes of the table body itself?

Thanks!

@jgm
Copy link
Owner

jgm commented Nov 7, 2020

Within the intermediate body, RowHeadColumns is a purely aesthetical attribute that indicates how many cells on the left should have a special formatting?

I don't know about "purely aesthetical" -- in HTML, for example, it means that th would be used instead of td. But yes.

If someone wanted to add something similar to the cells on the right (e.g. a "Total" column) then they would need to modify the attributes of the table body itself?

Yes.

sergiocorreia added a commit to sergiocorreia/panflute that referenced this pull request Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants