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

Add iOS Request Interception #33

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft

Conversation

jkmassel
Copy link
Contributor

No description provided.

@jkmassel jkmassel changed the base branch from trunk to add/android-request-interception October 21, 2024 23:12
@dcalhoun dcalhoun force-pushed the add/android-request-interception branch from ac9d15f to 8ed393e Compare October 23, 2024 00:11
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

You mentioned being unhappy, which aspects of this approach cause that?

Comment on lines +382 to +383
// We care about WordPress.com resources – let's modify those if needed
if request.url?.host?.contains("wordpress.com") == true {
Copy link
Member

Choose a reason for hiding this comment

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

Eventually, we should hoist this conditional into the host app, as we want to avoid such references in Gutenberg.

Comment on lines +376 to +377
// We don't want to interfere with loading the editor JS
guard request.url?.host != "localhost" else {
Copy link
Member

Choose a reason for hiding this comment

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

The development server can also be an IP when testing with a physical device. Maybe we can check against the GUTENBERG_EDITOR_URL environment variable as well?

Base automatically changed from add/android-request-interception to trunk November 1, 2024 13:49
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.

2 participants