-
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
Add Stock photo caption and include it when uploading a photo #13829
Conversation
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
You can test the changes on this Pull Request by downloading the APK here. |
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.
Thanks for these changes @fluiddot I tested this utilizing the instructions you provided and I was able to reproduce the described behavior. This is good to go once the FluxC PR has been approved and merged in and a new tag has been created based on the version bump. So that change will need to take place before this PR's approval.
@fluiddot FluxC PR has been approved via wordpress-mobile/WordPress-FluxC-Android#1846 (review) You may now mark them as "Ready For Review" if you see fit 🙇 |
I'm going to keep this PR in draft because I spotted a new flow that is not working properly. Once I fix it I'll let you know for another review, thanks! Insert multiple images from Media library
|
Caption field has been added to be included in the payload when uploading a stock media item.
f05b80b
to
24b5402
Compare
The issue related to inserting multiple images from Media library is fixed 🎊 . Unfortunately I found out a new issue related to sites with Jetpack, on those sites the caption value is not shown after uploading a stock photo. This means that these changes will only work for regular WP.com sites. The main goal of this PR is to fix the issue about including the photo credit as caption when adding an image from Free Photo library, however these changes will only work for regular WP.com sites, not the ones with Jetpack like Atomic or self-hosted. Not sure if we should block this PR until that is fixed or continue and leave it as an opened issue. What do you think @jd-alexander about the issue?. |
Generated by 🚫 dangerJS |
Thanks @fluiddot I will take a look at this tomorrow and share my findings :) |
@fluiddot I want to reproduce the comment below
So I am on an Atomic site and I am wondering how I recreate this. I added a photo from the device storage and the camera and both didn't have a caption but I am assuming that I have to add it. I haven't explored that in totality as yet as I thought it would be better to just await your comment.
Could you expound a bit more on this comment? the part that says "caption the Image block" isn't clear to me. I am going to sync up with you online to go through this issue more thoroughly. I tested the behavior on a WP.com site and the functionality works as expected. I wanted to also verify something about the Atomic site. How did you go about creating it? I assumed that they are created utilizing the normal WP.com site creation flow but once you add plugins then that's an Atomic site. Let me know the steps you followed. |
Thanks @jd-alexander for testing it 😊 !
The caption field is only included when uploading a photo from the Free Photo Library (Pexels - https://www.pexels.com/), it takes the caption from the photo when is fetched from Pexels and is included in the upload in the caption field. For reproducing the issue, when you you add a photo tap on
Yeah sorry, I typed it wrong 😄 , what I meant is that on Android the caption value that is returned from the upload response is used as the caption value of the Image block. The caption value of the Image block I was referring is the following one:
On regular WP.com sites it works as expected, the problem is on Atomic or self-hosted sites with Jetpack, although I haven't checked it yet, my impression is that is related somehow with Jetpack.
Yeah exactly, an Atomic site is automatically created when you add a plugin on a WP.com site with a Business plan. Another options is to create a self-hosted site with https://jurassic.ninja/ and connect Jetpack. |
Issue: wordpress-mobile/gutenberg-mobile#1609
Related PRs
gutenberg-mobile
gutenberg
WordPress-iOS
WordPress-FluxC-Android
Description
Parse
caption
field from the response of Stock media search requests and add it to the Stock media model and item. This field is now also included in the payload when a photo is uploaded.Test
WellSQL migrations
Verify migrations
Steps
User flows
Test scenarios - Site types:
The changes can be tested in the following described flows:
Media screen
Steps
Media
button.+
button.Choose from Free Photo Library
Caption
field has value.Note: It's possible that in some cases the item doesn't have a caption value with the credit/attribution of the photo. In this case try another one.
Image block / Free Photo Library
Steps
+
button.Image
block.ADD IMAGE
.Choose from Free Photo Library
Note: It's possible that in some cases the item doesn't have a caption value with the credit/attribution of the photo. In this case try another one.
Image block / Insert multiple images from Media library
Steps
Media
button.+
button.Choose from Free Photo Library
+
button.Image
block.ADD IMAGE
.WordPress Media Library
Note: It's possible that in some cases the item doesn't have a caption value with the credit/attribution of the photo. In this case try another one.
Screenshots
Media screen
media-screen-android.mp4
Image block
image-block-android.mp4
Image block / Insert multiple images from Media library
image-block-multiple-android.mp4
Merge Instructions:
Only merge after FluxC PR is merged and tagged, this PR will be updated with correct FluxC version and merged to develop.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.