Import Images from URL

[daemon1024]

Currently sequencer only allows to load image from file or use the camera to capture on the top of the page for the base image. We should add an option to load an image from URL too.

There's a import-image module as well but it also doesn't seem to load an image from a global url for some reason.

[vivek-30]

@daemon1024 ,sir can i work on this?

[vivek-30]

@daemon1024 i have fixed this issue .if you allow then can i raise a PR for this?

[daemon1024]

Hi @vivek-30 sorry I had been busy with some work. Sure you can work on this, I was hoping to discuss what the exact issue is here but if you have fixed it already might as well submit a PR we can look into it and discuss further there.
Thank You.

[vivek-30]

Hi @vivek-30 sorry I had been busy with some work. Sure you can work on this, I was hoping to discuss what the exact issue is here but if you have fixed it already might as well submit a PR we can look into it and discuss further there.
Thank You.

No big deal sir, i understand that you are busy . Also thanks for granting me the Permission.😊

[jywarren]

Hi @vivek-30 your solution looks great, we used to allow fetching remote images through the url via a simple sec= parameter, but maybe it no longer works? It may not have been well tested. But this method is more modular and structurally clear. Let's just be sure to add a good test for it!

[vivek-30]

@jywarren ,Sure sir.

[daemon1024]

I was hoping if we can add a import from url method on the top
Current:

What I wanted to propose:

cc @jywarren

[vivek-30]

@daemon1024 ,sir your proposal is interesting ,but it is a separate module in IS and should be remained it in that category. moreover due to CORS error we can't import most of the image so keeping on top will lead to a negative impact for the sequencer. Hope my point is clear. apologies in advance if i said something wrong.
BTW let us wait for views of @jywarren sir ,so whatever he will say we will proceed by same. Thanks 😊

[daemon1024]

Hi @vivek-30, your point seems valid, but it seems like the top part is a separate module kinda for now. Ref https://github.com/publiclab/image-sequencer/blob/3caa033c8febf5055c28f616b534933c00fcbe65/src/ui/SetInputStep.js

Maybe we can implement that here?
Thank You

Also it's fine if you just mention me or potentially everyone just with their usernames no need to append sir/maam :D

[vivek-30]

Hi @vivek-30, your point seems valid, but it seems like the top part is a separate module kinda for now. Ref https://github.com/publiclab/image-sequencer/blob/3caa033c8febf5055c28f616b534933c00fcbe65/src/ui/SetInputStep.js

Maybe we can implement that here?
Thank You

Also it's fine if you just mention me or potentially everyone just with their usernames no need to append sir/maam :D

ok @daemon1024 😅 . I will keep this in my mind . Lets hear what @jywarren will say on this. Also one more thing i wanted to ask : is your local repo is synced with remote one? bcoz it doesn't seems to be as you showed the outdated code.

[daemon1024]

@vivek-30 My bad for referencing an older commit. https://github.com/publiclab/image-sequencer/blob/main/src/ui/SetInputStep.js This should be it? The point remains the same I guess 😅

[vivek-30]

yes . you are right the point is still same.

[jywarren]

Hi all, we're starting to get into this discussion from early in project history of how the initial step is or isn't similar to a module itself... We had discussed a kind of "starter module" vs the code being part of the demo UI in #82 Also we had a discussion of using the "uppy" package for the initial module, to standardize the ways of initiating the sequence... #212 These could be good background for the discussion you're having now, what do you think?

…

On Sat, Feb 20, 2021, 7:47 AM Vivek Singh ***@***.***> wrote: yes . you are right the point is still same. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1808 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J6ZYT76G3TJ3776KS3S76VNZANCNFSM4XNBH7LA> .

[daemon1024]

"uppy" seems to be the exact thing I was trying to propose here, Maybe we can reopen it and try to work on it again 🤔

[vivek-30]

yes we can work on it but do remember that the sole purpose of this is not only to add a extra field for import-image. what @jywarren is thinking of is to merge load-image starter module with initial UI as you can see here #212 (comment)

[vivek-30]

Also it will require some more adjustments for drag-and-drop , capture a new picture and to display load-image as a starting stuff which is currently implemented.

[vivek-30]

@jywarren , @daemon1024 what's your views on this?

[jywarren]

I wonder, if we are able to incorporate uppy into one of the two modules, perhaps that's a good first step in merging the modules. This would take some thinking about the abstractions -- is it possible to have a module or a sub-module which can act as a regular module but also provide the UI and services of a starter module? It's a little complex. What are the lines of code we'd have to change to get them to use the same interface to this special module?

[vivek-30]

I wonder, if we are able to incorporate uppy into one of the two modules, perhaps that's a good first step in merging the modules. This would take some thinking about the abstractions -- is it possible to have a module or a sub-module which can act as a regular module but also provide the UI and services of a starter module? It's a little complex. What are the lines of code we'd have to change to get them to use the same interface to this special module?

@jywarren sir, you are right and we should first attempt to implement this on the topmost region. as it is main crux for the merging.
once we are successful in planning and implementing it for that region .we can plan accordingly for bottom part as well.

[vivek-30]

i am too busy to try this for now . but may be @daemon1024 can achieve this 😊

[jywarren]

I agree this is a bigger project, thank you!!!

…

On Sat, Feb 27, 2021 at 12:34 PM Vivek Singh ***@***.***> wrote: i am too busy to try this for now . but may be @daemon1024 <https://github.com/daemon1024> can achieve this 😊 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1808 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J3F7XGKW6DZ3EGCUD3TBEUKPANCNFSM4XNBH7LA> .