Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Download images plugin Issue#480 #491

Closed
wants to merge 2 commits into from
Closed

Download images plugin Issue#480 #491

wants to merge 2 commits into from

Conversation

aashu0148
Copy link
Contributor

All the downloading images will have a name

Copy link
Member

@argyleink argyleink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, PR to a PR, so fun! Ty!

  • Looks like you have a linter that went and formatted a bunch of CSS files. Can you remove those commits from the PR please 🙂
  • The updates to package json and the package lock files, can you remove those commits as well

I ran a developer edition of visbug and used the script on some pages, it mostly went great! See the above change requests and let's chat about how to resolve the ux issue i had. Thanks 🙂

try {
console.log(`Fetching ${url}`)
const response = await fetch(url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to do some modifying of image urls too, like we are with css urls. I ran the plugin on css-tricks.com and the script did not properly download everything. It missed over half. Still exciting to see it at work though. Things like ads, that are secure and serve once, I'm totally cool with not downloading. But avatars and other stuff, I def expect those to be in my folder, and they're currently missing for ~40% of css-tricks.com right now.

I didnt dig into it too deep, but it was urls like this one https://secure.gravatar.com/avatar/8081b26e05bb4354f7d65ffc34cbbd67?s=80&d=retro&r=pg that didnt download.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay @argyleink le me check for it and i'll update you with that tomorrow... now i have some important work to finish off.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @argyleink I have used the same url and it downloaded the image.
check the screenshots i have attached.
Screenshot (45)
Screenshot (46)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll look into this further and see what's going on different on css-tricks.com than the visbug test page.

@argyleink
Copy link
Member

closing since the work was merged in #492

@argyleink argyleink closed this Jan 4, 2021
@aashu0148
Copy link
Contributor Author

closing since the work was merged in #492

Oh :(

Is there any other issue suitable for me.
Any other issue i can contribute to ?

@argyleink
Copy link
Member

yep definitely!
what kind of stuff do you want to try next? add features, bug fix, learn more about X? i can help guide you to something new?

you can also see the current state of the plugin ux at the branch here #481, and it's not done yet! maybe you want to help finish it off? it's currently stuck at failing about 30% of images when run on real sites. You'll need to install the extension locally and run it on a webpage like css-tricks, to get the more "real world" set of urls. it's very very close! but when you run it, there's a few visual images that dont download, and it's due to either bad fetch urls, or bad names we try to write to disk. still trying to iron out all the frayed edges so this thing is pretty dependable and smooth.

thanks for all the help so far!

@aashu0148
Copy link
Contributor Author

hey @argyleink sorry for late reply actually i got a project to do... so i was busy in that.

ya some images are not downloading i guess the reason could be the images from local folder. like the URL can be "./assets/images/photo.jpeg" now in this case we can't get that image.

@argyleink
Copy link
Member

totally. there's a few cases we can't handle, totally cool with that, but it feels like too many are failing. especially since i can go find the url and open it in my browser. sometimes it's the name it tries to create on file is invalid, which makes sense, that logic is getting complex.

i figure we can widdle away at the few easy wins, and then merge it? i'd like it to be a bit more reliable. thoughts?

@aashu0148
Copy link
Contributor Author

totally. there's a few cases we can't handle, totally cool with that, but it feels like too many are failing. especially since i can go find the url and open it in my browser. sometimes it's the name it tries to create on file is invalid, which makes sense, that logic is getting complex.

i figure we can widdle away at the few easy wins, and then merge it? i'd like it to be a bit more reliable. thoughts?

ya if it is failing too many of images then its not good at all.
can you please provide those urls which are not working... i guess there can be some issue with the name.

@argyleink
Copy link
Member

I havent grabbed all the URLs yet, thats def a great next task. Aggregate them and figure out the problems.

I also think it might be the filename we create, like the file/url fetches 200, but we try to create a filename that isnt valid. I think it's a bit of both still left to do!?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants