-
Notifications
You must be signed in to change notification settings - Fork 805
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
Story block: Add more media options to choose from in the editor #19628
Conversation
Caution: This PR has changes that must be merged to WordPress.com |
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Jetpack plugin:
|
53fda1f
to
42cfc56
Compare
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 gave this a try, and the first time media are added, it works great! However I ran into some issues when trying to change media several times in one session:
- Start by adding an empty story block
- Add media from any source
- Tap on 'select media' again, and select at least one image from either Google Photos or Pexels
- The new images are appended correctly, but the previous images become blank
- Repeat step 3
- Notice that the new selection was completely ignored
At step 4 I inspected the block, and it looks like this:
<!-- wp:jetpack/story {"mediaFiles":[{},{},{"alt":"lake and mountain","id":296,"type":"image","caption":"Photo by James Wheeler on \u003ca href=\u0022https://www.pexels.com/photo/lake-and-mountain-417074/\u0022 rel=\u0022nofollow\u0022\u003ePexels.com\u003c/a\u003e","url":"https://website.com/wp-content/uploads/2021/04/pexels-photo-417074.jpeg"}]} -->
<div class="wp-block-jetpack-story wp-story"></div>
<!-- /wp:jetpack/story -->
Interestingly, in the gallery block, once media has been added (from any source), it looks like the options for Google Photos and Pexels are removed, and you can only select media from your media gallery. Perhaps we should do the same?
Significance: patch | ||
Type: other | ||
|
||
Story block: Add more media options to choose from in the editor |
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.
We should make this a Comment as in the other PR.
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.
Type other
is ignored. I can add the Comment
prefix as well 👍
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.
Oh interesting, I didn't know that - is that written down somewhere? I didn't see it in the instructions, maybe they should be updated?
@aforcier Thanks for finding and reporting this bug, I should have noticed 😞 . It's fixed now. I'll give it a more thorough round of testing before putting it back for review (need to test on WPCOM for instance) |
Hey @Tug , I gave this another try, on both Jetpack and WP.com. Things are working smoothly and the bug seems to be fixed 🎉 I did notice though that the edit button is now text (it used to be a pencil icon before this branch), that seems a bit out of place? Before: |
It looks like that may not be the case @Tug - those screenshots were from a self-hosted site. I checked again, this is a WPcom site before and after this branch: This is a self-hosted site: Any idea what's going on? |
So it wasn't actually related to WPCOM vs self-hosted, but kind of:
The logic for this is defined here. This is so that the menu with the different sources to choose from can be injected. We could either leave it as it is or try to update this component to try to re-use the @obenland what do you think? Can it be done without impacting other jetpack blocks? |
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.
Per our discussion, let's not block this PR since keeping the pencil icon is not a straightforward change/involves other components beside the story block. We'll open an issue and look into that separately.
Issue reported in #19742 |
Great news! One last step: head over to your WordPress.com diff, D60536-code, and commit it. Thank you! |
Deployed on WPCOM with Changeset ID 225384 |
Changes proposed in this Pull Request:
This change allows for selecting media in the story block from the same media sources as the gallery block and other visual blocks:
Does this pull request change what data or activity we track or use?
No
Testing instructions: