-
Notifications
You must be signed in to change notification settings - Fork 851
Agents Manager: Enqueue standalone agents manager app #46419
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
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
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.
Pull request overview
This PR enhances the Agents Manager to enqueue scripts and styles from a standalone agents manager application. The implementation dynamically loads different variants (Gutenberg vs wp-admin) based on the current context.
Key Changes:
- Renames
add_inline_script()method toenqueue_scripts()to better reflect its expanded responsibilities - Adds dynamic script/style loading from widgets.wp.com with transient caching and variant support
- Implements block editor detection to determine which variant to load
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php | Adds new methods for enqueueing standalone agents manager app assets with caching, variant detection, and asset fetching logic |
| projects/packages/jetpack-mu-wpcom/changelog/update-agents-manager-load-app | Changelog entry documenting the feature addition |
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
Code Coverage SummaryCoverage changed in 1 file.
Coverage check overridden by
Coverage tests to be added later
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php:221
- The inline script is now added to 'agents-manager' handle, but if
enqueue_script()fails to enqueue the script (e.g., whenget_assets_json()returns null), the inline script will fail to attach to anything, potentially causing a silent failure. Consider checking whether the script was successfully enqueued before adding the inline script data.
wp_add_inline_script(
'agents-manager',
'const agentsManagerData = ' . wp_json_encode(
array(
'agentProviders' => $agent_providers,
'useUnifiedExperience' => $use_unified_experience,
),
JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP
) . ';',
'before'
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
|
Setting up the local dev env. Unrelated to your PR, but to help with testing I think we'll want the |
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/tests/php/features/agents-manager/Agents_Manager_Test.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/agents-manager/class-agents-manager.php
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/tests/php/features/agents-manager/Agents_Manager_Test.php
Show resolved
Hide resolved
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
jom
left a comment
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.
This looks good and seems to work well on WPCOM and WoA.
|
Just saw the test failures that seem related. |
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
Part of https://linear.app/a8c/issue/BSKY-1472
Proposed changes:
Other information:
Does this pull request change what data or activity we track or use?
No
Testing instructions:
jetpack rsyncmethod