Boris-Em / BEMSimpleLineGraph

Elegant Line Graphs for iOS. (Charting library)
MIT License
2.65k stars 381 forks source link

Auto Layout XAxis Label Issue #206

Open pipeaesac opened 9 years ago

pipeaesac commented 9 years ago

When using autolayout on the view which represents the graph, the XAxis labels are shown/not shown depending on the height of the graph.

The following properties on the graph have been set:

self.myGraph.enableYAxisLabel = YES;
self.myGraph.enableXAxisLabel = YES;
self.myGraph.autoScaleYAxis = YES;

This first screenshot shows the graph with XAxisLabels. This is with a bottom space and top space constraint of _200_ to superview.

alt text

This second screenshot shows the graph without XAxisLabels, although those are still enabled. This is with a bottom space and top space constraint of _250_ to superview.

alt text

This can be reproduced using the following repository: https://github.com/pipeaesac/SimpleGraphSample

tanys123 commented 9 years ago

In, BEMSimpleLineGraphView.m

I have the same issue too. What i do was change the center calculation on

(UILabel *)xAxisLabelWithText:(NSString *)text atIndex:(NSInteger)index

I changed it to

center = CGPointMake(positionOnXAxis, self.frame.size.height - lRect.size.height + 5);

This will work for any size of the frame.

abdullacontractor commented 9 years ago

@tanys123 that solution worked for me! Do you know if the fix causes any other unexpected behaviour?

tanys123 commented 9 years ago

@Ace2196 thats great! Until now i don't have any problem. I even had tried resizing the frame to any size and cause me no problem.

isneyl16 commented 9 years ago

The solution also works for me.

mfrawley commented 9 years ago

Isn't this a bug in the library? can the above patch be generalised and merged?

Boris-Em commented 9 years ago

Yes, this is definitely a bug. How much testing has been done on the solution found by @tanys123? Feel free to submit a PR if you have the time.

cakl commented 9 years ago

+1, any updates? it's very sad if it's not auto layout compliant

ghost commented 9 years ago

This does fix the issue. Here is a related issue on stackoverlfow. Thanks for the quick fix.

http://stackoverflow.com/questions/30647819/bemsimplelinegraph-x-axis-labels-not-showing-for-phones-newer-than-iphone-5

interchen commented 9 years ago

@tanys123 Well done!

ghost commented 9 years ago

Will this be rolled into as a fix?

emartinson commented 9 years ago

Thanks! I helped me also! Isn't it strange to use the same code?

if (self.positionYAxisRight) {
    center = CGPointMake(positionOnXAxis, self.frame.size.height - lRect.size.height/2);
} else {
    center = CGPointMake(positionOnXAxis, self.frame.size.height - lRect.size.height/2);
}

In both cases the code is the same, why do you need this IF ? ))

skyhacker2 commented 9 years ago

@tanys123 Well done!!! Thanks.

Cellane commented 9 years ago

While the solution proposed by @tanys123 works just fine, I feel like using the magic constant of five doesn't feel exactly right – I suspect it might work for standard font size, but maybe not so much for larger fonts, or something like that. Mind you, I haven't tested this hypothesis of mine, but I wonder if there wasn't a better solution to this problem, one that might not break in the future so easily.

While looking into this, I noticed it's nothing more than a rounding issue:

(lldb) p (BOOL)CGRectContainsRect((CGRect)[self bounds],(CGRect)[label frame])
(BOOL) $9 = NO
(lldb) p (BOOL)CGRectContainsPoint((CGRect)[self bounds],CGPointMake(((CGRect)[label frame]).origin.x,((CGRect)[label frame]).origin.y))
(BOOL) $10 = YES
(lldb) p (BOOL)CGRectContainsPoint((CGRect)[self bounds],CGPointMake(((CGRect)[label frame]).origin.x,((CGRect)[label frame]).origin.y + ((CGRect)[label frame]).size.height))
(BOOL) $11 = NO
(lldb) p (BOOL)CGRectContainsPoint((CGRect)[self bounds],CGPointMake(((CGRect)[label frame]).origin.x,((CGRect)[label frame]).origin.y + ((CGRect)[label frame]).size.height - 0.1))
(BOOL) $12 = YES
(lldb) p (BOOL)CGRectContainsPoint((CGRect)[self bounds],CGPointMake(((CGRect)[label frame]).origin.x,((CGRect)[label frame]).origin.y + ((CGRect)[label frame]).size.height - 0.01))
(BOOL) $13 = YES
(lldb) p (BOOL)CGRectContainsPoint((CGRect)[self bounds],CGPointMake(((CGRect)[label frame]).origin.x,((CGRect)[label frame]).origin.y + ((CGRect)[label frame]).size.height - 0.001))
(BOOL) $14 = YES
(lldb) p (CGRect)[self bounds]
(CGRect) $15 = (origin = (x = 0, y = 0), size = (width = 280, height = 240))
(lldb) p ((CGRect)[label frame]).origin.y + ((CGRect)[label frame]).size.height
(double) $16 = 240.00000000000003

So… since it's a rounding issue, even raising the center by one point (center = CGPointMake(positionOnXAxis, self.frame.size.height - lRect.size.height/2 - 1);) should solve the problem (and I do realize that the -1 is still a magic number, but one with a clear meaning now); or, a more clean solution would be to align the bottom of the X axis label to the bottom of the entire graph, either via a constraint or by calculating the proper origin.y value.

FWIW, the UILabel gets actually removed from superview at around lines 794-802.

Sam-Spencer commented 8 years ago

Thank you to everyone for the contributions and suggestions. Commit e059531 on the feature branch should fix this issue. Testing and feedback is highly encouraged on this topic. Again, thank you to everyone! :fireworks:

mikemike396 commented 8 years ago

Can you please post when this is moved to main branch?

Thank You

mikemike396 commented 8 years ago

Any plans to push this to master? Thanks