Skip to content

move event handlers into plottables#238

Closed
StendProg wants to merge 7 commits intoScottPlot:masterfrom
StendProg:AxSpanDragEvents
Closed

move event handlers into plottables#238
StendProg wants to merge 7 commits intoScottPlot:masterfrom
StendProg:AxSpanDragEvents

Conversation

@StendProg
Copy link
Contributor

@StendProg StendProg commented Feb 6, 2020

Add PositionDragged event to PlottableAxLine.
Add Position1Dragged and Position2Dragged events to PlottableAxSpan.
This events simplify binding UI elements to Draggable objects.
Usage example:

var plot = formsPlot.plt.PlotVSpan(...)
plot.Position1Dragged += (sender, arg) => form1.TextBox1.Text = arg.ToString();
plot.Position2Dragged += (sender, arg) => form1.TextBox2.Text = arg.ToString();

Additionaly this PR force fix lag while dragging to master branch from dev/4.1 branch c7636a5

@swharden
Copy link
Member

swharden commented Feb 6, 2020

Thanks @StendProg, this definitely makes it easier to write UI code which responds to drag and drop events.

What do you think about adding an EventHandler to the IDraggable interface?

@StendProg
Copy link
Contributor Author

I thinked about this. But it actually different, 1 event for axLine, 2 for axSpans and nothing more.
So i prefered keep it simple. When more draggable classes comes, it would be possible to refactor and generalize that.

@swharden swharden changed the title Plottable drag events move event handlers into plottables Feb 7, 2020
@swharden
Copy link
Member

swharden commented Feb 7, 2020

@StendProg I'm so happy you made this suggestion! I haven't had have much experience with event handlers, and now that I've worked with this PR I have a much better understanding of them.

It seems I could put event handlers in the plottables (or even in the mouse tracker) rather than handling them in the user control (which I find very ugly):

https://github.com/swharden/ScottPlot/blob/f9d0e4630ecdd48813dbc2275e4cab617c43ce3d/src/ScottPlot.WinForms/FormsPlot.cs#L352-L367

I'll work on this a bit more and merge it in shortly (without modifying the existing control), but I am now adding a redesign of event handling as a goal in version 4.1

@swharden
Copy link
Member

swharden commented Feb 7, 2020

I think I simplified the code by not passing data in events.

End users can do something like this:

var vspan = formsPlot1.plt.PlotVSpan(x1, x2, draggable: true); 
vspan.DraggedPosition1 += OnDragAction1;
vspan.DraggedPosition2 += OnDragAction2; 

This makes it easy to use the same action function for both positions if you want, or use separate functions. Positions (actually all plottable properties) are easy to access from there:

void OnDragAction1(object sender, EventArgs e)
{
    var axSpan = (PlottableAxSpan)sender;
    Console.WriteLine($"Adjusted span position 1 to: {axSpan.position1}");
}

void OnDragAction2(object sender, EventArgs e)
{
    var axSpan = (PlottableAxSpan)sender;
    Console.WriteLine($"Adjusted span position 2 to: {axSpan.position2}");
}

@StendProg what do you think?

@StendProg
Copy link
Contributor Author

StendProg commented Feb 7, 2020

I think I simplified the code by not passing data in events.

Passing data is the main porpouse of EventArg in standart EventHandler pattern. Generic version make this look short and simple.
Casting sender to specific class brings unnesessary dependency in user code. There no possible to write single line lambda handler. You need at least 2, this means additional 2 for {} brackets.
Passing value as arg is the main target of this PR.

@swharden
Copy link
Member

swharden commented Feb 7, 2020

@StendProg your points make a lot of sense!

I'd like to take a few days to study this topic in detail, then return to this PR better prepared.

You're probably correct that using arguments is the best way to proceed ... so I will read more about event handling architecture and hopefully come back with a better understanding of how to proceed.

Thanks again for starting this PR! I am excited for the direction this is going

to reduce merge conflict
@swharden
Copy link
Member

I'm closing this PR because it's too hard to resolve the merge conflict (axis lines and spans are now in 4 files), but I'll track progress toward improving use of events throughout the code base in #266

Thanks for your work here @StendProg! I've been reading a lot about events since you first opened this PR and I look forward to improving how ScottPlot handles events.

@swharden swharden closed this Feb 20, 2020
@StendProg StendProg deleted the AxSpanDragEvents branch March 16, 2020 14:00
@StendProg StendProg mentioned this pull request Oct 19, 2020
swharden added a commit that referenced this pull request Dec 19, 2020
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.

2 participants