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

Feature/navigator_share #3974

Draft
wants to merge 41 commits into
base: develop
Choose a base branch
from

Conversation

carpoo
Copy link

@carpoo carpoo commented Jan 2, 2024

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

Webshare API requires a transient activation. If navigator.share fails with NotAllowedError a dialog is opened to request sharing confirmation.
image

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:

  • I've looked through existing tests, but I am not quite sure on how to implement tests. Could you point me in the right direction?
  • Documentation could be updated by adding a dedicated page for sharing files in this way. Would that be alright?
  • How can I add new Strings in https://translate.photoprism.app/?
  • What video format should be used? mp4 seems to work quite well for me. However mov files are not repackaged. However, windows and iOS seem to be fine with that.

Shared formats:

  • MediaImage, MediaLive, MediaRaw: JPEG or PNG
  • MediaSidecar (uses first file, I am not sure if it is even possible to select a file that is just a sidecar)
  • MediaAnimated: Gif (if available, otherwise JPEG)
  • MediaVector: SVG (if available, otherwise JPEG)
  • MediaVideo: MP4 (from preview url)

Browser compatibility: (Source: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share)
image

Acceptance Criteria:

  • Features and enhancements must be fully implemented so that they can be released at any time without additional work
  • Automated unit and/or acceptance tests are mandatory to ensure the changes work as expected and to reduce repetitive manual work
  • Frontend components must be responsive to work and look properly on phones, tablets, and desktop computers; you must have tested them on all major browsers and different devices
  • Documentation and translation updates should be provided if needed
  • In case you submit database-related changes, they must be tested and compatible with SQLite 3 and MariaDB 10.5.12+

@CLAassistant
Copy link

CLAassistant commented Jan 2, 2024

CLA assistant check
All committers have signed the CLA.

@lastzero
Copy link
Member

lastzero commented Jan 2, 2024

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?

@carpoo
Copy link
Author

carpoo commented Jan 3, 2024

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?

@lastzero
Copy link
Member

lastzero commented Jan 3, 2024

Instead of making the buttons even smaller, I thought about using a dropdown ⋮ menu similar to that:

Screenshot 2024-01-03 at 09 06 10

This would require some CSS to make it responsive (depending on the screen width), adding HTML for it and possibly also (re-)attaching the event handlers to the (alternative) elements.

@carpoo
Copy link
Author

carpoo commented Jan 3, 2024

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.

image
image

@carpoo
Copy link
Author

carpoo commented Jan 7, 2024

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.
Added a setting to disable the webshare feature. I have updated the top comment to reflect the current state.

@graciousgrey
Copy link
Member

Thank you! I am going to test your changes this week :)

@graciousgrey
Copy link
Member

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

  1. You mentioned that the share button doesn't show up in Firefox, or maybe I just misunderstood your comment. For me it showed up on Android and iOs. On iOs it worked, but on Android nothing happened when I clicked the share button. If it's possible, it would be great if the download button showed up on Android Firefox instead. In any case, the incompatibility with Android/Firefox is something we should add to the docs later.

  2. In the current implementation, for photos of each type (image, live, raw, video...) the primary jpg is shared.
    Either we need to add additional settings for sharing ( similar to the ones we have for download)
    Or we agree on a default behaviour, e.g. that for RAWs and images the JPG file is shared and for videos the video is shared. What do you think?

  3. In the case that the WebShare API generally does not work with HTTP, it would be great if the setting was only available for instances that use HTTPS. If this is not possible or too complicated, we could also disable the feature by default and add a note (ideally on the settings page) that it only works with HTTPS.

  4. I was not able to share a larger number (39) of files at once. I was able to open the share dialog and select a target, but then nothing happened. Have you tested with a larger number of files? Do you know if the API has a limit on the number of files that can be shared?

  5. We would prefer not to add additional actions to the context menu as space is limited on smaller mobile devices. We plan to redesign this menu or add an additional menu in the future so that we can display more options at once. However, we haven't had time to work on this yet. We could use similar logic as in the photo viewer and prioritize the share action over the download action on mobile when the feature is enabled. Downloading would then still be possible via the webShare API.

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

@carpoo
Copy link
Author

carpoo commented Jan 10, 2024

  1. Turns out webshare API needs to be enabled in desktop Firefox. That's why it did not show up during my tests.
  2. JPGs for RAWs and Video for Videos sounds good. What would you choose for Live Photos? I think the JPG is more useful as most photos are still meant as single photos, but that might just be my own usage.
  3. We could swap the download action with the share action in photo-viewer and context menu when:
    3.a On mobile devices
    3.b Sharing of current content type is supported
  4. I will look into it.
  5. See 3
    (I really like the clipboard menu style. I configured it to contain only the actions I am using, making it quite small.)

@graciousgrey
Copy link
Member

Turns out webshare API needs to be enabled in desktop Firefox. That's why it did not show up during my tests.

Thanks for the hint. I will have a look at my firefox settings and test again.

JPGs for RAWs and Video for Videos sounds good. What would you choose for Live Photos? I think the JPG is more useful as most photos are still meant as single photos, but that might just be my own usage.

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.
I think in the first version we can go with: raw -> jpg, image -> jpg, video -> video, live -> jpg.
I'm just not sure about the animations. These could be animated gifs or PNGs, for example. I'm going to do some more testing to see if these are generally supported by the most popular apps that people want to share their photos with.

That would be great :)

  1. I will look into it.

Thank you!

@carpoo
Copy link
Author

carpoo commented Jan 21, 2024

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.

@graciousgrey
Copy link
Member

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.

@carpoo
Copy link
Author

carpoo commented Jan 25, 2024

Latest tests:
Desktop with Windows: Edge, Chrome, Firefox: all show the download icon as expected
Mobile with iOS:
Safari: works
Chrome: works
Firefox: works

Images, GIFs, Videos work fine.

Things I could not test:

  • Sharing SVG and other vector images (my instance is complaining that this is not supported, does it need to be enabled somehow?)
  • RAW images (I just don't have any)
  • Pure Sidecar files (could not find a way on how to select them in the UI)
  • Mobile browsers on Android

@graciousgrey
Copy link
Member

Some more test results :)

You mentioned that the share button doesn't show up in Firefox, or maybe I just misunderstood your comment. For me it showed up on Android and iOs. On iOs it worked, but on Android nothing happened when I clicked the share button. If it's possible, it would be great if the download button showed up on Android Firefox instead.

This is working now ✔️

In the current implementation, for photos of each type (image, live, raw, video...) the primary jpg is shared.
Either we need to add additional settings for sharing ( similar to the ones we have for download)
Or we agree on a default behaviour, e.g. that for RAWs and images the JPG file is shared and for videos the video is shared.

This is working now ✔️

We would prefer not to add additional actions to the context menu as space is limited on smaller mobile devices. We plan to redesign this menu or add an additional menu in the future so that we can display more options at once. However, we haven't had time to work on this yet. We could use similar logic as in the photo viewer and prioritize the share action over the download action on mobile when the feature is enabled. Downloading would then still be possible via the webShare API.

This does look good now as well ✔️ 🎉

I was not able to share a larger number (39) of files at once.

When I try to share many files at once or a larger file, I sometimes see the following behavior:

  • After clicking "Share", I see the dialog "Download complete. Ready to share?".
  • If I then click on "Share", the message "Sharing photos failed - download icon is displayed" appears immediately.

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.

@carpoo
Copy link
Author

carpoo commented Mar 16, 2024

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.
We might change to the download icon as long as more than ten images are selected.

@carpoo
Copy link
Author

carpoo commented Mar 16, 2024

Ten image limit is implemented.

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

5 participants