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

Media & Text - Media picker buttons functionality #1378

Merged
merged 20 commits into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,270 changes: 636 additions & 634 deletions bundle/android/App.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/App.js.map

Large diffs are not rendered by default.

1,282 changes: 642 additions & 640 deletions bundle/ios/App.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/ios/App.js.map

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions ios/gutenberg/GutenbergViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ extension GutenbergViewController: GutenbergBridgeDelegate {
switch currentFilter {
case .image:
if(allowMultipleSelection) {
callback([(1, "https://cldup.com/cXyG__fTLN.jpg"), (3, "https://cldup.com/cXyG__fTLN.jpg")])
callback([(1, "https://cldup.com/cXyG__fTLN.jpg", "image"), (3, "https://cldup.com/cXyG__fTLN.jpg", "image")])
} else {
callback([(1, "https://cldup.com/cXyG__fTLN.jpg")])
callback([(1, "https://cldup.com/cXyG__fTLN.jpg", "image")])
}
case .video:
if(allowMultipleSelection) {
callback([(2, "https://i.cloudup.com/YtZFJbuQCE.mov"), (4, "https://i.cloudup.com/YtZFJbuQCE.mov")])
callback([(2, "https://i.cloudup.com/YtZFJbuQCE.mov", "video"), (4, "https://i.cloudup.com/YtZFJbuQCE.mov", "video")])
} else {
callback([(2, "https://i.cloudup.com/YtZFJbuQCE.mov")])
callback([(2, "https://i.cloudup.com/YtZFJbuQCE.mov", "video")])
}
default:
break
Expand All @@ -98,16 +98,16 @@ extension GutenbergViewController: GutenbergBridgeDelegate {

func gutenbergDidRequestImport(from url: URL, with callback: @escaping MediaPickerDidPickMediaCallback) {
let id = mediaUploadCoordinator.upload(url: url)
callback([(id, url.absoluteString)])
callback([(id, url.absoluteString, "image")])
}

func pickAndUpload(from source: UIImagePickerController.SourceType, filter: MediaFilter, callback: @escaping MediaPickerDidPickMediaCallback) {
mediaPickCoordinator = MediaPickCoordinator(presenter: self, filter: filter, callback: { (url) in
guard let url = url, let mediaID = self.mediaUploadCoordinator.upload(url: url) else {
callback([(nil, nil)])
callback([(nil, nil, nil)])
return
}
callback([(mediaID, url.absoluteString)])
callback([(mediaID, url.absoluteString, "image")])
self.mediaPickCoordinator = nil
} )
mediaPickCoordinator?.pick(from: source)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public interface GutenbergBridgeJS2Parent {
interface RNMedia {
String getUrl();
int getId();
String getType();
WritableMap toMap();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public WritableMap toMap() {
WritableMap map = new WritableNativeMap();
map.putInt("id", mId);
map.putString("url", mUrl);
map.putString("type", mType);
return map;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -512,11 +512,16 @@ public CharSequence getTitle(OnGetContentTimeout onGetContentTimeout) {
return "";
}

private String getMediaType(final boolean isVideo) {
return isVideo ? "video" : "image";
}

public void appendMediaFile(int mediaId, final String mediaUrl, final boolean isVideo) {
if (mPendingMediaSelectedCallback != null && mMediaPickedByUserOnBlock) {
String mediaType = getMediaType(isVideo);
mMediaPickedByUserOnBlock = false;
List<RNMedia> mediaList = new ArrayList<>();
mediaList.add(new Media(mediaId, mediaUrl));
mediaList.add(new Media(mediaId, mediaUrl, mediaType));
mPendingMediaSelectedCallback.onMediaSelected(mediaList);
mPendingMediaSelectedCallback = null;
} else {
Expand Down Expand Up @@ -558,9 +563,10 @@ public void toggleEditorMode() {

public void appendUploadMediaFile(final int mediaId, final String mediaUri, final boolean isVideo) {
if (isMediaUploadCallbackRegistered() && mMediaPickedByUserOnBlock) {
String mediaType = getMediaType(isVideo);
mMediaPickedByUserOnBlock = false;
List<RNMedia> mediaList = new ArrayList<>();
mediaList.add(new Media(mediaId, mediaUri));
mediaList.add(new Media(mediaId, mediaUri, mediaType));
mPendingMediaUploadCallback.onUploadMediaFileSelected(mediaList);
} else {
// we can assume we're being passed a new image from share intent as there was no selectMedia callback
Expand All @@ -573,7 +579,7 @@ public void appendUploadMediaFiles(ArrayList<Media> mediaList) {
mMediaPickedByUserOnBlock = false;
List<RNMedia> rnMediaList = new ArrayList<>();
for (Media media : mediaList) {
rnMediaList.add(new Media(media.getId(), media.getUrl()));
rnMediaList.add(new Media(media.getId(), media.getUrl(), media.getType()));
}
mPendingMediaUploadCallback.onUploadMediaFileSelected(rnMediaList);
mPendingMediaUploadCallback = null;
Expand All @@ -582,7 +588,7 @@ public void appendUploadMediaFiles(ArrayList<Media> mediaList) {

private void sendOrDeferAppendMediaSignal(final int mediaId, final String mediaUri, final boolean isVideo) {
// if editor is mounted, let's append the media file
String mediaType = isVideo ? "video" : "image";
String mediaType = getMediaType(isVideo);
if (mIsEditorMounted) {
if (!TextUtils.isEmpty(mediaUri) && mediaId > 0) {
// send signal to JS
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
public typealias MediaPickerDidPickMediaCallback = (_ media: [(Int32?,String?)]?) -> Void
public typealias MediaPickerDidPickMediaCallback = (_ media: [(Int32?,String?,String?)]?) -> Void
Copy link
Contributor

Choose a reason for hiding this comment

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

These callback parameter list is kind of growing a lot 🤔 . Maybe we could think about using a struct?
Having too many unnamed parameters in the public API doesn't sound like a good practice.
cc @koke @pinarol

Copy link
Member

Choose a reason for hiding this comment

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

That sounds like a good idea to me 😄


public enum MediaPickerSource: String {
case mediaLibrary = "SITE_MEDIA_LIBRARY"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ public class RNReactNativeGutenbergBridge: RCTEventEmitter {
return
}
if (allowMultipleSelection) {
let formattedMedia = media.map { (id, url) in
return [mediaDictKeys.IDKey: id, mediaDictKeys.URLKey: url]
let formattedMedia = media.map { (id, url, type) in
return [mediaDictKeys.IDKey: id, mediaDictKeys.URLKey: url, mediaDictKeys.TypeKey: type]
}
callback([formattedMedia])
} else {
guard let (mediaID, mediaURL) = media.first else {
guard let (mediaID, mediaURL, mediaType) = media.first else {
callback(nil)
return
}
callback([[mediaDictKeys.IDKey: mediaID, mediaDictKeys.URLKey: mediaURL]])
callback([[mediaDictKeys.IDKey: mediaID, mediaDictKeys.URLKey: mediaURL, mediaDictKeys.TypeKey: mediaType]])
}
})
}
Expand Down Expand Up @@ -194,5 +194,6 @@ extension RNReactNativeGutenbergBridge {
enum mediaDictKeys {
static let IDKey = "id"
static let URLKey = "url"
static let TypeKey = "type"
}
}