-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Feature/navigator_share #3974
base: develop
Are you sure you want to change the base?
Feature/navigator_share #3974
Conversation
Awesome, thanks for pushing this forward! One thing to consider/check in this regard is the limited screen width of mobile devices. We'll be happy to help you with that when you're ready? |
The width for small screens is really an issue. The new top bar would require at least 396px viewport. Reducing button width to 40px for small screens would solve it. Would you mind reducing button size to fit this in? |
I tried that menu style, but I am not really happy with it. When the close button is hidden, a close would require two taps. If the close button stays, it will look kind of unusual to me. However, I did notice something else when testing on my phone. Zoom and fullscreen buttons were missing. I couldn't find it in code. How are they hidden? Is that always on mobile devices? That would save 88px solving the space issue. |
Another way to solve the space issue is using the webshare button on mobile browsers and the existing download button on other browsers. It is currently implemented in that way. |
Thank you! I am going to test your changes this week :) |
First of all, thank you very much for working on this! I have done some initial testing of the functionality (without having looked at the code in detail). I've tested on Android (Chrome, Firefox, Edge) and iOs (Chrome, Firefox, Safari). In general, sharing of JPGs (when connected via HTTPS) seems to work for all tested browsers on iOs and for Chrome and Edge on Android 🎉 So here is some first feedback from my side - I appreciate your thoughts on this :)
Regarding your questions: I can take a look at your tests over the next few days and give you feedback. I will also be happy to help adding tests later. As for the documentation and translations, I would suggest that we wait until we have merged the functionality and have a version that we can release. In our experience, we iterate the wording for e.g. the settings page quite often before a feature is released :) |
|
Thanks for the hint. I will have a look at my firefox settings and test again.
I agree, also some live photos e.g. the ones from Google do only have a jpg, as the video part is embedded inside the jpg. That would be great :)
Thank you! |
This reverts commit 32c1a51.
I am struggling a little with video sharing. I am trying to share a .mov file on Windows 10 with Chrome and get an error, that is not previously visible through canShare. The same file works just fine on iPhone. Still working on a solution. Added a confirmation dialog. If downloading takes a while, a new user interaction is required to allow sharing. |
Thanks! I should be able to test your changes by the end of the week :) @lastzero can take a closer look and help resolve the remaining issues (if any) once we have released the new authentication options. |
Latest tests: Images, GIFs, Videos work fine. Things I could not test:
|
Some more test results :)
This is working now ✔️
This is working now ✔️
This does look good now as well ✔️ 🎉
When I try to share many files at once or a larger file, I sometimes see the following behavior:
It's great that the download action is displayed after sharing a selection has failed ❤️ I was only confused to see the other dialog before it failed. In my tests, sharing failed every time the dialog appeared. |
I did some more tests on this and found a limit of 10 images depending on the browser. iOS/Safari is fine with more, Windows/Chrome seems to be limited to 10 images. |
Ten image limit is implemented. |
I'm curious what the current status/prognoses is for this pull request. This feature would significantly raise the SOAP of Photoprism. |
@lastzero will proceed with a code/security review once we have released OpenID Connect support. |
This PR adds a share button using Web Share API. It picks up on #1792. This would solve #658.
It uses the sharing icon used for albums. It is displayed in photo-viewer to share the viewed photo and in clipboard to share selected photos. Photos are downloaded and the navigator.share-function is used to hand over photos to the devices sharing system.
WebShare icon in photo-viewer is shown only on mobile browsers instead of the download icon, if webshare is supported by the browser. WebShare icon is only shown for up to 10 images. More images lead to an error in some browsers.
Webshare API requires a transient activation. If navigator.share fails with NotAllowedError a dialog is opened to request sharing confirmation.
If sharing fails, the download icon is temporarily shown instead.
There are some open points where I am not quite sure on how to proceed:
Shared formats:
Browser compatibility: (Source: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share)
Acceptance Criteria: