feat: new protocol for chained functions, and added support for expli… #252
Conversation
…cit Y ranges. X coming as well
AppPear
force-pushed
the
feat/new-protocol-and-range
branch
from
3202ac9 to
b48b701
Compare
|
|
||
| var values: [String] { | ||
| data.map { $0.0 } | ||
| var values: [Double] { |
There was a problem hiding this comment.
This comment is very subjective but even when I started looking at the code it took me tiny bit of time to which axis these points and values are referring to. I would propose to use rather xData and yData instead of points and values.
There was a problem hiding this comment.
I would leave it points and values as not every chart has yValues (circle graphs so it wouldn't make sense there)
| } | ||
|
|
||
| var normalisedYRange: Double { | ||
| if let _ = rangeY { |
There was a problem hiding this comment.
I think could be rewritten as "one-liner"
rangeY == nil
? (normalisedPoints.max() ?? 0.0) - (normalisedPoints.min() ?? 0.0)
: 1
| if gridOptions.showBaseLine { | ||
| ChartGridBaseShape() | ||
| .stroke(gridOptions.color, style: gridOptions.baseStrokeStyle) | ||
| .rotationEffect(.degrees(180), anchor: .center) |
There was a problem hiding this comment.
It is not very clear why we need to do rotation by 180 degrees here. It seems it's just flipped, or not? In other words, is it necessary?
| Group { | ||
| ChartGridShape(numberOfHorizontalLines: 5, numberOfVerticalLines: 0) | ||
| .stroke() | ||
| .rotationEffect(.degrees(180), anchor: .center) |
There was a problem hiding this comment.
It also seems that these rotations are not necessary here. When I comment them the preview stays the same. But maybe I am just missing something important.
There was a problem hiding this comment.
Ok, I think I got it. Are you trying to get x and y axis to be the same as you would intuitively expect? As in, point (0.0) is not top left corner but you want it to be lower left?
I think padding of line .rotation3DEffect(.degrees(180), axis: (x: 0, y: 1, z: 0)) is not correct.
| Group { | ||
| MarkerShape(data: [(0, 0), (0.25, 0.5), (0.5,0.8), (0.75, 0.6), (1, 1)]) | ||
| .stroke() | ||
| .rotationEffect(.degrees(180), anchor: .center) |
There was a problem hiding this comment.
I think I would put it into View extension as it's keep being used on many places. Maybe something like this?
extension View {
func toStandardCoordinateSystem() -> some View {
self
.rotationEffect(.degrees(180), anchor: .center)
.rotation3DEffect(.degrees(180), axis: (x: 0, y: 1, z: 0))
}
}
2a52e35 to
f426dba
Compare
feat: new protocol for chained functions, and added support for explicit Y ranges. X coming as well