Notify Users for the failure in loading Image via external URL #1813

vivek-30 · 2021-02-16T14:48:00Z

Fixes #1808.
Now image can be rendered through URL as well.

gitpod-io · 2021-02-16T14:48:02Z

vivek-30 · 2021-02-16T14:48:41Z

Working Clip -

Screen.Recording.2021-02-16.at.8.05.58.PM.mov

vivek-30 · 2021-02-16T14:52:27Z

@jywarren , @harshkhandeparkar , @daemon1024 please have a look 😊

jywarren · 2021-02-16T15:01:52Z

examples/lib/defaultHtmlStepUi.js

@@ -57,7 +57,7 @@ function DefaultHtmlStepUi(_sequencer, options) {
                <div class="col-md-8 cal collapse in step-column">\
                  <div class="load load-spin" style="display:none;"><i class="fa fa-circle-o-notch fa-spin"></i></div>\
                  <div class="step-image">\
-                    <a class="cal collapse in"><img class="img-thumbnail step-thumbnail"/></a>\
+                    <a class="cal collapse in"><img class="img-thumbnail step-thumbnail" crossorigin="anonymous" /></a>\


Was this the only thing missing? It was working before but maybe browsers increased security requirements? Wow!

Do you think we can still run a test for this? We could pull in a remote image from a static source and use looks-same to compare it.

Ok sir i will try to run this one.
BTW i have one more approach to tackle this problem. We can create a local directory and once we place a url to load the image we can use that url to download the image in that directory and can render image directly from local folder this will remove the overhead of cross-origin.
What's your view?

Actually previously this was the error in the console when image failed to load-

Uncaught Security Error: Failed to execute ‘toDataURL’ on ‘HTMLCanvasElement’: tainted canvases may not be exported.

But after setting crossorigin=anonymous this error disappeared.

jywarren · 2021-02-16T16:38:07Z

That's great regarding the cross-origin error you caught. Thanks! I think we'd best preserve this because people may want to begin with an image from any other online source. The test will confirm that it is able to fetch it!

…

On Tue, Feb 16, 2021 at 11:13 AM Vivek Singh ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In examples/lib/defaultHtmlStepUi.js <#1813 (comment)> : > @@ -57,7 +57,7 @@ function DefaultHtmlStepUi(_sequencer, options) { <div class="col-md-8 cal collapse in step-column">\ <div class="load load-spin" style="display:none;"><i class="fa fa-circle-o-notch fa-spin"></i></div>\ <div class="step-image">\ - <a class="cal collapse in"><img class="img-thumbnail step-thumbnail"/></a>\ + <a class="cal collapse in"><img class="img-thumbnail step-thumbnail" crossorigin="anonymous" /></a>\ Actually previously this was the error in the console when image failed to load- Uncaught Security Error: Failed to execute ‘toDataURL’ on ‘HTMLCanvasElement’: tainted canvases may not be But after setting crossorigin=anonymous this error disappeared. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1813 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J5SHRVOOAFZCKPSKIDS7KKQ5ANCNFSM4XWSQZTQ> .

vivek-30 · 2021-02-17T15:16:47Z

@jywarren i think the problem is still unresolved 😥 .while writing the test i tried one more time to import a image but it failed
and reason is pretty clear and obvious (CORS Error) -

vivek-30 · 2021-02-17T15:27:00Z

Possible solutions -

use cors-anywhere proxy
create our own proxy

for handling of these requests.

But sir if you have some other approach for this please let me know 😊

jywarren · 2021-02-17T17:30:37Z

Ah, i see - well, for servers that have CORS permissions set, it's OK. But is it possible to detect the CORS error and to report an error on the screen for the user that links to a CORS explanation? I don't think we should build in a proxy as part of our core code, but we could at least inform the user of the limitations. How does that sound?

…

On Wed, Feb 17, 2021 at 10:27 AM Vivek Singh ***@***.***> wrote: Possible solutions - 1. use cors-anywhere <https://github.com/Rob--W/cors-anywhere/#documentation> proxy 2. create our own proxy for handling of these requests. But sir if you have some other approach for this please let me know 😊 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1813 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J7E3B2A52XBFYVJDSDS7PN5ZANCNFSM4XWSQZTQ> .

vivek-30 · 2021-02-17T17:37:57Z

sure sir, that sounds great!!!
I will find a way to accomplish this ☺️

jywarren · 2021-02-17T17:44:03Z

sounds great! And for the user message, you could consider using this: https://getbootstrap.com/docs/4.4/components/toasts/ just an idea! Thank you!!

…

On Wed, Feb 17, 2021 at 12:38 PM Vivek Singh ***@***.***> wrote: sure sir, that sounds great!!! I will find a way to accomplish this ☺️ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1813 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J4XY2FDXVV4QCWG5ZTS7P5ILANCNFSM4XWSQZTQ> .

vivek-30 · 2021-02-17T17:51:07Z

That will look nice 😊 .
BTW something similar can be done for #1310 as well right?.

jywarren · 2021-02-17T18:00:43Z

That sounds right, yes!!!

…

On Wed, Feb 17, 2021 at 12:51 PM Vivek Singh ***@***.***> wrote: That will look nice 😊 . BTW something similar can be done for #1310 <#1310> as well right?. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1813 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J3UIQJFFOBP4JS6ZOTS7P6ZVANCNFSM4XWSQZTQ> .

vivek-30 · 2021-02-18T13:29:56Z

@jywarren finally after investing a decent amount of time on this i have completed this task successfully 😃 and tested it as well with both desktop and mobile view.
Hope you like this ☺️

jywarren · 2021-02-18T15:38:31Z

examples/demo.css

@@ -347,7 +347,18 @@ a.name-header{
 left: 10%;
 top: 30px;
 }
-#update-prompt-modal.show {
+
+#notify-box {


Wow cool, it looks like you created a custom alert box style, was there a reason why the bootstrap styles didn't work for you? Thanks!!!

There is no special reason for this .But the thing which took me to create a custom one is because toast is available in bootstrap version v4 and currently we have v3 so either we have to use cdn links or update it which i feel not to do so just for a notification prompt.
But it worth having this custom Toast. right?

@jywarren sir is it ok😊

OK, that makes sense! And, it looks nice, good work!

Let's do this: let's use a classname instead of an ID so it can be more easily reused, and let's also use the same classname as the Bootstrap 4 code -- that way if we upgrade in the future, it'll be a relatively easy change where we remove our custom style and switch over. How does that sound?

Thank you for your help on this!!!

That sounds perfect!! . And sir you are right we should make this code a little bit more general in nature for further reusability. i will change it now 😊 .

BTW i was just planning to provide you a visual for the changes i have made so that you can judge it better ,but before that you send this pick isn't it funny

… into image-url

vivek-30 · 2021-02-19T17:38:16Z

@jywarren please have a look at these changes now ☺️ .

jywarren · 2021-02-19T19:27:31Z

examples/demo.css

@@ -358,7 +358,17 @@ a.name-header{
  background:#c3c3c3;
 }

-#update-prompt-modal.show,#notify-box {
+/* Bootstrap class for display none remove it after updating to version v4 */


very nicely done!

jywarren · 2021-02-19T19:29:49Z

Nicely done and many thanks!!!!

vivek-30 · 2021-02-20T02:19:25Z

@jywarren Thank you so much sir ☺️

Load image via URL

d4b7339

vivek-30 requested a review from a team as a code owner February 16, 2021 14:48

jywarren reviewed Feb 16, 2021

View reviewed changes

notify users for image load failue due to CORS error

9d82d16

vivek-30 force-pushed the image-url branch from ff86747 to 9d82d16 Compare February 18, 2021 10:18

vivek-30 added 2 commits February 18, 2021 18:39

added a check for notifybox

70df3cb

added some more checks

4ab5be2

vivek-30 changed the title ~~Load image via URL~~ Notify Users for the failure in loading Image via external URL Feb 18, 2021

jywarren reviewed Feb 18, 2021

View reviewed changes

jywarren and others added 3 commits February 19, 2021 11:17

Merge branch 'main' into image-url

40d74eb

added bootstrap version v4 classes

7d50715

Merge branch 'image-url' of https://github.com/vivek-30/image-sequencer…

fe644c6

… into image-url

jywarren reviewed Feb 19, 2021

View reviewed changes

jywarren approved these changes Feb 19, 2021

View reviewed changes

jywarren merged commit 3b85779 into publiclab:main Feb 19, 2021

vivek-30 deleted the image-url branch February 20, 2021 06:41

vivek-30 mentioned this pull request Mar 15, 2021

Discussion and Q&A of Outreachy + GSoC ideas 2021 publiclab/plots2#9257

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notify Users for the failure in loading Image via external URL #1813

Notify Users for the failure in loading Image via external URL #1813

vivek-30 commented Feb 16, 2021

gitpod-io bot commented Feb 16, 2021 •

edited

vivek-30 commented Feb 16, 2021

vivek-30 commented Feb 16, 2021

jywarren Feb 16, 2021

vivek-30 Feb 16, 2021

vivek-30 Feb 16, 2021 •

edited

jywarren commented Feb 16, 2021 via email

vivek-30 commented Feb 17, 2021

vivek-30 commented Feb 17, 2021

jywarren commented Feb 17, 2021 via email

vivek-30 commented Feb 17, 2021

jywarren commented Feb 17, 2021 via email

vivek-30 commented Feb 17, 2021

jywarren commented Feb 17, 2021 via email

vivek-30 commented Feb 18, 2021

jywarren Feb 18, 2021

vivek-30 Feb 18, 2021 •

edited

vivek-30 Feb 18, 2021

jywarren Feb 19, 2021

vivek-30 Feb 19, 2021

vivek-30 Feb 19, 2021

vivek-30 commented Feb 19, 2021

jywarren Feb 19, 2021

jywarren commented Feb 19, 2021

vivek-30 commented Feb 20, 2021

Notify Users for the failure in loading Image via external URL #1813

Notify Users for the failure in loading Image via external URL #1813

Conversation

vivek-30 commented Feb 16, 2021

gitpod-io bot commented Feb 16, 2021 • edited

vivek-30 commented Feb 16, 2021

vivek-30 commented Feb 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vivek-30 Feb 16, 2021 • edited

Choose a reason for hiding this comment

jywarren commented Feb 16, 2021 via email

vivek-30 commented Feb 17, 2021

vivek-30 commented Feb 17, 2021

jywarren commented Feb 17, 2021 via email

vivek-30 commented Feb 17, 2021

jywarren commented Feb 17, 2021 via email

vivek-30 commented Feb 17, 2021

jywarren commented Feb 17, 2021 via email

vivek-30 commented Feb 18, 2021

Choose a reason for hiding this comment

vivek-30 Feb 18, 2021 • edited

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

vivek-30 commented Feb 19, 2021

Choose a reason for hiding this comment

jywarren commented Feb 19, 2021

vivek-30 commented Feb 20, 2021

gitpod-io bot commented Feb 16, 2021 •

edited

vivek-30 Feb 16, 2021 •

edited

vivek-30 Feb 18, 2021 •

edited