Improve share sheet #576
Merged
Improve share sheet #576
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(We get more options this way)
(This way it is more close to the QE height)
pinarol
added
enhance
|
 |
App Name |  Gravatar Prototype Build |
| Build Number | 1793 | |
| Version | 1.0 | |
| Bundle ID | com.automattic.gravatar-sdk-demo-uikit.prototype-build | |
| Commit | 4801fd3 | |
| App Center Build | Gravatar SDK Demo - UIKit #422 |
pinarol
force-pushed
the
wppinar/improve-share-sheet
branch
from
eabc511 to
2faafb9
Compare
etoledom
approved these changes
Nov 27, 2024
Comment on lines
134
to
146
|
|
||
| func fetchAndSaveToFile(avatar: AvatarImageModel) async -> URL? { | ||
| guard let image = await fetchOriginalSizeAvatar(for: avatar), | ||
| let imageData = image.jpegData(compressionQuality: 1) else { return nil } | ||
| let fileURL = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("image.jpg") | ||
| do { | ||
| try imageData.write(to: fileURL) | ||
| return fileURL | ||
| } catch { | ||
| toastManager.showToast(Localized.avatarShareFail, type: .error) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Generally is good to have these functions which explicitaly do two things to have them split, even if they are two small functions like:
let image = fetchFullImage(of: avatar)`
let imageFileURL = image.saveToFile()
Maybe in this way we can also show the share sheet even if writing to storage fails 🤔
There was a problem hiding this comment.
Yeah totally makes sense to have a separate UIImage extension method like saveToFile(). I'll add it.
Maybe in this way we can also show the share sheet even if writing to storage fails
This is doable but I would consider this as an edge case. For sake of keeping our logic simple, I'd say let's just share the file for now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An improvement on #566
Description
I noticed that if we first write the image to a file and then share that file's URL, then there are more relevant options in the share sheet:
Also, updated the share sheet's initial height as same as QE's initial height (in horizontal mode it's not exactly same but it's close). So we see small or no height adjustment done by the system while share sheet is being opened.
Another thing is, I changed the word "download" as "share" in the error message.
Last but not least, I share the jpegData as there's a noticeable time difference between sharing pngData and jpegData. I set the compression quality to 1 (highest), and the shared image's quality seems good to me.
Testing Steps
(It can be good to compare with the previous version.)
Demo app > QE > avatar [...] menu > share