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

add Improved Image Support #44

Merged
merged 3 commits into from
Oct 17, 2022
Merged

Conversation

Metacor
Copy link
Contributor

@Metacor Metacor commented Oct 16, 2022

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.

  •   Main Image: grab when hovering ; grabbing when :active

    I also increased the visual selector area to match the actual selector area.
    Everything above the toolbar was selectable, but only the img area visually changed the cursor

  •   Custom Modifiers: pointer
  •   Custom Keys: pointer when hovering ; text when :focus-visible
  •   Zoom Ratio: pointer when hovering ; text when :focus-visible
  •   Default Theme, Toast position, and Toolbar Position: pointer

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

js/app.js Show resolved Hide resolved
@Ademking
Copy link
Owner

@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 :

  • I can't edit images using "Photo Editor"

image

  • Color picker is not working

image

  • Image details also not working

image

  • Print Image, Reverse Image Search and QR Scanning are not working

I want to ask you:

  • Do you want to merge this pull request and we fix it later in the next version ?
  • Wait until we fix those problems and then merge it ?

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

@Metacor
Copy link
Contributor Author

Metacor commented Oct 17, 2022

@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
Copy link
Owner

@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.

@Ademking Ademking merged commit d389175 into Ademking:master Oct 17, 2022
@Metacor Metacor deleted the fix/image-support branch October 18, 2022 00:53
Metacor added a commit to Metacor/BetterViewer that referenced this pull request Oct 20, 2022
@DevOdian
Copy link

DevOdian commented Jan 1, 2024

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

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.

Open locally saved image files Imgur upload Feature requests
3 participants