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

Fix MediaUpload override issue #200

Merged
merged 3 commits into from
Aug 3, 2020
Merged

Fix MediaUpload override issue #200

merged 3 commits into from
Aug 3, 2020

Conversation

ravichdev
Copy link
Contributor

Summary

Fixes #198

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Contributing Guidelines (updates are often made to the guidelines, check it out periodically).

@ravichdev
Copy link
Contributor Author

@derekherman This is a tricky issue, Jetpack's filter on MediaUpload is at priority 11 and AMP's filter is at priority 10. Because of the way Jetpack's filter uses a HOC to return a new wrapped component, there is no way we can keep our custom extended component if we add our filter with a priority greater than 11. Priority less than 10 would result in AMP plugin overwriting our component.

This fix enqueue the featured image script to load after the AMPs script with filter priority 10 so the filter sequence would be AMP -> Our Component -> Jetpack, but this is kind of a hack and not a viable long term solution. I will open an issue on the Jetpack repo suggesting the Jetpack filter to increase the priority to a big number like 100.

@postphotos
Copy link
Contributor

postphotos commented Jul 29, 2020

Tested locally, works for me! Thanks for this.

I will open an issue on the Jetpack repo suggesting the Jetpack filter to increase the priority to a big number like 100.

Please cc me on the ticket when you open this @ravichdev.

php/class-plugin.php Outdated Show resolved Hide resolved
Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

Some small tweaks.

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

👍

@spacedmonkey spacedmonkey merged commit b946d14 into develop Aug 3, 2020
@spacedmonkey spacedmonkey deleted the fix/198-jetpack branch August 3, 2020 12:11
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.

Image Block conflict when using Jetpack and Unsplash
3 participants