Url params 🎉 #276

tech4GT · 2018-06-02T12:50:29Z

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

tech4GT · 2018-06-02T12:56:23Z

@jywarren One issue I am facing right now is that importimage takes a function which also has commas and () should I handle it differently, or is there a better approach??

tech4GT · 2018-06-02T12:59:32Z

Oh I found one more logical error, now reworking .toJson()

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

tech4GT · 2018-06-02T13:22:42Z

@jywarren everything is working now!!!

jywarren · 2018-06-02T15:38:45Z

Oh wow! Did you figure out the comma issue? I had thought of that and wasn't sure what to do about it.

…

On Sat, Jun 2, 2018, 7:23 AM Varun Gupta ***@***.***> wrote: @jywarren <https://github.com/jywarren> everything is working now!!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#276 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJztY6gikGA9uYvf4fUHpZXwk0RlSks5t4pGngaJpZM4UXrUD> .

tech4GT · 2018-06-02T17:37:16Z

@jyywarren yes i did😁

jywarren

This looks really nice. How did the function-with-commas question get resolved -- what happens?

Awesome work, Varun!

jywarren · 2018-06-02T18:10:37Z

examples/lib/defaultHtmlSequencerUi.js

-        _sequencer.addSteps(step);
+      var stepsFromHash = _sequencer.toJson(hash);
+      stepsFromHash.forEach(function eachStep(stepObj) {
+        _sequencer.addSteps(stepObj.name,stepObj.options);


great, this would eventually be replaced by importString or what were we going to call it?

@jywarren yeah We can call it importString()

@jywarren actually I split on '),' !!

Hmm, I wonder if this could still be triggered by incorrect parsing with a specifically crafted function.

Maybe we should forbid functions unless it's a full json string? Or could we url-encode the function like with %20-like encoding? But that would be quite unreadable.

@jywarren this won't fail until the function is incorrect!!

In which case even if we encode it won't make sense since it would not be valid

jywarren · 2018-06-02T18:11:12Z

examples/lib/defaultHtmlSequencerUi.js

      });
      _sequencer.run();
    }
+    setUrlHashParameter("steps", sequencer.toString());


awesome :-)

jywarren · 2018-06-02T18:12:11Z

examples/lib/defaultHtmlSequencerUi.js

+    .run(null);
+
+    // add to URL hash too
+    setUrlHashParameter("steps", _sequencer.toString());


And we're sure this'll overwrite the previous one? Great.

jywarren · 2018-06-02T18:14:15Z

src/ImageSequencer.js

+    return steps.map(stepToJson);
+  }
+  // Converts one stringified step into JSON
+  function stepToJson(str){


Maybe stepStringToJson()? for extra clarity

And can we write a test for this function? Thank you!!!

@jywarren yeah I'l add the tests now!!

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

tech4GT · 2018-06-02T19:09:34Z

@jywarren I think this is ready!!

tech4GT · 2018-06-03T01:22:45Z

Oh yeah sure!!

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

tech4GT · 2018-06-03T04:17:35Z

Oh my bad!! I'm sorry @jywarren I forgot to push the commit that fixed the comma problem😅

tech4GT · 2018-06-03T04:18:07Z

@jywarren now this is done with the updated test, please have a look!! I tried it in the browser, looking very cool!!!

jywarren

This is almost there -- very exciting! Just made a suggestion with these encode/decode functions that'll make things simpler.

Thanks Varun!!!

jywarren · 2018-06-03T14:26:22Z

src/ImageSequencer.js

-    for(let input in inputs) inputs[input] = step.options[input] || inputs[input].default;
+    for(let input in inputs) {
+      inputs[input] = step.options[input] || inputs[input].default;
+      inputs[input] = inputs[input].toString().replace(',',',%20');


Try this -- encodeURIComponent() -- this is available in node too. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent

jywarren · 2018-06-03T14:28:33Z

src/ImageSequencer.js

@@ -232,9 +235,12 @@ ImageSequencer = function ImageSequencer(options) {
      str.substr(0,str.indexOf('(')),
      str.substr(str.indexOf('(')+1)
    ]
-
+    str[1] = str[1].split('%20').join(' ');


Would this then be decodeURIComponent() to reconstruct it? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent

I had mistakenly put %20 which is the code for a space, not a comma. I think commas are %2C but these converter functions should deal with it.

jywarren · 2018-06-03T14:29:04Z

src/ImageSequencer.js

@@ -232,9 +235,12 @@ ImageSequencer = function ImageSequencer(options) {
      str.substr(0,str.indexOf('(')),
      str.substr(str.indexOf('(')+1)
    ]
-
+    str[1] = str[1].split('%20').join(' ');
+    str[1] = str[1].split(', ').join('|');


Here are you replacing commas with | pipes? I think the encode/decode should make that unnecessary, right?

tech4GT · 2018-06-03T14:35:04Z

@jywarren Actually yeah I think this can simplify things a little, I'll try this out

jywarren · 2018-06-03T14:47:48Z

oops i was writing a long response while you were reading!

…

On Sun, Jun 3, 2018 at 8:35 AM, Varun Gupta ***@***.***> wrote: @jywarren <https://github.com/jywarren> Actually yeah I think this can simplify things a little, I'll try this out — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#276 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ31i7tvpB10a-VGvH-ScexC5L9PDks5t4_QYgaJpZM4UXrUD> .

tech4GT · 2018-06-03T14:53:17Z

@jywarren this is somewhat messing with the functionality

tech4GT · 2018-06-03T14:53:46Z

I would have to figure this out from scratch i guess

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

tech4GT · 2018-06-03T15:28:44Z

@jywarren Done!!

jywarren · 2018-06-03T16:11:24Z

AMAZING!!!!!!!!

tech4GT added 2 commits June 2, 2018 13:39

basic implementation

25b271f

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

fix bug

2c03c86

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

ghost assigned tech4GT Jun 2, 2018

ghost added the in-progress label Jun 2, 2018

fix everything

9b87a7b

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

jywarren reviewed Jun 2, 2018

View reviewed changes

tech4GT added 2 commits June 3, 2018 00:37

add test and refactor

ae21bb8

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

update demo

739de94

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

tech4GT added 2 commits June 3, 2018 09:22

fix comma issue

f2da9af

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

update test

75e5245

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

jywarren requested changes Jun 3, 2018

View reviewed changes

now doing with native encode

890c951

Signed-off-by: tech4GT <varun.gupta1798@gmail.com>

jywarren approved these changes Jun 3, 2018

View reviewed changes

jywarren merged commit 871453b into publiclab:master Jun 3, 2018

ghost removed the in-progress label Jun 3, 2018

jywarren mentioned this pull request Jun 3, 2018

"Meta" modules -- modules made of other sequences of modules #200

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Url params 🎉 #276

Url params 🎉 #276

tech4GT commented Jun 2, 2018

tech4GT commented Jun 2, 2018

tech4GT commented Jun 2, 2018

tech4GT commented Jun 2, 2018

jywarren commented Jun 2, 2018 via email

tech4GT commented Jun 2, 2018

jywarren left a comment

jywarren Jun 2, 2018

tech4GT Jun 2, 2018

tech4GT Jun 2, 2018

jywarren Jun 2, 2018

tech4GT Jun 2, 2018

tech4GT Jun 2, 2018

jywarren Jun 2, 2018

jywarren Jun 2, 2018

tech4GT Jun 2, 2018

jywarren Jun 2, 2018

jywarren Jun 2, 2018

tech4GT Jun 2, 2018

tech4GT commented Jun 2, 2018

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

jywarren left a comment

jywarren Jun 3, 2018

jywarren Jun 3, 2018

jywarren Jun 3, 2018

tech4GT commented Jun 3, 2018

jywarren commented Jun 3, 2018 via email

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

jywarren commented Jun 3, 2018

Url params 🎉 #276

Url params 🎉 #276

Conversation

tech4GT commented Jun 2, 2018

tech4GT commented Jun 2, 2018

tech4GT commented Jun 2, 2018

tech4GT commented Jun 2, 2018

jywarren commented Jun 2, 2018 via email

tech4GT commented Jun 2, 2018

jywarren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tech4GT commented Jun 2, 2018

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

jywarren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tech4GT commented Jun 3, 2018

jywarren commented Jun 3, 2018 via email

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

tech4GT commented Jun 3, 2018

jywarren commented Jun 3, 2018