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

Adds Standard CSV Mime Type #1377

Merged
merged 3 commits into from
Nov 4, 2023

Conversation

SoftWyer
Copy link

@SoftWyer SoftWyer commented Oct 4, 2023

On Android, the CSV mime type from getMimeTypeFromExtension() returns "text/comma-separated-values" which is non-standard and doesn't filter CSV files in Google Drive.

This change adds the standard "text/csv" mime type when the csv extension is specified.

We did think about adding the ability to specify explicit mime types from the API, but this would be a bigger change when this only seems to be an Android issue.

see standards docs
see Android code

@miguelpruivo
Copy link
Owner

Hi, thank you for your PR but the problem here is that iOS doesn't support it and may result in some devs assuming that is working on both and then finding that iOS doesn't support this kind of filter and then later report yet another issue.

Every filter should be supported from all OS to avoid unnecessary results.

@SoftWyer
Copy link
Author

Hi @miguelpruivo, perhaps you've misunderstood the change. Filtering the file selection by CSV is already supported by both Android and iOS. We can see files with these extensions when using file-picker on these devices.

The problem is that Android uses a non-standard CSV mime type extension. This means that it can filter on-device CSV files, but cannot filter CSV files using any cloud service (e.g. Google Drive) as these use the standard mime type. This change just ensures that CSV filtering works on Android as expected regardless of file system. I tangentially mentioned the CSV problem in this issue.

If you'd rather not include this PR, that it OK with us. We needed Android CSV filtering to work consistently in our app so thought we'd give those changes back to the project 😃

Copy link
Owner

@miguelpruivo miguelpruivo left a comment

Choose a reason for hiding this comment

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

Ok, looks like a nice addition. Could you please rebase to the latest version and update changelog/pubspec accordingly? Thanks!

@miguelpruivo miguelpruivo self-assigned this Oct 31, 2023
@SoftWyer
Copy link
Author

SoftWyer commented Nov 1, 2023

Latest changes merged, should be good to go 👍

@miguelpruivo miguelpruivo merged commit 5eff0bf into miguelpruivo:master Nov 4, 2023
3 checks passed
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.

2 participants