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

Add gradient bars for BarChart #3533

Closed
wants to merge 3 commits into from

Conversation

larryonoff
Copy link
Contributor

This PR adds functionality to draw gradient bars with BarChartDataSet

@codecov-io
Copy link

codecov-io commented Jun 25, 2018

Codecov Report

Merging #3533 into 4.0.0 will decrease coverage by 0.21%.
The diff coverage is 16.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##            4.0.0    #3533      +/-   ##
==========================================
- Coverage   31.49%   31.27%   -0.22%     
==========================================
  Files         112      112              
  Lines       10050    10127      +77     
==========================================
+ Hits         3165     3167       +2     
- Misses       6885     6960      +75
Impacted Files Coverage Δ
...ata/Implementations/Standard/BarChartDataSet.swift 67.44% <ø> (ø) ⬆️
Source/Charts/Utils/ChartUtils.swift 43.47% <0%> (-1.98%) ⬇️
Source/Charts/Renderers/BarChartRenderer.swift 47.22% <16.66%> (-5.49%) ⬇️

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 5689a1c...33d8e48. Read the comment docs.

return CGPoint(x: CGFloat.infinity, y: CGFloat.infinity)
}

var isIntinite: Bool {
Copy link

Choose a reason for hiding this comment

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

isInfinite typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed and rebased on branch/4.0.0

@malikmani
Copy link

I need this functionality in the bar charts. Can you please add this in master branch.

@liuxuan30
Copy link
Member

it takes time to review and merge. I'm sorry we are short handed, so either you take the code and merge on your side, or just wait. After all, it's open source.

Copy link

@malikmani malikmani left a comment

Choose a reason for hiding this comment

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

@larryonoff Does the gradients works for horizontal bars?

let set = BarChartDataSet(values: yVals, label: "The year 2017")
set.drawValuesEnabled = false

setup(set)

Choose a reason for hiding this comment

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

I think setup(set) can go out of this if-else condition.

@forbidden404
Copy link

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)

@larryonoff
Copy link
Contributor Author

Nobody from the team responds on this PR for a long time. So I still use Charts from my branch

@liuxuan30
Copy link
Member

sorry. but really busy for the team. I just saw my inbox there is another #3225 about bar gradient. would guys work together to see which is better?

@liuxuan30 liuxuan30 mentioned this pull request Mar 5, 2019
@larryonoff
Copy link
Contributor Author

I'm not sure, but this PR can be based on #3225. This was long time ago.

@liuxuan30
Copy link
Member

liuxuan30 commented Mar 6, 2019

is it possible that you guys can work together to merge into one great pull request?

@pylapp
Copy link

pylapp commented May 5, 2020

Hi!

Could you be nice to process this pull request?
It may be cool to define such background colors for data sets :)

Cheers

@liuxuan30
Copy link
Member

I hope so, but we need to decide which to merge, this or #3225.

@ChristianVinterly
Copy link

I think it's probably better to go with one and keep improving it than delay the decision. These 2 PRs have been ready for over a year now, and it would be great to get this functionality😊🙏

@liuxuan30
Copy link
Member

if that is the case, than probably we need to take a look at both PR and decide the better one

@muinmomin
Copy link

@liuxuan30 Any idea which PR will be getting merged in? This one or #3225? Would love to have one of them merged in soon is possible. Seems like they both have passed all the checks but this one doesn’t have any pending changes. Thanks for your help!

@liuxuan30
Copy link
Member

I will see if I can pull it off this or next week.

@liuxuan30 liuxuan30 self-assigned this Jul 8, 2020
@liuxuan30
Copy link
Member

liuxuan30 commented Jul 8, 2020

first thing, this is targeted to 4.0 branch? @jjatie do you remember why we add the two similar PRs to 4.0?

is it possible to rebased on master?

@larryonoff
Copy link
Contributor Author

I believe that I rebased it properly #4411

@liuxuan30
Copy link
Member

liuxuan30 commented Jul 13, 2020

so we could close this and move to #4411?

@larryonoff
Copy link
Contributor Author

@liuxuan30 lets close current PR

@larryonoff larryonoff closed this Jul 13, 2020
@liuxuan30 liuxuan30 reopened this Jul 27, 2020
@jjatie jjatie closed this Jan 26, 2021
@jjatie jjatie deleted the branch ChartsOrg:4.0.0 January 26, 2021 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants