Problem with step object

publiclab / image-sequencer

A pure JavaScript sequential image processing system, inspired by storyboards

https://sequencer.publiclab.org

GNU General Public License v3.0

110 stars 208 forks source link

Problem with step object #1623

Open rishabhshuklax opened 4 years ago

rishabhshuklax commented 4 years ago

Please describe the problem (or idea)

Screenshot from 2020-02-12 22-54-18

Currently in the sequencer object there is a recursive loop of step inside options, we need to remove this!

Please show us where to look

https://beta.sequencer.publiclab.org

What's your PublicLab.org username?

rishabhshukla1999

Thank you!

Your help makes Public Lab better! We deeply appreciate your helping refine and improve this site.

To learn how to write really great issues, which increases the chances they'll be resolved, see:

https://publiclab.org/wiki/developers#Contributing+for+non-coders

YogeshSharma01 commented 4 years ago

Hi @blurry-x-face i want to claim this.

YogeshSharma01 commented 4 years ago

Hi @blurry-x-face i want to work on this if nobody is working on this ?

rishabhshuklax commented 4 years ago

Sure, go ahead!

YogeshSharma01 commented 4 years ago

Hi @blurry-x-face can you please explain the issue ? and what we exactly we need to do ?

harshkhandeparkar commented 4 years ago

The sequencer.steps array has multiple step objects. This is also the step object that is passed on to the defaultHtmlStepUi methods like onSetup and onComplete.

Each step object has an options object that stores the options passed on to the step from the UI. This options object has another step object which is identical to the original step object.

So each step object has recursive step objects. step > options > step > options > step > ..... and this continues for thousands of iterations.

We definitely don't want this. If you can fix it, it would be AWESOME.

YogeshSharma01 commented 4 years ago

@HarshKhandeparkar thanks! I'll try my best as its my first contribution after fto and I'll fix it.

sjbcastro commented 4 years ago

Hi @28-ys are you still working on this? Was thinking of having a look myself if you weren't looking at it any more. It's been a while since your last comment so I thought maybe you'd moved onto something else, just wanted to double check first.

harshkhandeparkar commented 4 years ago

Go ahead if you want :). The last comment is very old.

On Mon, 8 Jun 2020 at 14:58, sjbcastro notifications@github.com wrote:

Hi @28-ys https://github.com/28-ys are you still working on this? Was thinking of having a look myself if you weren't looking at it any more. It's been a while since your last comment so I thought maybe you'd moved onto something else, just wanted to double check first.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/image-sequencer/issues/1623#issuecomment-640486311, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIJI5H4E42AALIQZTFQMX5LRVSVLDANCNFSM4KT6FGCA .

sjbcastro commented 4 years ago

Cool! I've put in a pull request here: https://github.com/publiclab/image-sequencer/pull/1673

It's my first time contributing to publiclab. I've created a pull request from a fork rather than a feature branch - will this be a problem?

Screenshot below of the resulting behaviour after the fix:

harshkhandeparkar commented 4 years ago

Awesome! PRs are always opened from forks. Initially, using master branch is not a big issue. Ty :)