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

Non mv-list-item children (<tr>s) get replicated when the mv-list is a <table> #997

Open
karger opened this issue Nov 24, 2023 · 5 comments

Comments

@karger
Copy link
Collaborator

karger commented Nov 24, 2023

https://codepen.io/karger/pen/ZEwRzEd?editors=1100

@LeaVerou
Copy link
Member

Yeah, this is because the proper table structure assumes a <tbody>, and if you don't add that, the browser does. So by the time Mavo sees the structure of your HTML, it sees:

<div mv-app="appName" mv-mode="edit">
	Static trs are being replicated, as well as the "add item" buttons!
	<table mv-list="stuff" mv-initial-items="2">
		<tbody>
			<tr>
				<td>First</td>
				<td>Second</td>
			</tr>
			<tr mv-list-item>
				<td property="first"></td>
				<td property="second"></td>
			</tr>
			<tr>
				<td>First Footer</td>
				<td>Second Footer</td>
			</tr>
		</tbody>
	</table>
</div>

So it assumes the <tbody> is the item of the stuff list, and then you have an orphan mv-list-item. It tries to turn the latter into a list as well, by wrapping it with another <tbody>.

I'm tending to mark this as WONTFIX since there's no way to fix it — the original HTML structure cannot be recovered.

@karger
Copy link
Collaborator Author

karger commented Nov 24, 2023

But, the original mv-list-item is still present, now as a grandchild of the mv-list. So why not have mavo replicate the grandchild?

@LeaVerou
Copy link
Member

What is the general rule here? Any descendant mv-list-item?

@karger
Copy link
Collaborator Author

karger commented Nov 24, 2023

I would say any uninterrupted descendant---if there's an intervening mv-list then of course that should take precedence. Put another way mv-list-item should tie to the closest mv-list ancestor.

@LeaVerou
Copy link
Member

If creating lists with just mv-list or just mv-list-item becomes commonplace (rather than just error recovery), I don't think we can assume that was the author's intent.

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

No branches or pull requests

2 participants