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

Rounded bars in bar charts #4375

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Pearapps
Copy link

@Pearapps Pearapps commented May 22, 2020

Issue Link 🔗

Other PR: #3754

I understand that there are other attempts at this, but I am working on a project that requires this - so I forked this and gave it a shot with the latest master.

Goals ⚽

Provide ability to round bar charts for charts that contain only positive values.

Implementation Details 🚧

Pretty straightforward - provides a property to set the multiplier you wish to drive the corner radius - and the renderer searches for the correct bar segment to apply that to, and draws it with a path.

@Pearapps Pearapps changed the title Rounded bar charts. Rounded bars in bar charts May 22, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 22, 2020

Codecov Report

Merging #4375 into master will decrease coverage by 0.06%.
The diff coverage is 23.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4375      +/-   ##
==========================================
- Coverage   40.81%   40.75%   -0.07%     
==========================================
  Files         124      124              
  Lines        9490     9519      +29     
==========================================
+ Hits         3873     3879       +6     
- Misses       5617     5640      +23     
Impacted Files Coverage Δ
...ata/Implementations/Standard/BarChartDataSet.swift 72.09% <ø> (ø)
Source/Charts/Renderers/BarChartRenderer.swift 71.04% <23.33%> (-2.99%) ⬇️

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 bf3080f...83be0c4. Read the comment docs.

@liuxuan30
Copy link
Member

liuxuan30 commented May 27, 2020

any reason/highlight you send this PR to us?

@Pearapps
Copy link
Author

Pearapps commented May 27, 2020

any reason/highlight you send this PR to us?

Didn't seem like there was any movement on the existing efforts, and people really want and need this (including myself) - and I wanted to provide this work for you all to use or iterate on.

@amiantos
Copy link

amiantos commented Jun 5, 2020

Do you have an example of how this is supposed to work? Any value I pass to roundRadiusWidthMultiplier seems to result in a crash. I am using a stacked bar chart.

@Pearapps
Copy link
Author

Pearapps commented Jun 5, 2020

Do you have an example of how this is supposed to work? Any value I pass to roundRadiusWidthMultiplier seems to result in a crash. I am using a stacked bar chart.

What values are you passing in? And what is the crash?

Here is what I am setting it to: 1.0 / 4.0

@amiantos
Copy link

amiantos commented Jun 6, 2020

I tried 1.0, 0.2, 0.5..

I don't really know what the crash is. Seems to happen no matter what value I put in there.

image

image

@liuxuan30
Copy link
Member

for rounded bar, I think the best PR so far is #3754. However the author not responding, and we really are also busy with our jobs... so, if you think you want to contribute, I guess you will take some from #3754?

The main problem for rounded bar is, when the bar is thinner, the rounded bar will look like an oval, which is not ideal. #3754 said she/he solved it. if you want to take it from here, if you can solve this, we'd happy to review

@4np
Copy link
Contributor

4np commented Dec 15, 2020

This would be easy to accomplish by providing your custom renderer, if #4297 was merged.

@4np 4np mentioned this pull request Jan 29, 2021
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.

5 participants