-
Notifications
You must be signed in to change notification settings - Fork 31
Improve AI Experiments page UI with header, accordion, and entry points #153
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
base: develop
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,6 +42,24 @@ class Settings_Page { | |||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| private const PAGE_SLUG = 'ai-experiments'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * URL pointing to the plugin repository for contributions. | ||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||
| * @since 0.1.0 | ||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||
| * @var string | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| private const CONTRIBUTION_URL = 'https://github.com/WordPress/ai'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * URL pointing to the plugin documentation. | ||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||
| * @since 0.1.0 | ||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||
| * @var string | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| private const DOCUMENTATION_URL = 'https://github.com/WordPress/ai/tree/develop/docs'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Constructor. | ||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -125,8 +143,37 @@ public function render_page(): void { | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $global_enabled = (bool) get_option( Settings_Registration::GLOBAL_OPTION, false ); | ||||||||||||||||||||||||||||||||||||||||
| ?> | ||||||||||||||||||||||||||||||||||||||||
| <div class="wrap"> | ||||||||||||||||||||||||||||||||||||||||
| <h1><?php echo esc_html( get_admin_page_title() ); ?></h1> | ||||||||||||||||||||||||||||||||||||||||
| <div class="wrap ai-experiments-page"> | ||||||||||||||||||||||||||||||||||||||||
| <div class="ai-admin-header"> | ||||||||||||||||||||||||||||||||||||||||
| <div class="ai-admin-header__inner"> | ||||||||||||||||||||||||||||||||||||||||
| <div class="ai-admin-header__left"> | ||||||||||||||||||||||||||||||||||||||||
| <span class="ai-admin-header__icon"> | ||||||||||||||||||||||||||||||||||||||||
| <?php echo \WordPress\AI\get_ai_icon_svg(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
| <?php echo \WordPress\AI\get_ai_icon_svg(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?> | |
| <?php echo \WordPress\AI\get_ai_icon_svg(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Safe: SVG content is from a trusted static file, not user input. ?> |
Copilot
AI
Dec 19, 2025
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.
Entry point validation only checks for 'label' and 'url' fields being non-empty, but doesn't validate that 'url' is actually a valid URL. Consider adding URL validation using esc_url_raw() or filter_var() before outputting to catch malformed URLs from experiment implementations.
| $links[] = sprintf( | |
| '<a href="%s">%s</a>', | |
| esc_url( $action['url'] ), | |
| $action_url = esc_url_raw( (string) $action['url'] ); | |
| if ( empty( $action_url ) ) { | |
| continue; | |
| } | |
| $links[] = sprintf( | |
| '<a href="%s">%s</a>', | |
| esc_url( $action_url ), |
Copilot
AI
Dec 19, 2025
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.
The entry points loop uses continue when label or url is empty, but doesn't validate the array structure before accessing these keys. If an entry point is not an array or is malformed, this could cause PHP notices. Consider adding is_array($action) check before accessing array keys.
| if ( empty( $action['label'] ) || empty( $action['url'] ) ) { | |
| continue; | |
| } | |
| $links[] = sprintf( | |
| '<a href="%s">%s</a>', | |
| esc_url( $action['url'] ), | |
| esc_html( $action['label'] ) | |
| if ( ! is_array( $action ) ) { | |
| continue; | |
| } | |
| $label = $action['label'] ?? ''; | |
| $url = $action['url'] ?? ''; | |
| if ( '' === $label || '' === $url ) { | |
| continue; | |
| } | |
| $links[] = sprintf( | |
| '<a href="%s">%s</a>', | |
| esc_url( $url ), | |
| esc_html( $label ) |
Copilot
AI
Dec 19, 2025
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.
The inline JavaScript should be moved to a separate enqueued script file for better separation of concerns, security, and maintainability. Inline scripts in PHP templates make code harder to test and maintain. Consider creating a separate JS file in src/admin/settings/ and enqueuing it via Asset_Loader.
Copilot
AI
Dec 19, 2025
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.
The JavaScript accordion functionality is added inline on every page load, but it's not checking if experiments with settings actually exist before adding the event listeners. While harmless, this adds unnecessary DOM queries. Consider conditionally rendering the script only when at least one experiment has settings, by checking if any experiment returns true for has_settings() before the loop.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -260,3 +260,63 @@ | |||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Returns the AI icon as an inline SVG. | ||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||
| * @since 0.1.0 | ||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||
| * @param string $width The width of the icon. | ||||||||||||||||||||||||||||||||||||||
| * @param string $height The height of the icon. | ||||||||||||||||||||||||||||||||||||||
| * @return string The AI icon SVG markup. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| function get_ai_icon_svg( string $width = '1em', string $height = '1em' ): string { | ||||||||||||||||||||||||||||||||||||||
| static $svg_content = null; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if ( null === $svg_content ) { | ||||||||||||||||||||||||||||||||||||||
| $svg_path = dirname( __DIR__ ) . '/assets/images/ai-icon.svg'; | ||||||||||||||||||||||||||||||||||||||
| $svg_content = file_exists( $svg_path ) ? file_get_contents( $svg_path ) : ''; | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+277
to
+278
|
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if ( empty( $svg_content ) ) { | ||||||||||||||||||||||||||||||||||||||
| return ''; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Add width and height attributes, and fill="currentColor" for theme compatibility. | ||||||||||||||||||||||||||||||||||||||
| return preg_replace( | ||||||||||||||||||||||||||||||||||||||
| '/<svg\b/', | ||||||||||||||||||||||||||||||||||||||
| sprintf( '<svg width="%s" height="%s" fill="currentColor"', esc_attr( $width ), esc_attr( $height ) ), | ||||||||||||||||||||||||||||||||||||||
| $svg_content, | ||||||||||||||||||||||||||||||||||||||
| 1 | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+286
to
+291
|
||||||||||||||||||||||||||||||||||||||
| return preg_replace( | |
| '/<svg\b/', | |
| sprintf( '<svg width="%s" height="%s" fill="currentColor"', esc_attr( $width ), esc_attr( $height ) ), | |
| $svg_content, | |
| 1 | |
| ); | |
| $result = preg_replace( | |
| '/<svg\b/', | |
| sprintf( '<svg width="%s" height="%s" fill="currentColor"', esc_attr( $width ), esc_attr( $height ) ), | |
| $svg_content, | |
| 1 | |
| ); | |
| if ( null === $result ) { | |
| return $svg_content; | |
| } | |
| return $result; |
Check warning on line 308 in includes/helpers.php
GitHub Actions / Run PHPCS coding standards checks
file_get_contents() is uncached. If the function is being used to fetch a remote file (e.g. a URL starting with https://), please use wpcom_vip_file_get_contents() to ensure the results are cached. For more details, please see: https://docs.wpvip.com/technical-references/code-quality-and-best-practices/retrieving-remote-data/
Copilot
AI
Dec 19, 2025
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.
The base64_encode function can potentially fail with binary data in edge cases. While SVG content is typically safe, consider using phpcs:ignore for the WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode sniff if it triggers, or wrap the call with error handling for robustness.
| $data_uri = 'data:image/svg+xml;base64,' . base64_encode( $svg_content ); | |
| $encoded = base64_encode( $svg_content ); | |
| if ( false === $encoded ) { | |
| $data_uri = ''; | |
| } else { | |
| $data_uri = 'data:image/svg+xml;base64,' . $encoded; | |
| } |
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.
The example in the documentation shows a 'type' field in the entry points array (line 217, 222), but this field is marked as optional in the PHPDoc type annotation and is not validated or used anywhere in the Settings_Page.php implementation. Either document what the 'type' field is intended for and implement its usage, or remove it from the example to avoid confusion.