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

Fix lines not displaying when x-coordinates are unordered #3201

Closed

Conversation

barriserloth
Copy link

This commit fixes an issue brought up in #2567
(#2567), where
lines (and points) in LineCharts would not render correctly if the
x-values were not in order.

Let me know if this duplicates something that is already open, or if this is not a fix that users are interested in.

This commit fixes an issue brought up in ChartsOrg#2567
(ChartsOrg#2567), where
lines (and points) in LineCharts would not render correctly if the
x-values were not in order.
@jjatie
Copy link
Collaborator

jjatie commented Jan 22, 2018

@liuxuan30 I don't understand why we use the old logic that was there, are you able to shed any light on it?

@liuxuan30
Copy link
Member

liuxuan30 commented Jan 22, 2018

The old code seems starting from v2.x. And lowestVisibleX and the highest is changed after zooming.
I doubt this PR could work correctly when chart zoomed? @barriserloth
when zooming in, the lowest and highest vixible X values are a subset of the data set, so simply usingdataSet.entryCount shouldn't work.

This reminds me of the sorting, when zooming in, it needs to get the correct range of the current visible x values. In v2.x, x values are index based, so it's naturally sorted. If we could come up with another solution, maybe it won't need sort

@barriserloth
Copy link
Author

I hadn't tested it with zooming in, sorry about that. I can investigate a better fix, but please feel free to close the PR if you'd rather take a different approach to solving this @liuxuan30

@jjatie
Copy link
Collaborator

jjatie commented Jan 22, 2018

@barriserloth Please investigate further if you have time, and reopen this PR if you come up with a better solution.

@jjatie jjatie closed this Jan 22, 2018
@liuxuan30
Copy link
Member

liuxuan30 commented Jan 23, 2018

yeah, you are welcome on this, but sorting is not that simple to remove for the moment. If it's just about axis values, we could replace them with axis.entries index. maybe?

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@b0022f8). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3201   +/-   ##
=========================================
  Coverage          ?   22.25%           
=========================================
  Files             ?      116           
  Lines             ?    15593           
  Branches          ?      265           
=========================================
  Hits              ?     3471           
  Misses            ?    12089           
  Partials          ?       33
Impacted Files Coverage Δ
...Renderers/BarLineScatterCandleBubbleRenderer.swift 56.52% <100%> (ø)

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 b0022f8...45c8ecb. Read the comment docs.

@hungcaobss
Copy link

The line chart display correct now for unordered x-coordinates but can not highlight. Please support me on 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.

5 participants