Gap does not support a value of Auto

[nicoburns]

Problem

To match CSS, thegap property should not really have Auto or Undefined variants.

See: bevyengine/bevy#6743 (comment)

Proposed solution

A new LengthPercentage enum which does not include the Undefined or Auto variants

enum LengthPercentage {
    /// Abstract absolute units
    Points(f32),
    /// The dimension is stored in percentage relative to the parent item.
    Percent(f32),
}

Not 100% sure on the name, but LengthPercentage is what the CSS specification uses and is pretty descriptive.

Notes

This isn't really a problem in practice. We currently just treat Auto and Undefined as 0.0. But it is potentially confusing to users of taffy.

[alice-i-cecile]

Yep, I'm happy with LengthPercentage as a name.

[Weibye]

Shooting from the hip here:

// Dimension with a fixed length or known quantity
enum Dimension {
    Points(f32),
    Pixels(f32),
    Em(f32),
    Percent(f32)
}

// Dimension with a possible unknown quantity
enum UnDimension {
    Auto,
    Undefined,
    Points(f32),
    Pixels(f32),
    Em(f32),
    Percent(f32)
}

Alternatively

enum UnDimension {
    Auto,
    Undefined,
    Defined(Dimension)
}

Or another attempt at removing Dimension::Undefined for Option<Dimension>. It does not solve the Auto property though.

[nicoburns]

I think we might want to avoid Pixels/Em AND Points, as it is hard to know what the conversion between them would be. Overall I'm currently of the opinion that leaving Em out is for the best. Otherwise it requires Taffy to know about font sizes when we don't otherwise deal with text.

Regarding UnDimension, there are a lot more variants that we might well need: Content, MaxContent, MinContent, FitContent(f32), Calc(CalcExpr), etc. And which variants are valid for a given style property will not be consistent (e.g. grid track sizes have similar variants but also need to accept Fr units). As such a think a more specific enum like SizeDimension or SizeValue with a different enum for other properties may make sense.

[geom3trik]

Just out of curiosity, what is the Undefined variant for?

[nicoburns]

Just out of curiosity, what is the Undefined variant for?

In the case of gap it doesn't really serve a purpose. In the case of size, min_size and max_size it serves the "not set" use case. It perfectly valid for a node not to have any explicit sizes set, in which case it will fall back to being sized based on the size of it's content and/or flex sizing. Indeed it all nodes had explicit sizes then there wouldn't be much purpose to this crate.

[geom3trik]

Just out of curiosity, what is the Undefined variant for?

In the case of gap it doesn't really serve a purpose. In the case of size, min_size and max_size it serves the "not set" use case. It perfectly valid for a node not to have any explicit sizes set, in which case it will fall back to being sized based on the size of it's content and/or flex sizing. Indeed it all nodes had explicit sizes then there wouldn't be much purpose to this crate.

Sorry to take this off-topic, but is having an Undefined variant preferrable to using something like Option<Dimension> and using None in place of undefined?

[nicoburns]

Sorry to take this off-topic, but is having an Undefined variant preferrable to using something like Option and using None in place of undefined?

The answer to that is not obvious to me (advantage: it uses standard types. disadvantage: it requires wrapping everything in Some when defining styles). But that certainly seems like a valid opinion. There was a previously a PR #188 implementing that change, but it was never merged and it's implementation got so far behind the latest main that it was closed.

[Weibye]

Sorry to take this off-topic, but is having an Undefined variant preferrable to using something like Option<Dimension> and using None in place of undefined?

😅 I tried that (#188, #160, #148) but could never get it across the finish line due to subtle bugs that I was never able to come to the bottom of. The codebase in general looks much better now (❤️ @nicoburns), so maybe it is time for another attempt at it. Achieving that would make the codebase more idiomatic rust, but might move further away from being "css-like" which we may or may not want.