Ademking / BetterViewer

a replacement for the image viewing mode built into Firefox and Chrome-based web browsers.
MIT License
252 stars 19 forks source link

add Improved Image Support #44

Closed Metacor closed 2 years ago

Metacor commented 2 years ago

fix #37 - added support for local image files opened in the browser

changed ['*://*/*'] to ['<all_urls>'] added chrome.webRequest.onCompleted : for file:// scheme

fix #19 - added the option to use imgur or imgbb (suggestion 2)

suggestion 1 was to add Hover Zoom, which doesn't seem feasible. suggestions 3, 4, and 5 were already implemented in v1.0.4

close #36 - answered their question

fix cursor hover/selected values - to make interactive elements more easily identifiable.

also not sure why, but editing 'css/viewer.min.css' seemed to update thousands of lines in all.js/.css. nothing changed, and the actual code was only 15 lines of .css, and 0 lines of .js

Ademking commented 2 years ago

@Metacor Thank you so much for your contribution ❤️ We appreciate all the time and effort you’ve put in helping us get to where we are today.

Everything is LGTM.. But, I have some problems with local image files #37 :

image

image

image


I want to ask you:

Thank you again for your contribution @Metacor ! You're the best🥇

Metacor commented 2 years ago

@Ademking I would say to just merge the pull request since it doesn't seem to break anything, but in the changelog specify state "added rudimentary Local File Support" or something similar so people know that it's kinda broken.

For the above reasons, I'm unsure if these issues can ever be fixed -- local files will likely only ever have access to the base viewer (view, zoom, reset, fullscreen, flip, crop, and extract text).

I believe the base functionality is enough to justify adding Local File support, (being able to zoom in as far as I want is the main reason I use this extension), if you disagree, then I'm not sure if there is any way to salvage Local Files without uploading them as soon as they are loaded (which seems sketchy, and kinda defeats the purpose of it being local in the first place).

If we plan on keeping Local File support, then the best option is to probably add a 'isLocalFile' check before rendering the affected buttons, and just hide them on local files. (I'm having trouble getting this to work, if we go this route I would need help)

Online files seem to be unaffected, (as we would expect, since the 'file://' scheme is on a separate Listener)

Ademking commented 2 years ago

@Ademking I would say to just merge the pull request since it doesn't seem to break anything, but in the changelog specify state "added rudimentary Local File Support" or something similar so people know that it's kinda broken.

For the above reasons, I'm unsure if these issues can ever be fixed -- local files will likely only ever have access to the base viewer (view, zoom, reset, fullscreen, flip, crop, and extract text).

I believe the base functionality is enough to justify adding Local File support, (being able to zoom in as far as I want is the main reason I use this extension), if you disagree, then I'm not sure if there is any way to salvage Local Files without uploading them as soon as they are loaded (which seems sketchy, and kinda defeats the purpose of it being local in the first place).

If we plan on keeping Local File support, then the best option is to probably add a 'isLocalFile' check before rendering the affected buttons, and just hide them on local files. (I'm having trouble getting this to work, if we go this route I would need help)

Online files seem to be unaffected, (as we would expect, since the 'file://' scheme is on a separate Listener)

Sounds good to me 👍 Again.. I can't thank you enough.

DevOdian commented 9 months ago

Works like a charm. Please do a new release when you have time, @Ademking