taffy icon indicating copy to clipboard operation
taffy copied to clipboard

Clarify or remove `Layout::order` field

Open oceantume opened this issue 1 year ago • 1 comments

What problem does this solve or what need does it fill?

The description of the order field from the Layout is a little confusing and can be interpreted as the node's order in the entire tree:

The relative ordering of the node

Nodes with a higher order should be rendered on top of those with a lower order. This is effectively a topological sort of each tree.

At first, from reading the description I thought this was a "global" ordering, where all nodes in a tree would get their own index so that you could just render all nodes without bothering about iterating over the hierarchy, but it's not the case. From looking at the code, I see that the value seems purely based on the node's order relative to its siblings, which is already available to a user iterating over the nodes hierarchy.

What solution would you like?

Make it clear that this is the same ordering that can be retrieved by calling Taffy::children().

What alternative(s) have you considered?

Alternatively, that field seems like it could be removed, because it's confusing (i.e. can I rely on the ordering from Taffy::children() or should I sort that result by layout order?) and isn't useful by itself since you can only use it to sort siblings with each other.

oceantume avatar Sep 12 '22 15:09 oceantume

I believe the order field in Layout will not always be the same as the ordering of the children. Specifically looking at the code (although I haven't looked into this deeply), it looks like absolutely positioned children are ordered separately from relatively (normally) positioned children.

Taffy doesn't currently support it, but CSS also has an order property (See: https://developer.mozilla.org/en-US/docs/Web/CSS/order) which can be used to override the "source order" (order in children).

I do agree that documentation could be better though.

nicoburns avatar Sep 13 '22 12:09 nicoburns