New TestBed App and multiple bug fixes/cleanups #295

mackworth · 2017-03-26T19:53:13Z

Summary

Creates new TestBed application to manipulate essentially all properties. Also miscellaneous bug fixes, particularly around Null data values.

Fixes Issues

This pull request fixes the following issues:

when my points count is two. enableBezierCurve = YES; Why the main line? #282

Changes

The following changes are included in this pull request:

New TestBed application, intended for iPad, but runs on iPhone as well. Allows manipulation of essentially all properties to test for any problems, and allow visualization of different combinations of options.
Adds encoding/decoding to BEMSimpleGraph to save all properties during a restore.
Enabled (and fixed) more compiler type checking (e.g. NSArray <BEMCircle *> *)
Assorted bug fixes as well (versus existing Feature branch), especially null-data related
AverageLine:
- Color should be picked up from line if not set (default nil)
Line:
- If null data and interpolation on, then extrapolate for beginning/ending nulls.
- Bezier curve should be used even when only two points.
- If interpolation off, then bezier line should be interrupted by gaps (but not top/bottom).
- Avoid infinity result in midPoint calculation.
SimpleLineGraphView:
- Support restoration during startup.
- NoDataLabel color should not default from Line (which defaults to white, invisible on default background)
- If null value, ensure corresponding Dot isn't left on chart
- If label isn't used after initially being created, ensure it's removed from view
- If neither X nor Y reference lines, set line's enableRefLines to NO (although default, might have been previously YES
- If Xaxis background is defaulting to colorBottom, then also use alphaBottom to match; same for Yaxis and colorTop.
- Avoid infinite loop if delegate gives a zero incrementIndex for x axis
- Fix one-pixel gap between yaxis and graph
- Remove spurious NSLogs

Notes

This pull request makes breaking changes, and if so, involves the following:
- Public API availability
- Internal functionality or behavior
I have run and ensured that this pull request passes all XCTests

Include any additional notes on your pull request
After working on another app for a while, I'm back to the one I wanted to use this for, so I've been fixing bugs in the revised version (both mine and long-standing ones). I've merged in the changes that Sam Spencer did, but I may have misunderstood some of those; please check.

In the process of doing this, I wanted a visualization tool that would let me check various attributes beyond what the demo program or IB does. I then got carried away and essentially implemented all the properties. (Remaining unimplemented ones are touchReportFingersRequired, autoScaleYAxis, Dashpatterns for averageLine, XAxis, Yaxis, and Gradient choices beyond a default one.) It also would be cool if it could export to the clipboard the choices you've made into ObjC (or Swift).

The only breaking API change is that I couldn't restrain myself from fixing the multiple typos of Refrence in Line. h. As this is primarily an internal API, I think that's ok, but feel free to put it back if you differ.

I don't think I've broken any real API calls, but there are some changes:
Some of the types are now decorated (e.g. NSArray <NSNumber *> *) and for nullability
alwaysDisplayPopUpAtIndex is now an NSUInteger rather than a CGFloat
Gradients are now strongly held
Added averageValueForLineGraph to delegate
Restored modifyPopupView to be available again; not clear why removed.
Added title and label properties to AverageLine API

As last time, I apologize if I've overstepped, but I really like this library and want it to be as useful as possible

Add averageValue delegate method to allow BEMLineGraph to be separate from BEMCalculator Restore modifyPopUpView delegate method Use caption preferredFont instead of SystemFont fro NoData Handle overlapping labels better Reuse code for popup and permanent labels Bug fixes ensure chart doesn't grow views when zero or one data points

Add coding to AverageLine Move gradient properties to strong to properly retain Fix compiler error about integers used in enums Change alwaysDisplayPopupatIndex to use NSUInteger for index instead of float Fix Frame warning in SimpleLineChart's launch Screen

Fix many bugs and buglets

AverageLine: •Color should be picked up from line if not set (default nil) Typo in encoding macro Line: If null data and interpolation on, then extrapolate for beginning/ending nulls. Bezier curve should be used even when only two points. If interpolation off, then bezier line should be interrupted by gaps (but not top/bottom). Avoid infinity result in midPoint calculation SimpleLineGraphView: Support restoration during startup. NoDataLabel color should not default from Line (which defaults to white, same as background) If null value, ensure corresponding Dot isn't left on chart If label isn't used after initially being created, ensure it's removed from view If neither X nor Y reference lines, set line's enableRefLines to NO (although default, might have been previously YES If Xaxis background is defaulting to colorBottom, then also use alphaBottom to match; same for Yaxis and colorTop. Avoid possible infinite loop if delegate gives a zero incrementIndex for x axis Fix one-pixel gap between yaxis and graph Remove spurious NSLogs

mackworth · 2017-03-26T22:48:55Z

I have no idea about the CI build error; RVM and Debian?

In running the BEMSimple tests, I noticed a couple of issues.
By adding more compiler checks, it now complains about reserved macro names. Need to add `#pragma GCC diagnostic ignored "-Wreserved-id-macro" in Test
Similarly change deployment target of test to 9.0 for test
Finally, it fails while comparing the default UIColor of the dots. This looks like either an issue of Extended color space in iOS10, or a float v double issue.

I also note that I left a spurious NSLog(@"VWA"); in MasterViewController

Oh, one other thought: do you like separating the colors/alpha/gradients, or should they be with their groups. More broadly, the grouping of parameters may need to be rethought here.

Line clipsToBounds to avoid drawing outside the box (when extrapolating nulls or user set minValue too High) Top/Bottom reference lines were outside box When Bezier off and Interpolate nulls off, properly draws line segments now Removed VMA NSLog from TestBed/MasterController

Boris-Em · 2017-03-28T17:35:33Z

This looks very solid @mackworth! I'll make sure to make some time and review the PR soon. Thanks a lot for your work =)

mackworth · 2017-04-03T07:20:35Z

Thanks; I look forward to your comments.

More broadly, is there a current project plan? As I'm going to be using this in my own project, I've been making some other (what I consider) improvements, and I don't know what things you all are working on, or would be interested in receiving. For example, I've moved to a cubic Bezier (from the quadratic) to fix the wave-like pattern on linear growth curves. I also needed a more flexible X-axis (where an X value can be any float, not just integers) to display data that might be timestamped at any time. I've implemented it for my own use, but I'd need to collaborate on the API to be able to submit it as a possible PR. Finally, as I fully "get" the code now, I'd be happy to put some time in to implement other features high on your wishlist as well. Let me know.

mackworth · 2017-05-12T16:53:24Z

Superseded by PR #300, which also incorporates the changes mentioned above. I created a separate pull request in case the addition of the pan/zoom and variableX are determined to be inappropriate for this library. Let me know what you think and/or if you have any problems with the other build.

Boris-Em · 2017-05-12T17:22:25Z

Thanks for creating a new PR. And I'm really sorry for not reviewing all of this earlier. I've been busy with life/work, but truly appreciate your work and want to get at it as soon as possible.

mackworth · 2017-05-13T19:03:47Z

No problem. As a first step, try running the new Testbed, preferably on an iPad to get a feel for some of the new changes.
I'm pretty happy with its usefulness for both learning the API and testing combinations.

Sam-Spencer · 2017-05-13T19:50:51Z

@mackworth @Boris-Em I have begun my review of the changes and should finish in the next day or two. There are some changes & fixes which I definitely would like to see integrated into the project and that I believe will move the it in the correct direction. That being said, I still need to test and use all the changed aspects, and (after having read through the commits & change logs) there are some things of which I am skeptical.

I will also take a look at PR #300 (thank you for separating this requests).

Thank you, @mackworth, for your extensive work on the project. Your contributions are greatly appreciated!

Boris-Em

Thanks again for you work @mackworth. This is really helpful!
I just did a first pass and found a few minor things here and there. I'll spend more time reviewing the changes tomorrow, digging more into the business logic.

A few thoughts:

While I agree that this PR doesn't break any public APIs, it's still a pretty risky one, and some of the rendering is changed (for example reference lines).
Do we want to keep both the Sample App and the TestBed app? I feel like the TestBed app is just superior at that point. @Sam-Spencer thoughts?
On iPhone, it's sort of hard to go back to the graph VC from the options. Could we get a button in the navigation bar to navigate to the Graph VC?

Boris-Em · 2017-05-14T03:11:35Z

Classes/BEMAverageLine.m

        _alpha = 1.0;
        _width = 3.0;
        _yValue = NAN;
    }

    return self;
 }
+
+- (instancetype) initWithCoder:(NSCoder *)coder {


Nit: extra space.

Boris-Em · 2017-05-14T03:14:47Z

Classes/BEMAverageLine.m

+    return self;
+}
+
+- (void) encodeWithCoder: (NSCoder *)coder {


Nit: extra space.

Boris-Em · 2017-05-14T03:19:12Z

Sample Project/TestBed/StatsViewController.m

+
+- (void)viewDidLoad {
+    [super viewDidLoad];
+	// Do any additional setup after loading the view.


Nit: let's remove that comment.

Boris-Em · 2017-05-14T03:25:53Z

Classes/BEMLine.m

@@ -59,8 +60,8 @@ - (void)drawRect:(CGRect)rect {
    if (self.enableReferenceFrame == YES) {
        if (self.enableBottomReferenceFrameLine) {
            // Bottom Line
-            [referenceFramePath moveToPoint:CGPointMake(0, self.frame.size.height)];
-            [referenceFramePath addLineToPoint:CGPointMake(self.frame.size.width, self.frame.size.height)];
+            [referenceFramePath moveToPoint:CGPointMake(0, self.frame.size.height-self.referenceLineWidth/4)];


I realize that this wasn't introduced by you @mackworth, but could we use a constant here for all of the magic numbers (4)?

Boris-Em · 2017-05-14T03:29:59Z

Classes/BEMLine.m

@@ -85,15 +86,15 @@ - (void)drawRect:(CGRect)rect {
    if (self.enableReferenceLines == YES) {
        if (self.arrayOfVerticalReferenceLinePoints.count > 0) {
            for (NSNumber *xNumber in self.arrayOfVerticalReferenceLinePoints) {
-                CGFloat xValue;
+                CGFloat xValue =[xNumber doubleValue];


Nit: missing space after =.

Boris-Em · 2017-05-14T03:59:01Z

Classes/BEMSimpleLineGraphView.m

@@ -1207,6 +1233,7 @@ - (UIImage *)graphSnapshotImageRenderedWhileInBackground:(BOOL)appIsInBackground

 - (void)reloadGraph {
    [self drawGraph];
+    //    [self setNeedsLayout];


Let's remove this dead code.

Boris-Em · 2017-05-14T04:01:01Z

Classes/BEMSimpleLineGraphView.m

@@ -1394,7 +1464,8 @@ - (CGFloat)yPositionForDotValue:(CGFloat)dotValue {

 #pragma mark - Deprecated Methods

- (NSNumber *)calculatePointValueSum {
+


Nit: Let's remove the extra line.

Boris-Em · 2017-05-14T04:02:05Z

Sample Project/TestBed/AppDelegate.m

+//  Copyright © 2017 Boris Emorine. All rights reserved.
+//
+
+#import "AppDelegate.h"


Let's cleanup all of the default comments and unused methods on this file.

Boris-Em · 2017-05-14T04:04:33Z

Sample Project/TestBed/MasterViewController.m

+@property (strong, nonatomic) UIView * currentColorChip;
+@end
+
+@implementation UITextField (Numbers)


Can we put this category on its own file?

Boris-Em · 2017-05-14T04:06:20Z

Sample Project/TestBed/MasterViewController.m

+
+- (void)viewDidLoad {
+    [super viewDidLoad];
+    // Do any additional setup after loading the view, typically from a nib.


Let's set the title of the view controller somewhere in viewDidLoad: self.title = @"Options".
This way the back arrow will display "Options" instead of "Master" which doesn't really make sense.

Fixing breaks, spaces to match project style Explaining constant 4 in BEMLine (note bug on line76 0+offset should be 0)

Fix top reference line offset bug Adds categories for UIKit extensions in MasterVC Adds Options title for iPhone version of TestBed Only changes in storyboard due to newer IB

mackworth · 2017-05-14T18:21:49Z

There you go on the requested changes. Still don't know what to do about Travis

While I agree that this PR doesn't break any public APIs, it's still a pretty risky one, and some of the rendering is changed (for example reference lines).

Definitely agreed on the risky; I've built it into my app, and it's running great, but it might be good to get a couple of others to try even before putting it in "Feature". On reference lines, assuming you mean the frame ones, they were just wrong before! (Even now, as a result of your question on "4", i noticed that the top line was still not right.

Do we want to keep both the Sample App and the TestBed app? I feel like the TestBed app is just superior at that point. @Sam-Spencer thoughts?

I would definitely keep both; Testbed is way too complicated to be your sample. OTOH, Sample App might want to be stripped some to make it even simpler to follow as an example.

On iPhone, it's sort of hard to go back to the graph VC from the options. Could we get a button in the navigation bar to navigate to the Graph VC?

I did most of my work on the iPad( and it's really a far better vehicle to avoid going back and forth), but agreed and done.

Note that if you decide not to implement #300, or only some parts of it, then I'll have to move some of the fixes in #300 back into this branch. Conversely, if you want both, then I'll have to merge these changes into that PR.

Looking forward to more comments...

Boris-Em · 2017-05-14T19:23:00Z

Thank you for being so responsive @mackworth!
I just opened a PR against your branch that should fix Travis.
I'm alright with merging your changes on the feature branch pretty quickly. We will need more tests before merging it to master, and it will definitely require a version bump.
Keeping both the TestBed and Sample App sounds good. Your explanation makes sense.
I really think that we'll be merging #300 as well. I'd like to finish merging this branch first though.

Spacing consistency with rest of project topline fix Move UIButton+switch and UITextField+Numbers from Master to separate files Title of Master "Options" Add Graph button if collapsed

Fix Travis Build

mackworth · 2017-05-15T00:47:30Z

Okay, yes, looks like that travis.yml now passes. I updated it in #300 as well, and it passes there too.

mackworth · 2017-05-29T01:31:21Z

Any further thoughts on this PR?

Boris-Em · 2017-06-21T23:10:30Z

@Sam-Spencer, did you get a chance to finish your review?

Sam-Spencer

Okay so I have three major thoughts on this pull request:

There's a whole pile of great bug fixes and minor improvements in here that are much needed for the project. A few may need some tweaking, but from what I can see it looks good and I'm satisfied that these changes are heading in the right direction. Especially appreciated changes include the addition of NSCoding compliance and NSArray with type specifications. I also like the changes you've made to the line and frame drawings, and the related issues fixed in that scope. @Boris-Em As you stated in your review, I don't think that the internal API changes here are too severe - and in cases where the changes may affect implementation I think that it would be worth it.
The use of other third-party libraries in this project makes me very nervous. I would imagine that developers looking to integrate a UIKit project would like it to be as self-contained as possible... it's already a risk including a third-party project in your app (let alone including one that relies on other ones, etc.). I think in most cases where these dependencies have been added, they can be removed and replaced with much simpler self-contained solutions (i.e. choose between two fonts or colors).
In all honesty I am not 100% sure how I feel about the TestBed application. I can see where it may be useful to test all of the different available properties on the graph, however to me it seems that a more effective use of time and resources would be to maintain unit tests and good documentation. Frankly, I can't see a use case for the TestBed that isn't already covered by the sample app, unit tests, and documentation. If testing different properties or features is the goal, I personally find it easier to make the necessary changes in code and then quickly build, run, and analyze. Maybe you could explain where this addition would be helpful?

With that in mind, know that I am open to approving and merging this pull request as is. Once on the feature branch we can work on it and improve it until its at the point where we are ready for master.

Thank you for all of your contributions, @mackworth! This means a lot.

Sam-Spencer · 2017-06-28T19:46:30Z

Classes/BEMSimpleLineGraphView.h

@@ -516,7 +520,7 @@ IB_DESIGNABLE @interface BEMSimpleLineGraphView : UIView <UIGestureRecognizerDel
 @param graph The graph object requesting the padding value.
 @param popupView The popup view owned by the graph that needs to be modified
 @param index The index of the element associated with the popup view */
- (void)lineGraph:(BEMSimpleLineGraphView *)graph modifyPopupView:(UIView *)popupView forIndex:(NSUInteger)index __unavailable;
+- (void)lineGraph:(BEMSimpleLineGraphView *)graph modifyPopupView:(UIView *)popupView forIndex:(NSUInteger)index;


Potential misunderstanding here... why has this been marked as available again?

Hmm, never saw it as unavailable. Why was it taken out? How are you supposed to use the custom Popup if it stays the same on every point?

Sam-Spencer · 2017-06-28T19:50:39Z

Classes/BEMSimpleLineGraphView.m

+#pragma clang diagnostic pop
+}
+
+- (void) encodeWithEncoder: (NSCoder *)coder {


Love ❤️ that you've added NSCoding support to this object. If not already written, unit tests for this should be added. Documentation should also cover this.

Sam-Spencer · 2017-06-28T19:51:55Z

Classes/BEMSimpleLineGraphView.m

 - (void)layoutNumberOfPoints {
    // Get the total number of data points from the delegate
+#ifndef TARGET_INTERFACE_BUILDER


Okay, good move with the interface builder compatibility.

Sam-Spencer · 2017-06-28T20:01:59Z

Classes/BEMSimpleLineGraphView.m


+        if (self.enableTouchReport && [self.delegate respondsToSelector:@selector(lineGraph:didTouchGraphWithClosestIndex:)]) {
+            [self.delegate lineGraph:self didTouchGraphWithClosestIndex:index];


Not sure if this was introduced in this PR but it looks like when the data source provides a BEMNullGraphValue for a data point it also gets returned by the touch report and displayed in popup views. Something to look into... should be an easy fix.

Hmm, I don't think so; circleDots aren't created when there's no value, so closestDotfromTouchInputLine can't see them. I tried it in TestBed and it didn't show null values.

Sam-Spencer · 2017-06-28T20:02:33Z

Classes/BEMAverageLine.h

@@ -35,7 +35,7 @@


 /// Dash pattern for the average line
-@property (strong, nonatomic, nullable) NSArray *dashPattern;
+@property (strong, nonatomic, nullable) NSArray <NSNumber *> *dashPattern;


Yes! Love these changes! Perfect!

Sam-Spencer · 2017-06-28T20:06:27Z

Sample Project/TestBed/ARFontPickerViewController.h

+
+@import UIKit;
+
+@class ARFontPickerViewController;


I am very wary of using third-party dependencies for a demo / test app included with the project unless it is absolutely necessary. While this is a robust solution, it may be better to simply include a toggle between two hard-coded system-installed fonts.

Sam-Spencer · 2017-06-28T20:23:18Z

Classes/BEMLine.m

@@ -142,26 +143,62 @@ - (void)drawRect:(CGRect)rect {

    self.points = [NSMutableArray arrayWithCapacity:self.arrayOfPoints.count];


Absolutely agreed. Maybe this is something we can work on after merging...

Sam-Spencer · 2017-06-28T20:25:34Z

Sample Project/SimpleLineChartTests/CustomizationTests.m

@@ -12,7 +12,7 @@
 #pragma clang diagnostic ignored "-Wfloat-equal"

 @interface BEMSimpleLineGraphView ()
-// Allow tester to get to internal properties
+//Allow tester to get to internal properties


Nit: need space between comment marker and first word.

Sam-Spencer · 2017-06-28T20:27:53Z

Sample Project/TestBed/MSColorPicker/MSColorComponentView.h

+
+#import <UIKit/UIKit.h>
+
+@class MSSliderView;


Again, I am concerned about including third-party components here when not absolutely necessary. For example, this color choice problem can be easily solved with a few lines of our own code that provide a few standard values... rather than relying on external code which may cause issues, need to be rewritten, updated, etc. at some point.

Sam-Spencer · 2017-06-28T20:28:45Z

Sample Project/TestBed/MSColorPicker/MSColorWheelView.m

+#import "MSColorWheelView.h"
+#import "MSColorUtils.h"
+
+@interface MSColorWheelView ()


See other comments about third-party library inclusion...

Boris-Em · 2017-06-28T22:12:14Z

Thank you @Sam-Spencer!

Agreed.
Unless I misunderstood something, it doesn't seem to me that BEMSimpleLineGraph is taking on any third party dependencies: only the TestBed app is. A developer using our library wouldn't take on any additional dependency by using BEMSimpleLineGraph. I'm fine with this, as long as the external dependencies don't make it to the actual library. Having said that, I also agree that this practice should be avoided as much as reasonably possible. As you mentioned, we probably could avoid using those dependencies here.
I agree with you that ideally using tests would be a better way to unsure nothing is getting broken. But this is something that we've been really bad at, and furthermore, testing UI components is far from easy... This is why I can see value in the TestBed app.

mackworth · 2017-06-29T03:57:48Z

So, thanks for the nice compliments. To respond to the general points,

On the third-party libraries, Boris is correct, there's no dependency for the library itself, just internal to TestBed.
So on TestBed, the main uses for me are:
(a) to test all kinds of different combinations of features. Ticking them on/off and seeing how they interact is much faster than coding and recompiling,
(b) lets one learn what all the different options mean, better than the comments do, cause you can see them work.
(c) Finally, lets one visually create the combination desired to get the ideal graph look beyond what IB can do. This is particularly where the third party libraries that let me choose the exact font and color I would use in my program are nice. (this is also where it would be cool to be able to export the exact code itself).

The sample app is to see how to incorporate SimpleLineGraphView into your own program; Testbed is to see what all the possibilities are.

But as I've said, it's your library :), so let me know.

To summarize, I think the open questions are:

Including TestBed
Using third party (open source and included) libraries in TestBed
"unavailable" on modifyPopupView
Sam, let me know if you're really seeing BEMNulls in touchReport. I remember I did break that once, but I don't think this version has that problem.
Missing space in comment.

Avoid comma operator in ColorPicker (compiler correctly complains)

mackworth · 2017-07-14T16:51:29Z

So, @Sam-Spencer, are we good on 1 and 2 above, and any further info on 3 and 4?

* commit '3dd033de496f74a541c3117f3e7432cbb8bce2c7': (37 commits) Remove unuseful breaking test Update .travis.yml Add "Graph" button to Compact devices Further fixes for more substantive comments on PR Boris-Em#295 Fix top reference line offset bug Adds categories for UIKit extensions in MasterVC Adds Options title for iPhone version of TestBed Only changes in storyboard due to newer IB Requested changes to PR 3295 Fixing breaks, spaces to match project style Explaining constant 4 in BEMLine (note bug on line76 0+offset should be 0) Bug fixes: Line clipsToBounds to avoid drawing outside the box (when extrapolating nulls or user set minValue too High) Top/Bottom reference lines were outside box When Bezier off and Interpolate nulls off, properly draws line segments now Removed VMA NSLog from TestBed/MasterController Fix bug with TouchLineInput color (doesn't change after initial setting) Allow use on iPhone and Split View Fix assorted bugs in BEMSimpleLineGraph, especially null-data related AverageLine: •Color should be picked up from line if not set (default nil) Typo in encoding macro Add colors, gradients, and alphas to TestBed Fix many bugs and buglets Create new TestBed app to manipulate almost all parameters Add coding to AverageLine Move gradient properties to strong to properly retain Fix compiler error about integers used in enums Change alwaysDisplayPopupatIndex to use NSUInteger for index instead of float Fix Frame warning in SimpleLineChart's launch Screen Add Types to all Arrays Add encoder/decoder for BEMLineGraph properties Add averageValue delegate method to allow BEMLineGraph to be separate from BEMCalculator Restore modifyPopUpView delegate method Use caption preferredFont instead of SystemFont fro NoData Handle overlapping labels better Reuse code for popup and permanent labels Travis CI Yet another Travis CI change Update .travis.yml N^th time's a charm? Storyboard Update (to fix Travis CI) Update .travis.yml Update .travis.yml ... # Conflicts: # Classes/BEMLine.m # Classes/BEMSimpleLineGraphView.h # Classes/BEMSimpleLineGraphView.m

Sam-Spencer · 2017-07-18T18:45:34Z

@mackworth It looks like when a BEMNullGraphValue is added to the graph, those values are not handled properly in the interface. For example, if the delegate does not implement a custom popup but requests that popups be shown, the graph will print the literal value of BEMNullGraphValue in the popup (which happens to be CGFLOAT_MAX).

Interestingly enough, it seems that the graph does not display a dot when it comes across one of these values. Yet it is still also reported back to the delegate in other methods (i.e. didTouchGraphWithClosestIndex:).

Here's what it looks like when this happens:

I should also note that this behavior only seems to occur when doing subsequent (non-initial) graph updates (e.g. a point has been added or removed and the graph object is reloaded).

Use lack of superview to distinguish Null points

mackworth · 2017-07-19T04:44:59Z

Good catch! There was a confusion whether to have the same number of circleDots as dataPoints (including Nulls) or only have one for each valid dataPoint.
I've added a commit where they are always the same length, but uses lack of superview to distinguish the ones representing Nulls.

Sam-Spencer · 2017-07-21T18:10:16Z

The PR now looks good to merge into the feature branch. Thank you so much for all the work you've done here @mackworth! 🙌

mackworth added 7 commits March 18, 2017 16:06

Add Types to all Arrays

9bfe179

Add colors, gradients, and alphas to TestBed

7dc9f3a

Fix many bugs and buglets

Allow use on iPhone and Split View

a10ad12

Fix bug with TouchLineInput color (doesn't change after initial setting)

1169854

Boris-Em requested review from Boris-Em and Sam-Spencer March 28, 2017 17:34

Boris-Em added the enhancement label Mar 28, 2017

mackworth mentioned this pull request May 12, 2017

Pan/Zoom and Variable X Axis; many other changes #300

Open

4 tasks

Sam-Spencer added the bugfix label May 13, 2017

Boris-Em suggested changes May 14, 2017

View reviewed changes

mackworth added 3 commits May 14, 2017 08:48

Requested changes to PR 3295

0e33b98

Fixing breaks, spaces to match project style Explaining constant 4 in BEMLine (note bug on line76 0+offset should be 0)

Further fixes for more substantive comments on PR Boris-Em#295

c107658

Fix top reference line offset bug Adds categories for UIKit extensions in MasterVC Adds Options title for iPhone version of TestBed Only changes in storyboard due to newer IB

Add "Graph" button to Compact devices

e69998a

Boris-Em added 2 commits May 14, 2017 11:46

Update .travis.yml

212058f

Remove unuseful breaking test

3dd033d

Merge pull request #1 from Boris-Em/PR295

b2aff57

Fix Travis Build

Sam-Spencer suggested changes Jun 28, 2017

View reviewed changes

mackworth added 2 commits July 8, 2017 13:18

Fix space nit in comment

fa2c33e

Avoid comma operator in ColorPicker (compiler correctly complains)

Merge remote-tracking branch 'origin/feature' into feature

2072327

Ensure circleDots always same length as dataPoints

e1c6fd6

Use lack of superview to distinguish Null points

Sam-Spencer approved these changes Jul 21, 2017

View reviewed changes

Boris-Em approved these changes Jul 21, 2017

View reviewed changes

Boris-Em merged commit 42bb744 into Boris-Em:feature Jul 21, 2017

mackworth deleted the feature branch July 21, 2017 18:50

		@@ -1394,7 +1464,8 @@ - (CGFloat)yPositionForDotValue:(CGFloat)dotValue {

		#pragma mark - Deprecated Methods

		- (NSNumber *)calculatePointValueSum {


		if (self.enableTouchReport && [self.delegate respondsToSelector:@selector(lineGraph:didTouchGraphWithClosestIndex:)]) {
		[self.delegate lineGraph:self didTouchGraphWithClosestIndex:index];

		@@ -142,26 +143,62 @@ - (void)drawRect:(CGRect)rect {

		self.points = [NSMutableArray arrayWithCapacity:self.arrayOfPoints.count];


		@import UIKit;

		@class ARFontPickerViewController;


		#import <UIKit/UIKit.h>

		@class MSSliderView;

New TestBed App and multiple bug fixes/cleanups #295

New TestBed App and multiple bug fixes/cleanups #295

Conversation

mackworth commented Mar 26, 2017 • edited by Sam-Spencer Loading

Summary

Fixes Issues

Changes

Notes

mackworth commented Mar 26, 2017 • edited Loading

Boris-Em commented Mar 28, 2017

mackworth commented Apr 3, 2017

mackworth commented May 12, 2017

Boris-Em commented May 12, 2017

mackworth commented May 13, 2017

Sam-Spencer commented May 13, 2017

Boris-Em left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mackworth commented May 14, 2017 • edited by Boris-Em Loading

Boris-Em commented May 14, 2017

mackworth commented May 15, 2017

mackworth commented May 29, 2017

Boris-Em commented Jun 21, 2017

Sam-Spencer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Boris-Em commented Jun 28, 2017 • edited Loading

mackworth commented Jun 29, 2017

mackworth commented Jul 14, 2017

Sam-Spencer commented Jul 18, 2017

mackworth commented Jul 19, 2017

Sam-Spencer commented Jul 21, 2017 • edited Loading

mackworth commented Mar 26, 2017 •

edited by Sam-Spencer

Loading

mackworth commented Mar 26, 2017 •

edited

Loading

mackworth commented May 14, 2017 •

edited by Boris-Em

Loading

Sam-Spencer left a comment •

edited

Loading

Boris-Em commented Jun 28, 2017 •

edited

Loading

Sam-Spencer commented Jul 21, 2017 •

edited

Loading