neuroanatomy / BrainBox

BrainBox is a web application that lets you annotate and segment 3D brain imaging data in real time, collaboratively.
https://brainbox.pasteur.fr
Other
96 stars 46 forks source link

allow the use of non-secure websockets #246

Closed ntraut closed 4 years ago

ntraut commented 4 years ago

Secure web-sockets are not easy to set up. I propose to preserve the support of non-secure web-sockets if not too difficult to maintain.


r03ert0 commented 4 years ago

other than that, it looks great! It'll be nice to have plain ws for local testing : )

ntraut commented 4 years ago

Do you mean for node or ESLint? For node it is perfectly ok:

> datadir = "./test/data/bar/foo"
'./test/data/bar/foo'
> datadir.split("/")[2]
'data'
> [,, datadir] = datadir.split("/");
[ '.', 'test', 'data', 'bar', 'foo' ]
> datadir
'data'

For ESLint I don't find their explanations very clear. They say not to do it for var foo = array[100];, that's obvious the code would look pretty ugly, but they don't say what to do for small numbers. If we keep datadir.split("/")[2] we have a warning. I saw the same issue for an index like 5, I did not touch but I don't know what to do to suppress the warning. Maybe we could do something like that: var {2: datadir} = datadir.split("/"); https://medium.com/the-andela-way/this-trick-will-help-you-de-structure-javascript-arrays-in-a-better-cleaner-way-a3bbc39ce226

r03ert0 commented 4 years ago

It seems jquery returns 'array-like' objects which are not really arrays, and array destructuring doesn't work in those cases. This is the example from the eslint website:

var $ = require('jquery');
var foo = $('body')[0];
var [bar] = $('body'); // fails with a TypeError

In all other cases when you are dealing with normal arrays, destructuring is :+1: :)

ntraut commented 4 years ago

Ok, I did not look at the right file so. I cannot reproduce the error and ESLint gives a warning with var foo = $('body')[0];. Maybe for more safety we could add Array.from: var [bar] = Array.from($('body'));?

r03ert0 commented 4 years ago

actually, the best would be maybe just to drop jquery. In vanilla JS there's two functions to query objects: document.querySelector("body") will query a single "body" element (and there should be only one, so that's what we want). For elements that may be more than once, there's document.queryAllSelectors("div"), for example, which gives an array (or array-like?) with all elements that match the selector (div in this case).

Could you try me.canvas = me.container.querySelector('canvas'); and see if it works?

it may not work if me.container is a jquery object...

if that's the case, jquery allow you to do me.container.find('canvas').get(0)... but I'd be very happy to get rid of all the jquery, little by little :p

thank you !!!

ntraut commented 4 years ago

I replaced some jquery, I hope I did not break anything :D