Percent values not settable through YogaKit #371

New Issue

2017-02-01T22:55:05-08:00

jconst commented

2017-02-01 22:55:05 -08:00

(Migrated from github.com)

Hi! I looked through #258 and it looks like SetXPercent functions were added to the C API, and the iOS bindings were discussed but never implemented? Is anyone working on this right now? If not, I could take a stab and open a PR, but there are a lot of possible ways to write the API so I'd want to make sure I went down the right path. (Copy the C API exactly, create new properties, go for a breaking change and replace the CGFloat properties with some sort of unit type, etc.)

emilsjolander commented

2017-02-02 05:05:45 -08:00

(Migrated from github.com)

@dshahidehpour Have you thought about how we want to add this to the API?

d16r commented

2017-02-02 07:00:48 -08:00

(Migrated from github.com)

@emilsjolander I haven't, actually.

@jconst if you have the time, and you're interested, take a stab at it! I'd be happy to review. Shouldn't be too much plumbing.

@emilsjolander I haven't, actually. @jconst if you have the time, and you're interested, take a stab at it! I'd be happy to review. Shouldn't be too much plumbing.

d16r commented

2017-02-02 17:20:15 -08:00

(Migrated from github.com)

@hartbit Have any ideas about the API ideas for this?

I would down with going down the line of view.yoga.widthPercentage = .4,but I'm not sure what value should be returned when you access view.yoga.width after setting a percentage.

@hartbit Have any ideas about the API ideas for this? I would down with going down the line of `view.yoga.widthPercentage = .4`,but I'm not sure what value should be returned when you access `view.yoga.width` after setting a percentage.

hartbit commented

2017-02-02 23:14:51 -08:00

(Migrated from github.com)

I've know for a while what I want for Swift:

postfix operator %

postfix func %(value: Int) -> YGValue {
  return YGValue(value: Float(value), unit: .percent)
}

postfix func %(value: Float) -> YGValue {
  return YGValue(value: value, unit: .percent)
}

extension YGValue : ExpressibleByIntegerLiteral, ExpressibleByFloatLiteral {
  init(integerLiteral value: Int) {
    self.value = Float(value)
    unit = .pixel
  }
  
  init(floatLiteral value: Float) {
    self.value = value
    unit = .pixel
  }
}

view.yoga.width = 10
view.yoga.width = 5.6
view.yoga.width = 40%

For Objective-C, I think view.yoga.widthPercentage = .4 is the way forward. And I think view.yoga.width after setting a percentage should return YGUndefined.

I've know for a while what I want for Swift: ``` postfix operator % postfix func %(value: Int) -> YGValue { return YGValue(value: Float(value), unit: .percent) } postfix func %(value: Float) -> YGValue { return YGValue(value: value, unit: .percent) } extension YGValue : ExpressibleByIntegerLiteral, ExpressibleByFloatLiteral { init(integerLiteral value: Int) { self.value = Float(value) unit = .pixel } init(floatLiteral value: Float) { self.value = value unit = .pixel } } view.yoga.width = 10 view.yoga.width = 5.6 view.yoga.width = 40% ``` For Objective-C, I think `view.yoga.widthPercentage = .4` is the way forward. And I think `view.yoga.width` after setting a percentage should return `YGUndefined`.

hartbit commented

2017-02-02 23:17:35 -08:00

(Migrated from github.com)

@jconst I don't mind writing the PR btw if you want :)

emilsjolander commented

2017-02-03 02:46:21 -08:00

(Migrated from github.com)

@hartbit Could you explore using YGValue directly? This would make the getter return value more meaningful and reduce the number of properties needed. I like the syntactic sugar suggested for swift!

@hartbit Could you explore using `YGValue` directly? This would make the getter return value more meaningful and reduce the number of properties needed. I like the syntactic sugar suggested for swift!

hartbit commented

2017-02-03 03:14:06 -08:00

(Migrated from github.com)

@emilsjolander Here are a few ideas off the top of my head, from most to least verbose & idiomatic:

view.yoga.width = YGValueMake(10, YGUnitPixel);
view.yoga.width = YGPixelMake(10);
view.yoga.width = YGPixel(10);
view.yoga.width = pixel(10);
view.yoga.width = px(10);

Two points:

I'm not sure if the goal of reducing the number of properties is worth loosing the simplicity of view.yoga.width = 10.
I've always felt weird using the terminology pixel instead of point in a world where different platforms are moving rapidly away from pixel measurements and more towards dynamic DPI (especially on iOS). But I don't know if we should try to hide the term pixel on iOS only or also modify the C implementation.

@emilsjolander Here are a few ideas off the top of my head, from most to least verbose & idiomatic: ``` view.yoga.width = YGValueMake(10, YGUnitPixel); view.yoga.width = YGPixelMake(10); view.yoga.width = YGPixel(10); view.yoga.width = pixel(10); view.yoga.width = px(10); ``` Two points: 1. I'm not sure if the goal of reducing the number of properties is worth loosing the simplicity of `view.yoga.width = 10`. 2. I've always felt weird using the terminology `pixel` instead of `point` in a world where different platforms are moving rapidly away from pixel measurements and more towards dynamic DPI (especially on iOS). But I don't know if we should try to hide the term pixel on iOS only or also modify the C implementation.

emilsjolander commented

2017-02-03 03:20:04 -08:00

(Migrated from github.com)

Regarding pixel vs point. I'm fine with changing the YGUnit name to be point instead of pixel. That should be a separate PR though.

I agree that loosing view.yoga.width = 10 would be sad. However if it was as easy as view.yoga.width = pt(10) then maybe that's ok. I agree that view.yoga.width = YGValueMake(10, YGUnitPoint) or view.yoga.width = YGPointMake(10) would be more idiomatic though.

I'm fairly confident that I would like view.yoga.width to return a YGValue though. This means we won't have undefined return values and it would automatically handle YGUnitAuto added in https://github.com/facebook/yoga/pull/357.

Regarding pixel vs point. I'm fine with changing the `YGUnit` name to be point instead of pixel. That should be a separate PR though. I agree that loosing `view.yoga.width = 10` would be sad. However if it was as easy as `view.yoga.width = pt(10)` then maybe that's ok. I agree that `view.yoga.width = YGValueMake(10, YGUnitPoint)` or `view.yoga.width = YGPointMake(10)` would be more idiomatic though. I'm fairly confident that I would like `view.yoga.width` to return a `YGValue` though. This means we won't have undefined return values and it would automatically handle `YGUnitAuto` added in https://github.com/facebook/yoga/pull/357.

hartbit commented

2017-02-03 04:26:20 -08:00

(Migrated from github.com)

Regarding pixel vs point. I'm fine with changing the YGUnit name to be point instead of pixel. That should be a separate PR though.

Will submit a PR with this change first of all then. This evening probably (GMT+1).

I agree that loosing view.yoga.width = 10 would be sad. However if it was as easy as view.yoga.width = pt(10) then maybe that's ok. I agree that view.yoga.width = YGValueMake(10, YGUnitPoint) or view.yoga.width = YGPointMake(10) would be more idiomatic though.

Now that I think more about it, I don't like YGPointMake(10) and YGPercentMake(40). It seems as if you're "making" a point/pixel, which you're not, you're creating a value with those units. That means this is what we're left with:

view.yoga.width = pt(10)
view.yoga.width = percent(40)

I'm fairly confident that I would like view.yoga.width to return a YGValue though. This means we won't have undefined return values and it would automatically handle YGUnitAuto added in #357.

Yeah, I agree, it has to be a YGValue property. By the way, here's how we could do auto in Swift by surfacing YGValue as a Value enum with associated values:

enum Value {
	case point(Float)
	case percent(Float)
	case auto
}

postfix operator %

postfix func %(value: Int) -> Value {
	return .percent(Float(value))
}

postfix func %(value: Float) -> Value {
	return .percent(value)
}

extension Value : ExpressibleByIntegerLiteral, ExpressibleByFloatLiteral {
	init(integerLiteral value: Int) {
		self = .point(Float(value))
	}
	
	init(floatLiteral value: Float) {
		self = .point(value)
	}
}

view.yoga.margin = 20
view.yoga.margin = 40%
view.yoga.margin = .auto

> Regarding pixel vs point. I'm fine with changing the YGUnit name to be point instead of pixel. That should be a separate PR though. Will submit a PR with this change first of all then. This evening probably (GMT+1). > I agree that loosing view.yoga.width = 10 would be sad. However if it was as easy as view.yoga.width = pt(10) then maybe that's ok. I agree that view.yoga.width = YGValueMake(10, YGUnitPoint) or view.yoga.width = YGPointMake(10) would be more idiomatic though. Now that I think more about it, I don't like `YGPointMake(10)` and `YGPercentMake(40)`. It seems as if you're "making" a point/pixel, which you're not, you're creating a value with those units. That means this is what we're left with: ``` view.yoga.width = pt(10) view.yoga.width = percent(40) ``` > I'm fairly confident that I would like view.yoga.width to return a YGValue though. This means we won't have undefined return values and it would automatically handle YGUnitAuto added in #357. Yeah, I agree, it has to be a `YGValue` property. By the way, here's how we could do auto in Swift by surfacing `YGValue` as a `Value` enum with associated values: ``` enum Value { case point(Float) case percent(Float) case auto } postfix operator % postfix func %(value: Int) -> Value { return .percent(Float(value)) } postfix func %(value: Float) -> Value { return .percent(value) } extension Value : ExpressibleByIntegerLiteral, ExpressibleByFloatLiteral { init(integerLiteral value: Int) { self = .point(Float(value)) } init(floatLiteral value: Float) { self = .point(value) } } view.yoga.margin = 20 view.yoga.margin = 40% view.yoga.margin = .auto ```

👍 2

emilsjolander commented

2017-02-03 04:29:44 -08:00

(Migrated from github.com)

/drool

That swift code looks amazing

/drool That swift code looks amazing

d16r commented

2017-02-03 06:47:04 -08:00

(Migrated from github.com)

+1 on the swift code, great idea @hartbit!

Regarding pixel vs point. I'm fine with changing the YGUnit name to be point instead of pixel. That should be a separate PR though.

Yeah, I think this is the right move for YogaKit.

So did we ever come to conclusion about the getter values? .width will now return a YGValue? I don't really see this being a big deal. I don't think I've used a getter a single time since I've started converting views, and it looks like we have a great solution for Setters.

@hartbit What do you think about this for Objective-C syntax?

view.yoga.width = YGPercent(50);
view.yoga.height = YGPoint(50);

+1 on the swift code, great idea @hartbit! > Regarding pixel vs point. I'm fine with changing the YGUnit name to be point instead of pixel. That should be a separate PR though. Yeah, I think this is the right move for YogaKit. So did we ever come to conclusion about the getter values? `.width` will now return a `YGValue`? I don't really see this being a big deal. I don't think I've used a getter a single time since I've started converting views, and it looks like we have a great solution for Setters. @hartbit What do you think about this for Objective-C syntax? ``` view.yoga.width = YGPercent(50); view.yoga.height = YGPoint(50); ```

hartbit commented

2017-02-03 11:48:01 -08:00

(Migrated from github.com)

So did we ever come to conclusion about the getter values? .width will now return a YGValue? I don't really see this being a big deal. I don't think I've used a getter a single time since I've started converting views, and it looks like we have a great solution for Setters.

Yeah, sounds good to me.

@hartbit What do you think about this for Objective-C syntax?

I think that's a good compromise. I'll go forward with that. Anyway, who uses Objective-C nowadays (joking :p)

> So did we ever come to conclusion about the getter values? .width will now return a YGValue? I don't really see this being a big deal. I don't think I've used a getter a single time since I've started converting views, and it looks like we have a great solution for Setters. Yeah, sounds good to me. > @hartbit What do you think about this for Objective-C syntax? I think that's a good compromise. I'll go forward with that. Anyway, who uses Objective-C nowadays (joking :p)

😆 1

hartbit commented

2017-02-03 11:50:08 -08:00

(Migrated from github.com)

I posted the PR that renames YGUnitPixel to YGUnitPoint: #375. Once it's merged I'll get the PR up for percent support.

I posted the PR that renames `YGUnitPixel` to `YGUnitPoint`: #375. Once it's merged I'll get the PR up for percent support.

NickGerleman commented

2022-10-04 12:18:38 -07:00

(Migrated from github.com)

Looks like this was resolved by https://github.com/facebook/yoga/pull/390

Sign in to join this conversation.