Hareeshchandera / jsplumb

Automatically exported from code.google.com/p/jsplumb
0 stars 0 forks source link

Regressions with 'getInstance' #190

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There seem to be some serious regressions when using 'getInstance' in 
1.3.4/1.3.5; see http://jsfiddle.net/sx7Ak/11/ for a test case. No clue what's 
causing this unfortunately.

In our app, I don't seem to get those exact errors, but both the endpoints 
mentioned in the connection actually move to whatever <div> is being 
dragged/given focus.

Original issue reported on code.google.com by paul.uit...@gmail.com on 26 Jan 2012 at 10:43

GoogleCodeExporter commented 8 years ago
it was a weird closure thing.  i think its fixed in 1.3.5 now:

http://jsfiddle.net/sporritt/sx7Ak/32/

that jsfiddle points to a compiled version of the 1.3.5 dev version.

Original comment by simon.po...@gmail.com on 27 Jan 2012 at 1:36

GoogleCodeExporter commented 8 years ago
Alright, thanks! I immediately ran into the next error (also 'getInstance' 
related) though, when using a connection without setting up endPoints first: 
http://jsfiddle.net/sx7Ak/33/

Original comment by paul.uit...@gmail.com on 27 Jan 2012 at 11:00

GoogleCodeExporter commented 8 years ago
i think that one is fixed now too.

Original comment by simon.po...@gmail.com on 27 Jan 2012 at 11:33

GoogleCodeExporter commented 8 years ago
You're fast man, thanks a lot! I don't see a commit for either issue yet 
though, maybe they're not pushed to the server yet?

Original comment by paul.uit...@gmail.com on 27 Jan 2012 at 11:46

GoogleCodeExporter commented 8 years ago
yeah not pushed yet.  just got a few housekeeping things to do and i'll push 
later today.

Original comment by simon.po...@gmail.com on 27 Jan 2012 at 9:43

GoogleCodeExporter commented 8 years ago

Original comment by simon.po...@gmail.com on 28 Jan 2012 at 11:05

GoogleCodeExporter commented 8 years ago
changes pushed now.  i got an email from another guy with a couple of bits on 
this too, so it's all a lot more robust now.  i'm going to rejig the unit tests 
slightly in the next version to make it easy for me to run the entire test 
suite against an instance; hopefully that will prevent regressions of this sort 
in the future.

1.3.5 will be out very soon - in the next few days.

Original comment by simon.po...@gmail.com on 29 Jan 2012 at 8:38

GoogleCodeExporter commented 8 years ago

Original comment by simon.po...@gmail.com on 29 Jan 2012 at 8:25

GoogleCodeExporter commented 8 years ago
I've tried this version; it works a bit better, but I'm still seeing some very 
weird behavior (endpoints appearing at 0,0, connections all going to the 
top-left corner as well) that only occurs when using `getInstance`.

I'm having a hard time creating a test case though; can't figure out what part 
of the code is triggering it yet. Will post it when I get it..

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 11:29

GoogleCodeExporter commented 8 years ago
what library are you using - jquery?

when does this occur - using a programmatic connect, or when dragging a new 
connection, or when moving some element?

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 12:08

GoogleCodeExporter commented 8 years ago
..and are you setting a 'container' anywhere, like on the Defaults of the 
instance?

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 12:33

GoogleCodeExporter commented 8 years ago
Yep, jQuery. I don't set 'container' - the canvas elements do get appended to 
the right element.

It's quite similar to http://jsfiddle.net/Ep7yy/16/ - except that on the second 
tab, most endpoints won't render in the right location, but will end up with 
such (negative) offsets that they'll appear at the page's 0,0. However, some 
other endpoints do render properly. Really weird.

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 12:44

GoogleCodeExporter commented 8 years ago
And as soon a I move the element, the endpoints will snap to it's top-left 
corner

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 12:47

GoogleCodeExporter commented 8 years ago
but this doesnt happen if you're using the static jsPlumb instance?

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 12:51

GoogleCodeExporter commented 8 years ago
i just updated the qunit tests to take a jsPlumb instance as an argument, and 
when i ran them using jsPlumb.getInstance() I had some failures, which i have 
now fixed.  the script on the test site has been updated.

this is no guarantee that your problems are fixed of course.  but might be 
worth checking.

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 1:11

GoogleCodeExporter commented 8 years ago
Ah, I got it!

It's only when using 'getInstance'. It's always fine with the default instance. 
It looks like somehow, it's messing up the `id` assignment (id="jsPlumb_x"). It 
will first assign "jsPlumb_1" for the first instance. Then, you create a new 
instance, and it will start at "jsPlumb_1" all over again! Of course, this will 
backfire as soon as it tries to select an element by id in '_updateOffset' (var 
s = _getElementObject(elId);).

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 2:03

GoogleCodeExporter commented 8 years ago
ah.  ok i can fix that easy enough.  thanks for pursuing this - i'll put the 
fix in tomorrow and get 1.3.5 out the door shortly.

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 2:06

GoogleCodeExporter commented 8 years ago
Yeah, looks like you changed how ids are generated after 1.3.3. I can imagine 
using timestamps could give duplicates, but I can guarantee the current 
approach will give duplicates when using instances ;).

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 2:10

GoogleCodeExporter commented 8 years ago
Had a typo in the example btw - you should get the duplicate ids in 
http://jsfiddle.net/Ep7yy/17/ (other one was still svg), although it doesn't 
always seem to be a problem for some reason.

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 2:30

GoogleCodeExporter commented 8 years ago
it's not always a problem because that monotonically increasing integer is used 
for a variety of things - missing ids, connection ids, endpoint ids etc.  so it 
depends what you've done with each instance.

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 8:48

GoogleCodeExporter commented 8 years ago
ok fixed.  i've assigned a unique "instance index" to each created instance of 
jsplumb, and it forms part of any generated ids from that instance.  if you 
could try it and let me know how it goes that would be great.

Original comment by simon.po...@gmail.com on 30 Jan 2012 at 9:05

GoogleCodeExporter commented 8 years ago
Well, this seems to work pretty good, no more 'getInstance' bugs I think! I do 
have one more though, that I vaguely remember as something we've had before as 
well. I'll open a separate issue for that

Original comment by paul.uit...@gmail.com on 30 Jan 2012 at 9:59

GoogleCodeExporter commented 8 years ago

Original comment by simon.po...@gmail.com on 31 Jan 2012 at 11:42

GoogleCodeExporter commented 8 years ago
1.3.5 was released today.

Original comment by simon.po...@gmail.com on 1 Feb 2012 at 1:57