Skip to content
This repository has been archived by the owner on Dec 2, 2020. It is now read-only.

Pr/263 (@mackworth) #287

Merged
merged 2 commits into from
Feb 2, 2017
Merged

Pr/263 (@mackworth) #287

merged 2 commits into from
Feb 2, 2017

Conversation

Sam-Spencer
Copy link
Collaborator

ORIGINAL PULL REQUEST: #263 from @mackworth. This pull request makes a number of significant improvements to the project and @mackworth deserves a huge round of applause for these contributions. The reason this duplicate of the original PR is being merged is due to my own negligence and some issues with git... 😬. Below is the original text from the Pull Request.

Summary

First, thanks for a very nice project. I realize this is a weird PR. In order to fix a rotation issue (due to labels being regenerated every time), I changed to tracking the subviews, but did that in my own personal repo. In order to do that, as I was understanding each component, I refactored to avoid duplicate code.

I wasn't planning to submit it, just did it for my own use, then I noticed requests (#286, #272, #203, #109) to remove the tag dependency. So, I re-pulled from your Feature version and generated a new commit with all my changes in it. My guess is you're not going to like the extent of my refactoring or the testing that would be required, but thought I'd offer it. If you do want to incorporate some or all of this, but are offended by the obviously massive commit, I'm willing to go back through and break it up into smaller commits and do further testing.

Fixes Issues

This pull request fixes the following issues:

  • Reuses views when possible to make rotation work properly (and 40% better reload performance)
  • Leaking UIViews during rotation
  • A few typos in documentation
  • Added nullability to BEMLine.h, circle etc
  • Combined label and background view for pouplabels
  • Typo: "Refrence [sic.]" misspelling corrected
  • More consistency on prefix/suffix usage

[The following may not be included in this duplicate PR due to merge conflicts and changes made earlier] Compiler warnings (for projects with -WEverything)

  • Compiler warnings about CGFloat losing precision
  • Compiler warnings about sign changing (uses NSUInteger when possible)
  • Compiler warnings about using Formats as literals

Changes

Features:

  • Removes all tags (except for mapping from dots back to values)
  • Configurable y-axis title for Average line
  • Refactored to remove redundant code
  • Access to graphLabelsForYAxis (parallel to graphLabelsForXAxis)

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 (as modified)

Breaking Change Notes
Breaking reference is to the use of NSUInteger in API to keep consistent sign usage throughout. Could be put back and use NSInteger internally instead. Otherwise, it will break any code that looks for tags (although they could be added back just for this reason) such as tests prior to modification.

Additional Notes
One known bug, I deleted and forgot to restore the customPopup delegate calls.

mackworth and others added 2 commits May 16, 2016 13:45
	Removes all UIView tags (except for mapping from dots back to values)
	Access to graphLabelsForYAxis (parallel to graphLabelsForXAxis)
	Configurable title for Average line
	Refactored to remove redundant code

Fixes:
	Reuses views when possible to make rotation work properly (and 40% better reload performance)
	Avoids leaking UIViews during rotation
	A few typos in documentation
	Added nullability to BEMLine.h, circle etc
	Combined label and background view for pouplabels
	Typo: Refrence misspelling corrected
	More consistency on prefix/suffix usage

Compiler warnings (for projects with -WEverything)
	Compiler warnings about CGFloat losing precision
	Compiler warnings about sign changing (uses NSUInteger when possible)
	Compiler warnings about using Formats as literals
# Conflicts:
#	Classes/BEMSimpleLineGraphView.h
#	Classes/BEMSimpleLineGraphView.m
#	Sample Project/SimpleLineChart.xcodeproj/project.pbxproj
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants