-
-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added bar gradient #3225
Added bar gradient #3225
Conversation
Codecov Report
@@ Coverage Diff @@
## 4.0.0 #3225 +/- ##
==========================================
- Coverage 31.49% 31.47% -0.02%
==========================================
Files 112 112
Lines 10050 10091 +41
==========================================
+ Hits 3165 3176 +11
- Misses 6885 6915 +30
Continue to review full report at Codecov.
|
@alekzuz works fine, thanx! |
Thanks for your help! Currently master won't accept any feature PRs until 3.1 is released. |
Yes, I can do this.
I have to open new pull request or to change target branch and resolve conflicts ? |
@alekzuz seems you don't have to, just edit the title to see if you can change the base branch, at least I can see it. |
The issue with this is there are two ways to draw the gradient, option 1 is how you have it where the gradient is stretched to the length of the bar. Option 2 is to have the gradient fixed, and the longer the bar, the more of the gradient is revealed. I'm not sure which is better. |
This also restricts the gradient, what if you want it on a different angle? |
Note that I changed the branch to 4.0.0. You will need to rebase onto 4.0.0 branch |
When this will be merged and the new version released?? |
Done |
Hey @alekzuz , i tried to use your fork because i need gradient support for bar charts (pod 'Charts' , :git => 'https://github.com/alekzuz/Charts.git', :branch => 'bar-gradient' ) and it does not compile. Any idea? |
Hello @Lukafin , check you build scheme, it seems that you selected wrong one |
@alekzuz Build scheme is for app that uses your fork of Charts lib. I dont have any other schemes. |
@Lukafin Take a look at the demo project, it works fine. May be you have to clean your project, I do not know. |
@alekzuz Now checking your demo project and it works there. |
@@ -15,6 +15,13 @@ import CoreGraphics | |||
|
|||
open class BarChartDataSet: BarLineScatterCandleBubbleChartDataSet, BarChartDataSetProtocol | |||
{ | |||
@objc | |||
public enum BarGradientOrientation: Int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense to nest this in BarChartDataSet
/// This prevents out-of-bounds by performing a modulus on the gradient color index, so colours will repeat themselves. | ||
open func barGradientColor(atIndex index: Int) -> [NSUIColor]? | ||
{ | ||
guard let gradientColors = barGradientColors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one line
@@ -149,6 +156,26 @@ open class BarChartDataSet: BarLineScatterCandleBubbleChartDataSet, BarChartData | |||
/// the alpha value (transparency) that is used for drawing the highlight indicator bar. min = 0.0 (fully transparent), max = 1.0 (fully opaque) | |||
open var highlightAlpha = CGFloat(120.0 / 255.0) | |||
|
|||
/// array of gradient colors [[color1, color2], [color3, color4]] | |||
open var barGradientColors: [[NSUIColor]]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a nested array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested array is array of colors for each bar.
|
||
/// - returns: The gradient colors at the given index of the DataSet's gradient color array. | ||
/// This prevents out-of-bounds by performing a modulus on the gradient color index, so colours will repeat themselves. | ||
open func barGradientColor(atIndex index: Int) -> [NSUIColor]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swift naming convention would be func barGradientColor(at index: Int) -> [NSUIColor]?
else { return nil } | ||
|
||
var index = index | ||
if index < 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't necessary, it is already common place in Swift and Obj-C that collections require a positive index. Array
doesn't perform these checks, we don't need to either.
open func drawBar(context: CGContext, dataSet: BarChartDataSetProtocol, index: Int, barRect: CGRect) | ||
{ | ||
context.saveGState() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defer { context.restoreGState() }
@objc open class func gradients () -> [[NSUIColor]] | ||
{ | ||
return [ | ||
[NSUIColor(red: 146/255.0, green: 132/255.0, blue: 240/255.0, alpha: 1.0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did these colours come from? If they are from other templates, you should directly call them instead of redefining them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's custom colours especially for the gradients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but where did the values for the colours come from? We already have a list of themes in ChartColorTemplates
. If this adds to that, it should be done there. It probably should just use the already existing theme colours though.
Source/Charts/Utils/Platform.swift
Outdated
@@ -17,6 +17,7 @@ types are aliased to either their UI* implementation (on iOS) or their NS* imple | |||
public typealias NSUIGestureRecognizerDelegate = UIGestureRecognizerDelegate | |||
public typealias NSUITapGestureRecognizer = UITapGestureRecognizer | |||
public typealias NSUIPanGestureRecognizer = UIPanGestureRecognizer | |||
public typealias NSUIBezierPath = UIBezierPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed.
Source/Charts/Utils/Platform.swift
Outdated
@@ -229,6 +230,7 @@ types are aliased to either their UI* implementation (on iOS) or their NS* imple | |||
public typealias NSUIPinchGestureRecognizer = NSMagnificationGestureRecognizer | |||
public typealias NSUIRotationGestureRecognizer = NSRotationGestureRecognizer | |||
public typealias NSUIScreen = NSScreen | |||
public typealias NSUIBezierPath = NSBezierPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed.
Source/Charts/Utils/Platform.swift
Outdated
@@ -610,4 +612,33 @@ types are aliased to either their UI* implementation (on iOS) or their NS* imple | |||
return NSUIScreen.main | |||
} | |||
|
|||
extension NSBezierPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this only for OSX/AppKit instead of universal with UIKit?
Hey @Lukafin now i have the same errors, how did you fix them? thanks! |
@alekzuz pardon where can i get your demo? |
any update? I saw it didn't passed the CI for tvOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the horizontal bar charts, did you forget to implement the gradient?
is there a way to use the radial gradient with the bubble chart?
Hello there, I would appreciate this feature being introduced in the Framework. Is there any way I can help with the process? (e.g.: Continue developing the feature and fix merge issues, or maybe with CC and tests) |
there is #3533 same as this feature. Is it possible you guys can work together to make this one PR? |
I will try to finish bar gradients issues the next few days |
# Conflicts: # Source/Charts/Renderers/BarChartRenderer.swift # Source/Charts/Renderers/HorizontalBarChartRenderer.swift
thanks |
Can I set rounded corners ? |
Added gradients for the vertical bars #2722 #1065
Usage:
At the same time made minor refactoring: removed
isSingleColor
in BarChartRenderer.Test case: iPhone 6s, iOS 11.1, 100 xValues, 100 yValues, single color.
Average execution time of the
drawDataSet
of 10 attempts:0.00160739421844482 seconds with
isSingleColor
0.00167040824890137 seconds without