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

Empty rows should not be rendered #1729

Open
ericscheid opened this issue Oct 1, 2021 · 8 comments
Open

Empty rows should not be rendered #1729

ericscheid opened this issue Oct 1, 2021 · 8 comments
Assignees
Labels
bug We say this works but it doesn't

Comments

@ericscheid
Copy link
Collaborator

Consider this table, where very long text data has been split over multiple lines for editing reasons, and all the cells of the row are rowspanned into the above row..

| Race     | Multi-class options                                         |
|:---------|:------------------------------------------------------------|
| Dwarf    | fighter/cleric, fighter/thief                               |
| Elf      | fighter/mage, fighter/thief, mage/thief,                    |
|         ^| fighter/mage/thief                                         ^|
| Gnome    | fighter/thief, fighter/cleric, fighter/illusionist,         | 
|         ^| thief/cleric, thief/illusionist, cleric/illusionist        ^|
| Half-elf | fighter/priest, fighter/mage, fighter/thief, ranger/priest, |
|         ^| mage/priest, mage/thief,                                   ^|
|         ^| fighter/mage/priest, fighter/mage/thief                    ^|
| Halfling | fighter/thief                                               |
| Human    | none                                                        |

Unfortunately, the markdown renders this with empty <tr></tr>...

image

This is a bug, and it messes up the even/odd row striping:

image

@ericscheid ericscheid added the bug We say this works but it doesn't label Oct 1, 2021
@calculuschild
Copy link
Member

Hmm interesting find. Good eye.

@G-Ambatte
Copy link
Collaborator

To further muddy the waters, if the <tr></tr> is removed completely, then the preceding rowspan(s) must also be decremented, or else the entire table format will break in new and interesting ways.

As I understand it, this is not actually the intended use of rowspan. I can see how it might be clearer in the editor and thus more convenient - but as per the current implementation, it doesn't actually work.

WORKAROUNDS:

  • If the user doesn't attempt to misuse rowspan in this manner and keeps the content for a single row in a single row, then it works fine.
    image
  • If the user ensures that they always span an odd number of rows (for the entire row), then it works fine.
    image

It's worth noting that, despite the different approaches, the two tables above appear to be visually identical.

@ericscheid
Copy link
Collaborator Author

Oh, good point on decrementing rowspans.

@dbolack-ab dbolack-ab self-assigned this Jan 17, 2024
@dbolack-ab
Copy link
Collaborator

Would the simplest fix be to strip empty pairs in html.renderer()?

Or must this be fixed in marked-extended-tables ?

@calculuschild
Copy link
Member

Fixes to table generation should go in marked-extended-tables

@dbolack-ab dbolack-ab removed their assignment Aug 23, 2024
@dbolack-ab dbolack-ab self-assigned this Jan 15, 2025
@dbolack-ab
Copy link
Collaborator

In addition to the two workarounds above, there is one breaking fix.

This is to wrap rows into sections using multiple <tbody> and adjust the CSS to nth-child on the <tbody> instead of the <tr>

@calculuschild
Copy link
Member

In addition to the two workarounds above, there is one breaking fix.

This is to wrap rows into sections using multiple <tbody> and adjust the CSS to nth-child on the <tbody> instead of the <tr>

Agreed, this would be breaking for most of our users. Let's not do this.

@dbolack-ab
Copy link
Collaborator

Demonstration of "fix"

calculuschild/marked-extended-tables#466 with #4002

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug We say this works but it doesn't
Projects
None yet
Development

No branches or pull requests

4 participants