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

The LineChart widget now supports zooming the content #133

Merged
merged 11 commits into from
Feb 18, 2019
Merged

Conversation

mum4k
Copy link
Owner

@mum4k mum4k commented Feb 18, 2019

Fixes #55

@coveralls
Copy link

coveralls commented Feb 18, 2019

Pull Request Test Coverage Report for Build 776

  • 380 of 416 (91.35%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 90.72%

Changes Missing Coverage Covered Lines Changed/Added Lines %
canvas/braille/braille.go 28 32 87.5%
widgets/linechart/linechart.go 26 34 76.47%
widgets/linechart/zoom/zoom.go 262 286 91.61%
Totals Coverage Status
Change from base Build 772: 0.1%
Covered Lines: 5406
Relevant Lines: 5959

💛 - Coveralls

@mum4k mum4k merged commit 2ab7083 into devel Feb 18, 2019
@mum4k mum4k deleted the linechart-zoom branch February 18, 2019 07:33
@keithknott26
Copy link

keithknott26 commented Feb 18, 2019

@mum4k,
This looks great!!!
I confirmed it works on both a Trackpad and Mouse w/ scroll wheel. Clicking anywhere in the linegraph box zooms 100% even if the Linegraph doesn't have "focus" indicated by the yellow border.

One very minor thing I noticed was that when you don't apply the XUnscaled option and continually fill the Series with records, scrolling will eventually start, for me it happens at about 1450 records in the Series. I'm sure you put this in as a feature but wanted to mention it just in case you didnt.

The other very minor thing I noticed was under that same scenario (XUnscaled option Disabled) and after scrolling starts, you can see the left most label changing to be the right most label. (1:11 vs 00:35)

I've included a screenshot of the behavior, I can attach a screen recording if you like. The interesting thing is it doesn't happen each time the graph is scrolled to the left, it happens every 1 out of 100 or so notches and ONLY happens when the graph is completely zoomed out. Almost like when the function that handles the scrolling might have a bug or its related the one you already identified #132

image

Edit: My screen recording provider isn't letting me upload the recording for some reason, I'll try again tomorrow morning.

@mum4k
Copy link
Owner Author

mum4k commented Feb 18, 2019

Thanks for the detailed testing @keithknott26, very helpful.

Note that the focus only applies to keyboard events, i.e. the infrastructure directs the keyboard events to the focused widget only. Mouse events don't have the same restriction because we know the coordinates of the mouse click.

Thinking about the zoom too 100% on a single click made me realize how misleading this can be. The intention is to make the user select a range (click and drag, then release). The fact that it zooms after a single click (single row highlighted) can be confusing. I have filed #135 and will fix it next.

It wasn't my intention to allow the graph to scroll when XAxisUnscaled isn't set. I have tried to reproduce this, but even after pushing 3000+ values, the X axis is still firmly based at zero. I waited until 5000+ and it still didn't happen.

image

However I believe this is related to zooming, see #136. I was also able to reproduce the behavior of the first label being misplaced and believe it is related to #136.

@mum4k
Copy link
Owner Author

mum4k commented Feb 19, 2019

With both #135 and #136 resolved, this should all behave better.

Both fixes pushed to the devel branch.

@keithknott26
Copy link

#135 and #136 confirmed through testing. I'm not able to find any other issues - I'll research a bit more and see if that bug where the line starts scrolling is on my end.

@mum4k
Copy link
Owner Author

mum4k commented Feb 19, 2019

Great, fyi I found one more bug (#140) which shows itself when we try to zoom into a linechart that rolls (and keep it zoomed until the zoomed portion rolls out of the view).

The fix is already on its way to devel.

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

Successfully merging this pull request may close these issues.

None yet

4 participants