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

[Stories] Temporary gutenberg update for media files type #15811

Merged
merged 6 commits into from
Feb 11, 2021

Conversation

bjtitus
Copy link
Contributor

@bjtitus bjtitus commented Feb 5, 2021

Fixes an incorrect type in the GutenbergBridgeDelegate methods for the "media files" block (a.k.a. Stories).

See wordpress-mobile/gutenberg-mobile#3113
See WordPress/gutenberg#28766

Testing

  • Open a post containing a Story block.
  • Attempt to edit the story.
  • An error screen should not appear after this change has been made.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@bjtitus bjtitus added this to the 16.7 milestone Feb 5, 2021
@bjtitus bjtitus self-assigned this Feb 5, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 5, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@bjtitus bjtitus changed the title Temporary gutenberg update for media files type [Stories] Temporary gutenberg update for media files type Feb 5, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 5, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@bjtitus bjtitus requested a review from etoledom February 5, 2021 16:19
@bjtitus bjtitus marked this pull request as ready for review February 5, 2021 16:20
Podfile.lock Outdated
@@ -73,7 +73,7 @@ PODS:
- GTMSessionFetcher/Core (1.5.0)
- GTMSessionFetcher/Full (1.5.0):
- GTMSessionFetcher/Core (= 1.5.0)
- Gutenberg (1.46.0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Just came here from WordPress/gutenberg#28766 and noticed that this is going down a version (1.46.0 to 1.45.0), could your Gutenberg Mobile or Gutenberg branches need updating?

@bjtitus bjtitus requested a review from guarani February 8, 2021 18:34
@bjtitus bjtitus removed the request for review from etoledom February 8, 2021 20:03
@jkmassel
Copy link
Contributor

jkmassel commented Feb 8, 2021

👋 We're freezing 16.7 today, so this PR is being bumped to 16.8. If you need this to be part of the 16.7 release, please merge it into the release/16.7 branch and DM me – I'll be happy to cut a new beta release!

@jkmassel jkmassel modified the milestones: 16.7, 16.8 Feb 8, 2021
Podfile Outdated
@@ -154,7 +154,8 @@ target 'WordPress' do
## Gutenberg (React Native)
## =====================
##
gutenberg :tag => 'v1.46.0'
#gutenberg :tag => 'v1.46.0'
gutenberg :commit => 'dcacde'
Copy link
Contributor

Choose a reason for hiding this comment

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

I undid this change temporarily on my machine, ran rake xcode and ran the project – without the packager running – expecting to see the red error screen, and I didn't get an error when I opened the story post.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't get an error either when running the packager on the Gutenberg Mobile branch, which is a good thing, but I expected to be able to reproduce the error by undoing the above change but wasn't successful.

@guarani guarani self-requested a review February 9, 2021 00:47
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

👋 @bjtitus, I'm not sure if you're planning on merging this PR, or just letting the Gutenberg Mobile changes get incorporated into WPiOS in the next release. I guess your options include:
a. If you want these changes incorporated immediately into WPiOS develop (e.g. because you need to do more work that depends on this), you'll need to run npm run bundle in the gutenberg-mobile directory, commit the results and push to your PR. Then you'll have to update this PR to point to that latest commit
b. However, if this change isn't needed rightaway (i.e. no PRs this week or next depend on it), you can use this PR only for testing. If this is the case, you can close this PR once the Gutenberg and Gutenberg Mobile PRs are ready and merged. In this scenario, this PR is only used for creating an App Center build with your Gutenberg changes.

Let me know if you need a hand with any of this.

@guarani guarani self-requested a review February 11, 2021 19:34
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Tested and looking good!

@bjtitus bjtitus merged commit 1696f43 into develop Feb 11, 2021
@bjtitus bjtitus deleted the stories/gutenberg-type branch February 11, 2021 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants