YGNodeStyleGetGap does not account for units of gap #1778

New Issue

2025-01-14T17:35:15-08:00

nicoburns commented

2025-01-14 17:35:15 -08:00

(Migrated from github.com)

Support for percentage gaps was recently added to Yoga. This uses a separate setter (presumably to avoid needing to add a units parameter to the existing setter), however it still has a single getting which returns a float value. This may either be a pixel value or a percentage and there is no way to tell which it is.

Suggested fix:

Change YGNodeStyleGetGap to return YGValue

Support for percentage gaps was recently added to Yoga. This uses a separate setter (presumably to avoid needing to add a units parameter to the existing setter), however it still has a single getting which returns a float value. This may either be a pixel value or a percentage and there is no way to tell which it is. Suggested fix: - Change `YGNodeStyleGetGap` to return `YGValue`

NickGerleman commented

2025-01-14 17:54:20 -08:00

(Migrated from github.com)

This was fixed by ae2d06d0f5

This was fixed by https://github.com/facebook/yoga/commit/ae2d06d0f5056c5f28ef9732b00b9ca34a6e6c6c

nicoburns commented

2025-01-15 01:21:10 -08:00

(Migrated from github.com)

Ah yes, so it was. I've been updating Yoga's rust bindings and I decided it was probably better to go with a versioned release so I don't have that commit yet.

Ah yes, so it was. I've been [updating Yoga's rust bindings](https://github.com/bschwind/yoga-rs/pull/58) and I decided it was probably better to go with a versioned release so I don't have that commit yet.

NickGerleman commented

2025-01-15 11:00:17 -08:00

(Migrated from github.com)

The mildly annoying part is that it's a breaking change, so I've been planning to make the next release Yoga 4.0 to account for that (it's also why I wanted to avoid it being part of last branch cut).

nicoburns commented

2025-01-15 13:48:41 -08:00

(Migrated from github.com)

Yeah, that is annoying. If you're doing a breaking release, then you may also wish to consider renaming YGGutter to YGGap (for consistency). The other things on my wishlist from an API perspective (with my Rust bindings hat on) would be setter methods that take a YGValue (or separate float and YGUnit parameters) with some convention around what to do if an invalid unit is used (we've been implementing this as a wrapper on top of the C API in the Rust bindings).

Yeah, that is annoying. If you're doing a breaking release, then you may also wish to consider renaming `YGGutter` to `YGGap` (for consistency). The other things on my wishlist from an API perspective (with my Rust bindings hat on) would be setter methods that take a `YGValue` (or separate `float` and `YGUnit` parameters) with some convention around what to do if an invalid unit is used (we've been implementing this as a wrapper on top of the C API in the Rust bindings).

NickGerleman commented

2025-01-16 09:41:47 -08:00

(Migrated from github.com)

That naming around YGGutter was kinda intentional to avoid ambiguity with having a "gap" parameter in style setters which means row/column/all, vs the gap amount. So spec language around "gutter" is used. https://www.w3.org/TR/css-align-3/#column-row-gap

That naming around `YGGutter` was kinda intentional to avoid ambiguity with having a "gap" parameter in style setters which means row/column/all, vs the gap amount. So spec language around "gutter" is used. https://www.w3.org/TR/css-align-3/#column-row-gap

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#1778

YGNodeStyleGetGap does not account for units of gap #1778

`YGNodeStyleGetGap` does not account for units of gap #1778