Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Rework the "picker" results to correctly manage files #1555

Merged
merged 4 commits into from
Dec 2, 2020

Conversation

mattleibow
Copy link
Contributor

@mattleibow mattleibow commented Nov 30, 2020

Description of Change

This PR solves a few issues.

  • The URI to the picked file is only valid as long as the calling Activity is alive.
    Because we use an intermediate activity, this means that the URI almost immediately becomes invalid.
    To solve for this, we have to resolve the physical path of the picked file. Many providers have a way to retrieve the physical path, or the path is known based on the type of content URI.
  • Some picked files cannot be resolve to a physical path.
    This might be because there are no permissions, or the file only exists as a stream from a file provider.
    In this case, we need to copy the file to a local location.
  • Some picked *files are virtual.
    This might be a Google docs spreadsheet which is not a real file format.
    To solve this, we detect the virtual and then ask the provider what formats are supported. We then use that format and open the stream.

This PR also changes a few things in the code that overlap with the fixes.

  • Removed the usage of magic strings to a static list of mime types and extensions.
  • Made use of the IsIntentSupported method on Android.
  • Switched to List<T> for ShareMultipleFilesRequest.Files for consistency.

Bugs Fixed

API Changes

None visible.

Behavioral Changes

This PR will make sure all the FileResult instances point to a physical file location after a picker operation.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

@mattleibow mattleibow added this to the 1.6.0 milestone Nov 30, 2020
pictos
pictos previously approved these changes Dec 1, 2020
Copy link
Contributor

@pictos pictos left a comment

Choose a reason for hiding this comment

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

That looks good, I have just one comment, but is more a suggestion than a request.

return resolved;
}
else if (uri.Scheme.Equals("file", StringComparison.OrdinalIgnoreCase))
{
Copy link
Contributor

Choose a reason for hiding this comment

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

how about moving those strings to constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. You got me. I am always pestering people abut these magic strings. I will hang my head in shame and walk to the nearest corner.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's ok that you like to type the same string over and over again 🤣

- Mime types
- Extensions
- Changed property type of ShareMultipleFilesRequest.Files to be a List<T> for consistency
@mattleibow mattleibow marked this pull request as ready for review December 1, 2020 22:33
@mattleibow mattleibow merged commit add928f into main Dec 2, 2020
@mattleibow mattleibow deleted the dev/fix-android-pickers branch December 2, 2020 13:58
<intent>
<action android:name="android.intent.action.DIAL" />
<data android:scheme="tel" />
</intent>
<!-- MediaPicker -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be added to documentation :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.