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

Update project to use react-native-url-polyfill package for the URL class #1996

Merged
merged 3 commits into from
Mar 12, 2020

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Mar 10, 2020

Update project to use react-native-url-polyfill package for the URL class.

This is related to this Gutenberg PR that adds full test for the isURL method: WordPress/gutenberg#20537

To test:

  • Make sure the unit tests run correctly
  • Add a video block and insert a video
  • Check if all works correctly ( the video block code uses the isURL method)

@etoledom can you check that the build process error that you saw before no longer occurs?

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.

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Looking great! 🎉
So nice that the issue was solved.

Tested on Android and iOS, tests local and on CI are ✅ on gutnberg and gutenberg-mobile

Thank you @SergioEstevao !

@@ -170,6 +170,7 @@
"react-native-modal": "^6.5.0",
"react-native-safe-area": "^0.5.0",
"react-native-svg": "git+https://github.com/wordpress-mobile/react-native-svg.git#a628e92990a2404e30a0086f168bd2b5b7b4ce96",
"react-native-url-polyfill": "^1.1.2",

Choose a reason for hiding this comment

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

🎉

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.

Looks good!

@cameronvoell cameronvoell force-pushed the issue/update_isURL_to_use_polyfill branch from bd293fc to 49bed94 Compare March 12, 2020 05:38
@cameronvoell cameronvoell merged commit 351877b into develop Mar 12, 2020
@SergioEstevao SergioEstevao deleted the issue/update_isURL_to_use_polyfill branch March 12, 2020 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improves a current area of the editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants