Skip to content
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 value text rotation #2200

Merged
merged 6 commits into from
Jul 14, 2018
Merged

Added value text rotation #2200

merged 6 commits into from
Jul 14, 2018

Conversation

chinh-tran
Copy link
Contributor

@chinh-tran chinh-tran commented Feb 25, 2017

I've implemented the option to rotate value texts, which looks like below:
charts-rotated

Usage:

let ds = LineChartDataSet(values: yse1, label: "Hello")
ds.valueRotationAngle = 270.0

There is a new attribute in IChartDataSet:

var valueRotationAngle: CGFloat { get set }

and ChartBaseDataSet:

open var valueRotationAngle: CGFloat = CGFloat(0.0)

In order to rotate the values, I'm reusing the drawText function in ChartUtils, which is already being used to rotate the labels:

open func drawText(_ text: String, at point: CGPoint, anchor: CGPoint, angleRadians: CGFloat, attributes: [NSAttributedStringKey : Any]?)

@chinh-tran chinh-tran closed this Feb 25, 2017
@chinh-tran chinh-tran reopened this Feb 25, 2017
@codecov-io
Copy link

codecov-io commented Feb 25, 2017

Codecov Report

Merging #2200 into 4.0.0 will increase coverage by 0.2%.
The diff coverage is 48%.

Impacted file tree graph

@@            Coverage Diff            @@
##            4.0.0    #2200     +/-   ##
=========================================
+ Coverage   29.98%   30.18%   +0.2%     
=========================================
  Files         116      116             
  Lines       13197    13252     +55     
=========================================
+ Hits         3957     4000     +43     
- Misses       9240     9252     +12
Impacted Files Coverage Δ
...Charts/Data/Implementations/ChartBaseDataSet.swift 13.86% <ø> (ø) ⬆️
.../Charts/Renderers/HorizontalBarChartRenderer.swift 0% <0%> (ø) ⬆️
Source/Charts/Renderers/BubbleChartRenderer.swift 0% <0%> (ø) ⬆️
...ce/Charts/Renderers/CandleStickChartRenderer.swift 0% <0%> (ø) ⬆️
Source/Charts/Renderers/RadarChartRenderer.swift 0% <0%> (ø) ⬆️
Source/Charts/Renderers/ScatterChartRenderer.swift 0% <0%> (ø) ⬆️
Source/Charts/Renderers/LineChartRenderer.swift 56.34% <100%> (+0.11%) ⬆️
Source/Charts/Renderers/PieChartRenderer.swift 52.61% <30%> (-0.3%) ⬇️
Source/Charts/Renderers/BarChartRenderer.swift 53.58% <57.89%> (+0.16%) ⬆️
Source/Charts/Utils/ChartUtils.swift 60.2% <95%> (+12.86%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73183d5...aadc3a8. Read the comment docs.

@jjatie
Copy link
Collaborator

jjatie commented Jan 23, 2018

@chinh-tran are you able to rebase on master?

@chinh-tran
Copy link
Contributor Author

@jjatie yes, rebase done :)

@liuxuan30
Copy link
Member

liuxuan30 commented Feb 16, 2018

if this is a good feature to have, let's target 4.0 branch?
master branch currently won't accept any feature PRs until 3.1 is released.
You better switch to 4.0 branch for merging. Thanks you :)

@jjatie
Copy link
Collaborator

jjatie commented Feb 21, 2018

I think this is good for 4.0
@chinh-tran Thank you for your work. I'm sorry to ask this of you again, but can you please rebase on 4.0.0 and change the PR target to 4.0.0?
We can merge it in right away then and you won't have to keep updating the PR.

@jjatie jjatie added this to the 4.0.0 milestone Feb 21, 2018
@chinh-tran
Copy link
Contributor Author

Alright, I replayed the changes onto 4.0.0.

@@ -302,6 +302,9 @@ open class ChartBaseDataSet: NSObject, ChartDataSetProtocol
/// the font for the value-text labels
open var valueFont: NSUIFont = NSUIFont.systemFont(ofSize: 7.0)

/// the rotation angle for value-text labels
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to "The rotation angle (in degrees) for value-text labels

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change it in the protocol, you can delete the documentation comment here and Xcode will use the one from the protocol.

@@ -302,6 +302,9 @@ open class ChartBaseDataSet: NSObject, ChartDataSetProtocol
/// the font for the value-text labels
open var valueFont: NSUIFont = NSUIFont.systemFont(ofSize: 7.0)

/// the rotation angle for value-text labels
open var valueRotationAngle: CGFloat = CGFloat(0.0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valueLabelAngle

@@ -200,6 +200,9 @@ public protocol ChartDataSetProtocol
/// the font for the value-text labels
var valueFont: NSUIFont { get set }

/// the rotation angle for value-text labels
var valueRotationAngle: CGFloat { get set }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valueLabelAngle

@@ -209,6 +207,21 @@ extension CGContext {
NSUIGraphicsPopContext()
}

func getDrawPoint(text: String, point: CGPoint, align: NSTextAlignment, attributes: [NSAttributedStringKey : Any]?) -> CGPoint
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private func

{
// align left to center text with rotation
context.drawText(value, at: CGPoint(x: xPos, y: yPos), align: align, anchor: anchor,
angleRadians: angleRadians, attributes: [.font: font, .foregroundColor: color])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no line break please

@jjatie
Copy link
Collaborator

jjatie commented Mar 15, 2018

Ah! Sorry, I thought I had submitted this review ages ago. @chinh-tran please take a look

@chinh-tran
Copy link
Contributor Author

@jjatie It's updated now, however the CI build is failing but it's not related to this PR I guess.

@jjatie
Copy link
Collaborator

jjatie commented Mar 16, 2018

No, it’s my fault. I moved 4.0.0 to swift 4.1 and forgot to update the Travis script. It’s done in a different branch atm, but @liuxuan30 is busy this week. I’ll let you know when it’s merged.

@liuxuan30
Copy link
Member

@jjatie I'm seeing many PRs you updated and many conflicts. Is this caused by new commits on 4.0 branch?

@jjatie
Copy link
Collaborator

jjatie commented Mar 20, 2018

@liuxuan30 4.0.0 is updated for Swift 4.1, but I forgot to update the travis ci file. It's updated in the top ticket in the 4.0 project, so when that's merged everything will be good to go again.

@liuxuan30
Copy link
Member

Can you send me the link. I didn't see a similar one neither in More Swift nor 4.0 project

@pmairoldi pmairoldi merged commit 1d44ab3 into ChartsOrg:4.0.0 Jul 14, 2018
@yuldong
Copy link

yuldong commented Mar 25, 2019

would love to see in this feature in master,is there any plan on master to archive this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants