-
Notifications
You must be signed in to change notification settings - Fork 56
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
720-Add support for giphy and pexel images #1469
Changes from all commits
cdcd68f
fa362ec
d703d87
6130d0c
f326576
ae19042
ab2b2fe
52d2491
79c3ee8
c3516e1
81e9d9f
bc0f09f
8cbce18
a2f50cf
b5b9e19
262494f
86116ce
7551068
c7a5c3d
41e99a9
706d976
ac6d631
e19a386
b7c27e5
e4ff3b2
8395eac
3b03798
c4feb40
6666209
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 |
---|---|---|
@@ -1,5 +1,6 @@ | ||
1.16.0 | ||
------ | ||
* Add support for pexels images | ||
* Add left, center, and right image alignment controls | ||
|
||
1.15.0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package org.wordpress.mobile.WPAndroidGlue; | ||
|
||
import com.facebook.react.bridge.WritableMap; | ||
import com.facebook.react.bridge.WritableNativeMap; | ||
|
||
public class MediaOption { | ||
|
||
private static final String KEY_VALUE = "value"; | ||
private static final String KEY_LABEL = "label"; | ||
|
||
private String mId; | ||
private String mName; | ||
|
||
public MediaOption(String id, String name) { | ||
mId = id; | ||
mName = name; | ||
} | ||
|
||
public String getId() { | ||
return mId; | ||
} | ||
|
||
public String getName() { | ||
return mName; | ||
} | ||
|
||
public WritableMap toMap() { | ||
WritableMap map = new WritableNativeMap(); | ||
map.putString(KEY_VALUE, mId); | ||
map.putString(KEY_LABEL, mName); | ||
return map; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,4 +89,12 @@ export function requestImageUploadCancel( mediaId ) { | |
return RNReactNativeGutenbergBridge.requestImageUploadCancel( mediaId ); | ||
} | ||
|
||
export function getOtherMediaOptions( filter, callback ) { | ||
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. I don't want to block on this to get this feature out, but why are these "other"? Is there something intrinsically different in requesting an image from the device library or pexels? I understand we might need to pass the available options through the bridge because we want to show them in the existing bottom sheet (although I think a native one is in the works), but other than that, I think the apps should define which options are available more generally. 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. That's a good question @koke. Main idea is to provide 3rd party media sources from native applications so that in the future we can just change native apps WPAndroid or WPiOS and everything should work (without changing JS side or bridge). I agree that it would be easier to control which options are available from one place (JS) and not from two places (WPAndroid and WPiOS), but we can see in the future if this current solution has some big downsides and if's true, we can change approach. maybe also @etoledom can share his opinion on this one. 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.
I actually think it makes more sense to define those in the apps, and avoid gutenberg making any assumptions about the available options. As an example, the demo app still offers device library, camera, and wp media library, but only the device library works as expected. The camera option does nothing in the iOS simulator (it has no camera), and the media library option inserts a sample image/video. It would be better if gutenberg just asked about the available options to present them in the picker, and then request the selected one. In the demo app, we could have some options like I'm not sure how much effort this would be, but if we are already requesting the "others" through the bridge, it doesn't feel like it would be a hard thing to request all the options. 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. I missed the point at first but now it's clear. 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.
Actually, the only option on gutenberg web (stand alone) is to add an image from a link. The WordPress Media Picker is some kind of "add on" iirc. 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. I'd suggest to implement the bridge with this new options as it is now, and on a second step, migrate the "older" options to be handled on this new way. Hopefully we keep the testing steps and possible issues lower on this PR this way. 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. |
||
return RNReactNativeGutenbergBridge.getOtherMediaOptions( filter, callback ); | ||
} | ||
|
||
export function requestOtherMediaPickFrom( mediaSource, multiple, callback ) { | ||
return RNReactNativeGutenbergBridge.requestOtherMediaPickFrom( mediaSource, multiple, callback ); | ||
} | ||
|
||
export default RNReactNativeGutenbergBridge; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,16 @@ public class RNReactNativeGutenbergBridge: RCTEventEmitter { | |
}) | ||
} | ||
} | ||
|
||
@objc | ||
func requestOtherMediaPickFrom(_ source: String, allowMultipleSelection: Bool, callback: @escaping RCTResponseSenderBlock) { | ||
//TODO implement me | ||
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. Just curious when these will be implemented, perhaps during the iOS implementation for Pexel? 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. Great questions @cameronvoell ! |
||
} | ||
|
||
@objc | ||
func getOtherMediaOptions(_ filter: [String]?, callback: @escaping RCTResponseSenderBlock) { | ||
//TODO implement me | ||
} | ||
|
||
@objc | ||
func requestMediaImport(_ urlString: String, callback: @escaping RCTResponseSenderBlock) { | ||
|
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.
Perhaps this should be removed with Giphy code?
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.
Good question @cameronvoell!
I was thinking to remove it but then I realize that we might need it as the user could already have
gif
in his Wordpress Library and it would be cool that he can see animatedgif
while editing the Post.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.
Just tested this, very cool!