HTML table import: keep row order, invent no header cells - #275
Merged
Conversation
The importer read the header off the first row containing any th, wrote that row as the djot header row, and moved it to the top. Two defects followed. Rows were reordered. A table whose third row held a th came back with that row first, so the reading order of the content changed - not the markup, the content. A data cell was promoted. A lone mixed row (th + td) became a header row and both cells came back as th, so a screen reader announces a header that nobody wrote. A row is now the header row only when it is the first row and every one of its cells is a th. Every other row is written in place as a body row. djot has no per-cell header spelling: a header row is the row before the separator, and that is all a djot table can express. So a th outside a leading all-header row cannot survive, and the only choice is which loss to take. Losing the header marker loses an annotation the author wrote; promoting a data cell invents one they did not. Inventing is worse, and reordering is worse still, so both are gone. Rows inside thead still count as leading rows, and a table with no th anywhere still imports as body rows with no separator.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #275 +/- ##
============================================
- Coverage 92.42% 92.42% -0.01%
Complexity 3682 3682
============================================
Files 109 109
Lines 10440 10438 -2
============================================
- Hits 9649 9647 -2
Misses 791 791 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HtmlToDjotdecided a table's header by finding the first row containing any<th>, writing that row as the djot header row - which also moved it to the top and wrote every one of its cells as a header. Two defects fall out of that one rule.Rows were reordered. This is the serious one: it changes reading order, not markup.
imported on master as:
A data cell was promoted to a header.
imported as
| R | 1 |plus a separator row, so1came back as<th>- a column header nobody wrote, which is what a screen reader will announce.The rule now
A row is the header row only when it is the first row and all of its cells are
<th>. Every other row is written in place as an ordinary body row. Rows inside<thead>still count as leading rows.Why this loss and not the other
djot has no per-cell header spelling. A header row is the row immediately before the
|---|separator, and that is the whole of what a djot table can express - unlike Carve, whose fix for the same defect (markup-carve/carve-php#1274) could keep each cell's role. So a<th>outside a leading all-header row cannot survive the import, and the only question is which loss to take:Inventing is worse than losing, and reordering the reader's content is worse than both. The remaining limitation is stated in the converter docs.
Unchanged
A leading all-
<th>row, a<thead>/<tbody>split, and a table with no<th>at all all import exactly as before - each now with a test.