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

Vertical headers in table should render as th element in HTML backend #738

Closed
DavidGamba opened this issue Oct 25, 2013 · 10 comments
Closed
Assignees
Milestone

Comments

@DavidGamba
Copy link
Contributor

The h in cols="1h,..." modifier for the column indicates a vertical header, however, it adds the class="header" (in the wrong place) instead of actually modifying the html with the <th> element.

Initially the class="header" needs to be placed in the right place and eventually it should be replaced by the <th> element.

Test data

[cols="1h,3*"]
|===

|Header 1
|data
|data
|data

|Header 2
|data
|data
|data

|Header 3
|data
|data
|data

|===

HTML rendered

<table class="tableblock frame-all grid-all" style="width:100%; ">
<colgroup>
<col style="width:25%;">
<col style="width:25%;">
<col style="width:25%;">
<col style="width:25%;"> 
</colgroup>
<tbody>
<tr>
<td class="tableblock halign-left valign-top"><p class="tableblock header">Header 1</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
</tr>
<tr>
<td class="tableblock halign-left valign-top"><p class="tableblock header">Header 2</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
</tr>
<tr>
<td class="tableblock halign-left valign-top"><p class="tableblock header">Header 3</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">data</p></td>
</tr>
</tbody>
</table>

Focus on the line:

<td class="tableblock halign-left valign-top"><p class="tableblock header">Header 1</p></td>

Initially it should be:

<td class="tableblock halign-left valign-top header"><p class="tableblock">Header 1</p></td>

But ideally it should be:

<th class="tableblock halign-left valign-top"><p class="tableblock">Header 1</p></th>

I am using
Asciidoctor 0.1.4 [http://asciidoctor.org]

@mojavelinux
Copy link
Member

@DavidGamba Great suggestion. The choice to use a <td> element and add the "header" class on the <p> tag was made to be consistent with what Python AsciiDoc generates. We can, of course, modify the compact/asciidoc.conf file to use a <th> instead and also update Asciidoctor. It's definitely the right thing to do.

@ghost ghost assigned mojavelinux Oct 25, 2013
@mojavelinux
Copy link
Member

Here's the configuration needed in compact/asciidoc.conf:

# override header cells to use th
[tabletags-header]
bodydata=<th class="tableblock halign-{halign=left} valign-{valign=top}" {colspan@1::colspan="{colspan}" }{rowspan@1::rowspan="{rowspan}" }>|</th>
paragraph=<p class="tableblock">|</p>

Look for [tabletags-default] and add this section below that one.

@mojavelinux
Copy link
Member

If you'd like to send a pull request, I'll merge it in.

To modify Asciidoctor's output, look in the file lib/asciidoctor/backends/html5.rb. You'll want to add a condition to where it makes the <th> tag:

<<%= tsec == :head || cell.style == :header ? 'th' : 'td' %>

then you can remove the when :header clause in the case cell.style block.

@mojavelinux
Copy link
Member

If you could update the test as well, that would be great! Look for:

test 'styles not applied to header cells' do
  ...
end

@DavidGamba
Copy link
Contributor Author

Will give it a try... :)

@mojavelinux
Copy link
Member

👍

@DavidGamba
Copy link
Contributor Author

The only thing I didn't test were the changes to the compatibility template since I really didn't know how to.

mojavelinux added a commit that referenced this issue Oct 27, 2013
resolves #738 vertical table headers use th element instead of header class
mojavelinux added a commit that referenced this issue Oct 27, 2013
@ghost ghost assigned DavidGamba Oct 27, 2013
@mojavelinux
Copy link
Member

Thanks David!

FYI, to test the compatibility template, just create an AsciiDoc file in that directory and run asciidoc on it. AsciiDoc automatically reads the asciidoc.conf file in the same directory.

@mojavelinux
Copy link
Member

Btw, I'm working on getting the Cucumber-based test suite setup so that we can run the same tests against Asciidoctor & AsciiDoc. That way, the compatibility file will be tested by default.

@DavidGamba
Copy link
Contributor Author

Thanks Dan,

It was fun to help out a bit, I love using Asciidoctor.
I tested the asciidoc compatibility and all is good. The automatic testing of asciidoc will be great.

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

No branches or pull requests

2 participants