BEMSimpleLineGraph icon indicating copy to clipboard operation
BEMSimpleLineGraph copied to clipboard

Auto Layout XAxis Label Issue

Open pipeaesac opened this issue 10 years ago • 16 comments

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

pipeaesac avatar May 04 '15 13:05 pipeaesac

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.

tanys123 avatar May 19 '15 03:05 tanys123

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

abdullacontractor avatar May 19 '15 07:05 abdullacontractor

@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.

tanys123 avatar May 19 '15 07:05 tanys123

The solution also works for me.

isneyl16 avatar May 20 '15 09:05 isneyl16

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

mfrawley avatar May 21 '15 13:05 mfrawley

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.

Boris-Em avatar May 21 '15 16:05 Boris-Em

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

cakl avatar Jun 02 '15 03:06 cakl

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

ghost avatar Jun 09 '15 17:06 ghost

@tanys123 Well done!

interchen avatar Jul 17 '15 06:07 interchen

Will this be rolled into as a fix?

ghost avatar Jul 17 '15 12:07 ghost

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 ? ))

emartinson avatar Sep 15 '15 08:09 emartinson

@tanys123 Well done!!! Thanks.

skyhacker2 avatar Sep 25 '15 03:09 skyhacker2

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.

Cellane avatar Sep 29 '15 07:09 Cellane

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:

Sam-Spencer avatar Feb 21 '16 22:02 Sam-Spencer

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

Thank You

mikemike396 avatar Mar 02 '16 20:03 mikemike396

Any plans to push this to master? Thanks

mikemike396 avatar Jul 22 '16 19:07 mikemike396