-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Gutenberg] Audio block - Consolidated & Normal Media Picker Integration #13612
Changes from 30 commits
1e75b68
08bd4aa
9c89fa3
6b389c4
10992e1
4163371
b286de9
b73c139
ff71990
203de09
a8a1e82
408cf6d
e4039c0
f278d14
c724657
bff3d36
7149d57
903229e
554bb06
5f70822
ed598e6
55f8c92
55b5345
a881acf
3ca76e1
66199d8
a286d91
fbc51cf
b87c11e
97abd5a
8262169
aedea20
beb48b7
7815032
1e5aa6d
3309187
adbb939
9f745ae
a75a1b3
21e7726
8025aaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
|
||
import androidx.annotation.NonNull; | ||
import androidx.annotation.Nullable; | ||
import androidx.annotation.StringRes; | ||
import androidx.appcompat.app.AlertDialog; | ||
import androidx.core.content.ContextCompat; | ||
import androidx.core.content.FileProvider; | ||
|
@@ -241,9 +242,15 @@ public static void launchMediaLibrary(Activity activity, boolean multiSelect) { | |
RequestCodes.MEDIA_LIBRARY); | ||
} | ||
|
||
public static void launchFileLibrary(Activity activity, boolean multiSelect) { | ||
activity.startActivityForResult(prepareFileLibraryIntent(activity, multiSelect), | ||
RequestCodes.FILE_LIBRARY); | ||
public static void launchFileLibrary(Activity activity, boolean multiSelect, int requestCode) { | ||
switch (requestCode) { | ||
case RequestCodes.FILE_LIBRARY: | ||
activity.startActivityForResult(prepareFileLibraryIntent(activity, multiSelect), requestCode); | ||
break; | ||
case RequestCodes.AUDIO_LIBRARY: | ||
activity.startActivityForResult(prepareAudioLibraryIntent(activity, multiSelect), requestCode); | ||
break; | ||
} | ||
} | ||
|
||
public static void launchChooserWithContext( | ||
|
@@ -256,34 +263,40 @@ public static void launchChooserWithContext( | |
requestCode); | ||
} | ||
|
||
private static Intent preparePictureLibraryIntent(Context context, boolean multiSelect) { | ||
return prepareIntent(context, multiSelect, Intent.ACTION_GET_CONTENT, "image/*", | ||
new MimeTypes().getImageTypesOnly(), R.string.pick_photo); | ||
} | ||
|
||
private static Intent prepareVideoLibraryIntent(Context context, boolean multiSelect) { | ||
Intent intent = new Intent(Intent.ACTION_GET_CONTENT); | ||
intent.setType("video/*"); | ||
intent.putExtra(Intent.EXTRA_MIME_TYPES, new MimeTypes().getVideoTypesOnly()); | ||
if (multiSelect) { | ||
intent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true); | ||
jd-alexander marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
return Intent.createChooser(intent, context.getString(R.string.pick_video)); | ||
return prepareIntent(context, multiSelect, Intent.ACTION_GET_CONTENT, "video/*", | ||
new MimeTypes().getVideoTypesOnly(), R.string.pick_video); | ||
} | ||
|
||
private static Intent prepareMediaLibraryIntent(Context context, boolean multiSelect) { | ||
Intent intent = new Intent(Intent.ACTION_GET_CONTENT); | ||
intent.setType("*/*"); | ||
intent.putExtra(Intent.EXTRA_MIME_TYPES, new MimeTypes().getVideoAndImageTypesOnly()); | ||
if (multiSelect) { | ||
intent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true); | ||
} | ||
return Intent.createChooser(intent, context.getString(R.string.pick_media)); | ||
return prepareIntent(context, multiSelect, Intent.ACTION_GET_CONTENT, "*/*", | ||
new MimeTypes().getVideoAndImageTypesOnly(), R.string.pick_media); | ||
} | ||
|
||
private static Intent prepareFileLibraryIntent(Context context, boolean multiSelect) { | ||
Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); | ||
intent.setType("*/*"); | ||
intent.putExtra(Intent.EXTRA_MIME_TYPES, new MimeTypes().getAllTypes()); | ||
return prepareIntent(context, multiSelect, Intent.ACTION_OPEN_DOCUMENT, "*/*", | ||
new MimeTypes().getAllTypes(), R.string.pick_file); | ||
} | ||
|
||
private static Intent prepareAudioLibraryIntent(Context context, boolean multiSelect) { | ||
return prepareIntent(context, multiSelect, Intent.ACTION_GET_CONTENT, "*/*", | ||
new MimeTypes().getAudioTypesOnly(), R.string.pick_audio); | ||
} | ||
|
||
private static Intent prepareIntent(Context context, boolean multiSelect, String action, String intentType, | ||
String[] mimeTypes, @StringRes int title) { | ||
Intent intent = new Intent(action); | ||
intent.setType(intentType); | ||
intent.putExtra(Intent.EXTRA_MIME_TYPES, mimeTypes); | ||
if (multiSelect) { | ||
intent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true); | ||
intent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, multiSelect); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tbh I would rather keep the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yes, good spotting that! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. Resolved here 1e5aa6d |
||
} | ||
return Intent.createChooser(intent, context.getString(R.string.pick_file)); | ||
return Intent.createChooser(intent, context.getString(title)); | ||
} | ||
|
||
private static Intent prepareChooserIntent( | ||
|
@@ -311,19 +324,10 @@ private static Intent prepareVideoCameraIntent() { | |
|
||
public static void launchPictureLibrary(Activity activity, boolean multiSelect) { | ||
activity.startActivityForResult( | ||
preparePictureLibraryIntent(activity.getString(R.string.pick_photo), multiSelect), | ||
preparePictureLibraryIntent(activity, multiSelect), | ||
RequestCodes.PICTURE_LIBRARY); | ||
} | ||
|
||
private static Intent preparePictureLibraryIntent(String title, boolean multiSelect) { | ||
Intent intent = new Intent(Intent.ACTION_GET_CONTENT); | ||
intent.setType("image/*"); | ||
intent.putExtra(Intent.EXTRA_MIME_TYPES, new MimeTypes().getImageTypesOnly()); | ||
if (multiSelect) { | ||
intent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true); | ||
} | ||
return Intent.createChooser(intent, title); | ||
} | ||
|
||
private static Intent prepareGalleryIntent(String title) { | ||
Intent intent = new Intent(Intent.ACTION_PICK); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this approach is fine. What I did here was I duplicated the
showFilePicker
method because I need theallowTypes
to be different for the file and audio pickers. At first, I thought of making this property a default one in the method params but this doesn't work with Java. Is this behavior okay? Or should I do some form of method override? Let me know.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I didn't bother to bring the Normal Picker behavior up to speed here because it seems like the consolidated media picker will be the only one in use for the near future so I thought I would waste time building it out and there's a high possibility we won't do a rollback. Was that the right judgment call? ? I did make modifications in the past to support the File Block's file selection behavior on the normal and consolidated picker hence my question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I would consider doing here is creating a private fun that incorporates the common code and that accepts
(activity, canMultiselect, allowedTypes)
then the publicly exposed methods (showFilePicker, showAudioFilePicker) can call this private fun passing the desired allowedTypes (I would not expose the allowedTypes as public fun params though so the calling function should not worry about it). This in a way also ease the second item:I think the rationale here was as you mention to allow an easy rollback (doing it by remote) in case of any issues with the consolidated media picker. To allow this we need to have the standard media picker fallback in place. I agree it's pretty unlikely we should rollback since the media picker has been released in 16.2 already, but until we do not remove this
duplicated
approach (probably in other couple of release or so) I would keep this consistent. wdyt?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great insight @develric I am going to do this implementation and then share my findings with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done ff71990 3ca76e1