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

feat: MediaGallery #1146

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

feat: MediaGallery #1146

wants to merge 15 commits into from

Conversation

MartinZikmund
Copy link
Member

@MartinZikmund MartinZikmund commented May 29, 2024

GitHub Issue (If applicable): closes unoplatform/uno#16721, closes #1147

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@MartinZikmund MartinZikmund changed the title [WIP] MediaGalleryHelper feat: MediaGalleryHelper May 29, 2024
@MartinZikmund MartinZikmund marked this pull request as ready for review May 29, 2024 15:49
Copy link
Contributor

@agneszitte agneszitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Missing related documentation @MartinZikmund please -> https://github.com/unoplatform/uno.toolkit.ui/tree/main/doc

src/Uno.Toolkit.UI/Helpers/MediaGallery/MediaGallery.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@Xiaoy312 Xiaoy312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use some relevant doc on this

Co-authored-by: Xiaotian Gu <[email protected]>
Co-authored-by: Agnès ZITTE <[email protected]>
@MartinZikmund MartinZikmund changed the title feat: MediaGalleryHelper feat: MediaGallery May 31, 2024
@MartinZikmund
Copy link
Member Author

@agneszitte docs added


## Remarks

This class is designed to work on iOS, Mac Catalyst and Android platforms, utilizing platform-specific implementations for its methods.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This class is designed to work on iOS, Mac Catalyst and Android platforms, utilizing platform-specific implementations for its methods.
This class is designed to work on iOS, Mac Catalyst, and Android platforms, utilizing platform-specific implementations for its methods.


This class is designed to work on iOS, Mac Catalyst and Android platforms, utilizing platform-specific implementations for its methods.

The API allows setting the `targetFileName`, which **should ideally be unique** - otherwise the OS will either overwrite an existing file with the same name (Android behavior), or generate a new name instead (iOS behavior).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The API allows setting the `targetFileName`, which **should ideally be unique** - otherwise the OS will either overwrite an existing file with the same name (Android behavior), or generate a new name instead (iOS behavior).
The API allows setting the `targetFileName`, which **should ideally be unique** - otherwise the OS will either overwrite an existing file with the same name (Android behavior) or generate a new name instead (iOS behavior).


#### Summary

Checks the user's permission to access the device's gallery. Will trigger the permission request if not already granted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Checks the user's permission to access the device's gallery. Will trigger the permission request if not already granted.
Checks the user's permission to access the device's gallery. It will trigger the permission request if it has not already been granted.

<string>This app needs access to the photo gallery for saving photos and videos</string>
```

If you want to support earlier versions iOS, add the following as well:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you want to support earlier versions iOS, add the following as well:
If you want to support earlier versions of iOS, add the following as well:

#endif
```

### Copying an application package file to gallery
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Copying an application package file to gallery
### Copying an application package file to the gallery

}
catch (Exception ex)
{
tcs.TrySetResult(ex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you choose to use TrySetResult instead of TrySetException?

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.

Spec: MediaGallery Add support for storing photos and videos without using the FileSavePicker
5 participants