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

Add support for pexel images #10628

Conversation

marecar3
Copy link
Contributor

Fixes wordpress-mobile/gutenberg-mobile#720

To test:

Follow instructions on gb-mobile PR: wordpress-mobile/gutenberg-mobile#1469

PR submission checklist:

  • I have considered adding unit tests where possible.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 18, 2019

You can test the changes on this Pull Request by downloading the APK here.

for (Map.Entry<String, MediaFile> mediaEntry : mediaList.entrySet()) {
rnMediaList.add(
new Media(
isNetworkUrl ? Integer.valueOf(mediaEntry.getValue().getMediaId()) : mediaEntry.getValue().getId(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Reporter: Checkstyle
Rule: com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck
Severity: ERROR
File: /home/circleci/project/libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergEditorFragment.java L763
Line is longer than 120 characters (found 127).

@hypest
Copy link
Contributor

hypest commented Oct 25, 2019

Noticed a crash on rotation, already existing in previous app versions too but still happening here too: #10700

I feel that will probably need some work to fix so, let's not block this PR on it.

ArrayList<MediaOption> otherMediaOptions = new ArrayList<>();

String packageName = getActivity().getApplication().getPackageName();
int stockMediaResourceId = getResources().getIdentifier("photo_picker_stock_media", "string", packageName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I noticed while product reviewing the PR is that this string can get pretty long in other languages (tested in french) and in that case will be cropped in the selection menu. Not a blocker but we'll probably want to address that in gutenberg (I'm thinking long pressing to make the text scroll for instance).

image

@iamthomasbishop
Copy link

Good catch on the translations, @Tug!

There is probably a larger discussion to be had here regarding what we do in terms of truncation, but I think it's related to an existing proposal I made a while back to add a title/description to this sheet.

Specifically for this issue, what that would mean is adding a section header that is relevant to all items and then shorten the options as a result. That would look like this (Android left, iOS right):

image

  1. Add a table section header string of `Choose image from..."
  2. Shorten the cell text strings to My Device, Camera (or keep as Take a Photo, WordPress Media Library, and Free Photo Library)

Would that make more sense? I've been hoping to add a section header on this sheet for a while, so this might solve that concern as well. This seems clearer and also more scalable for translations. // @marecar3

Copy link
Contributor

@cameronvoell cameronvoell left a comment

Choose a reason for hiding this comment

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

Tested latest, Android updates look good to me!

@marecar3 marecar3 merged commit 48c886e into develop Oct 29, 2019
@marecar3 marecar3 deleted the gutenberg/pexels-and-giphy-search-not-available-from-image-block branch October 29, 2019 19:51
@marecar3 marecar3 changed the title Add support for giphy and pexel images Add support for pexel images Nov 14, 2019
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.

Pexels and Giphy search not available form image block
6 participants