Crosshair plugin - out of graph bounds selection filter#1039
Merged
mirabilos merged 3 commits intoJan 11, 2025
Conversation
mirabilos
reviewed
Jan 11, 2025
Collaborator
|
On Mon, 13 Jan 2025, n-gist wrote:
@mirabilos yes, `.y` was missing there, I see you have it covered
OK, thanks.
Glad to see you have merged my PRs for crosshair plugin, so I can
remove my patches after releasing update.
You’re welcome.
Could you check my other two PRs for synchronizer plugin #1035 and
#1037? Second one was closed by mistake imo, because I included
incorrect code in the post that is not actually part of the final fix.
I had a look but it needed more headspace to review than I had,
I’ll look again when I next work on Dygraphs. (There’s something
that needs probably a lot of work: it uses an old version of
twitter-bootstrap that’s now unsupported and the Debian maintainers
of that ask me to upgrade, but I’d rather remove the use of that
entirely and add minimal CSS and (ideally no) JS directly to the
project so the documentation/site renders… similar enough to how
it currently looks. But I’m no frontend person…
I'm using dygraph with all these fixes combined for over a year in my
project and it is stable. I haven't found anything I'd like to fix or
add to this yet, other than fixing the animation when zooming out
vertically, but I haven't found the time to try to do that yet. It is
not so critical
Hmm, okay. I’ll comment on #1037 directly a bit, too.
I'm not sure how to inject current version into my project for testing,
it is installed through yarn with loading patched plugins from other
I don’t know yarn, but could you not simply do an alpha release
on your own namespace and then point packages.json or whatever
it uses to that?
I will find a way to actually test it if you decide whether to merge my
PRs for synchronizer or not, just to not do it twice.
Fair.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the graph is zoomed in, crosshair can select points out of view area, drawing lines off graph.
Below is example where the area on the left is caused by
axisLabelWidth, so it is inside crosshair drawing canvas.The same happens on the right side when using
rightGap. Filtered by checkingselectedPoint.xto be in [0,1]