Improve AI Experiments page UI with header, accordion, and entry points #153
Conversation
…oints - Add full-width admin header with AI icon, title, and action buttons - Docs and Contribute links in header for easy access - White background page styling - Update experiment list to grid layout with border separators - Cleaner visual hierarchy with borderless items - Entry point links next to experiment title (Dashboard, Try, etc.) - Settings toggle button with accordion drawer - Add Abstract_Experiment methods for UI extensibility - get_entry_points(): Define contextual links for experiments - has_settings(): Indicate if experiment has custom settings - Add get_ai_icon_svg() helper for inline SVG icons - New _common.scss with shared admin page styles - Header component styles - White page background utilities - Reusable across other admin pages
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #153 +/- ##
=============================================
- Coverage 46.89% 43.63% -3.27%
- Complexity 208 216 +8
=============================================
Files 19 19
Lines 1271 1366 +95
=============================================
Hits 596 596
- Misses 675 770 +95
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add get_entry_points(), has_settings(), render_settings_fields() to Experiment interface - Add false check for file_get_contents() in get_ai_icon_data_uri()
There was a problem hiding this comment.
Pull request overview
This PR modernizes the AI Experiments settings page with a polished admin UI, introducing a full-width header section, cleaner experiment grid layout, contextual entry point links, and an accordion-style settings drawer for experiments with custom configurations.
Key Changes:
- Added
get_entry_points()andhas_settings()methods to the Experiment interface and Abstract_Experiment class for UI extensibility - Introduced full-width admin header with AI icon and action buttons (Docs, Contribute)
- Replaced card-per-experiment layout with a unified grid using border separators
- Implemented collapsible settings drawers with toggle buttons and animations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
includes/Settings/Settings_Page.php |
Added header structure, entry point rendering, and accordion drawer implementation with inline JavaScript |
includes/helpers.php |
Added get_ai_icon_svg() and get_ai_icon_data_uri() helper functions for icon rendering |
includes/Contracts/Experiment.php |
Added interface methods for get_entry_points(), has_settings(), and render_settings_fields() |
includes/Abstracts/Abstract_Experiment.php |
Provided default implementations for new experiment extensibility methods |
src/admin/_common.scss |
New shared admin styles for headers, icons, and white page backgrounds |
src/admin/settings/index.scss |
Updated experiment list to grid layout, added styles for entry points, settings toggle button, and drawer animation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| // Replace currentColor with a neutral color for admin menu compatibility. | ||
| $svg_content = str_replace( 'fill="currentColor"', 'fill="black"', $svg_content ); | ||
| $data_uri = 'data:image/svg+xml;base64,' . base64_encode( $svg_content ); |
There was a problem hiding this comment.
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; | |
| } |
| return preg_replace( | ||
| '/<svg\b/', | ||
| sprintf( '<svg width="%s" height="%s" fill="currentColor"', esc_attr( $width ), esc_attr( $height ) ), | ||
| $svg_content, | ||
| 1 | ||
| ); |
There was a problem hiding this comment.
The preg_replace function could fail and return null. Consider checking the return value or using preg_replace with error handling to ensure the function returns a valid string. You could use a fallback to return the original $svg_content if preg_replace fails.
| 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; |
| <script> | ||
| ( function() { | ||
| document.querySelectorAll( '.ai-experiments__settings-toggle' ).forEach( function( btn ) { | ||
| btn.addEventListener( 'click', function() { | ||
| var expanded = btn.getAttribute( 'aria-expanded' ) === 'true'; | ||
| var drawerId = btn.getAttribute( 'aria-controls' ); | ||
| var drawer = document.getElementById( drawerId ); | ||
| if ( drawer ) { | ||
| btn.setAttribute( 'aria-expanded', String( ! expanded ) ); | ||
| if ( expanded ) { | ||
| drawer.setAttribute( 'hidden', '' ); | ||
| } else { | ||
| drawer.removeAttribute( 'hidden' ); | ||
| } | ||
| } | ||
| } ); | ||
| } ); | ||
| } )(); | ||
| </script> |
There was a problem hiding this comment.
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.
| $svg_path = dirname( __DIR__ ) . '/assets/images/ai-icon.svg'; | ||
| $svg_content = file_exists( $svg_path ) ? file_get_contents( $svg_path ) : ''; |
There was a problem hiding this comment.
The get_ai_icon_svg() and get_ai_icon_data_uri() functions reference a file path 'assets/images/ai-icon.svg' that doesn't appear to exist in the repository. This will cause both functions to return empty strings. Ensure the ai-icon.svg file is added to the assets/images/ directory, or update the path if the file is located elsewhere.
| $links[] = sprintf( | ||
| '<a href="%s">%s</a>', | ||
| esc_url( $action['url'] ), |
There was a problem hiding this comment.
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 ), |
| * 'type' => 'try', | ||
| * ), | ||
| * array( | ||
| * 'label' => __( 'Dashboard', 'ai' ), | ||
| * 'url' => admin_url( 'admin.php?page=ai-mcp' ), | ||
| * 'type' => 'dashboard', | ||
| * ), | ||
| * ); | ||
| * | ||
| * @since 0.1.0 | ||
| * | ||
| * @return array<int, array{label: string, url: string, type?: string}> |
There was a problem hiding this comment.
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.
| * 'type' => 'try', | |
| * ), | |
| * array( | |
| * 'label' => __( 'Dashboard', 'ai' ), | |
| * 'url' => admin_url( 'admin.php?page=ai-mcp' ), | |
| * 'type' => 'dashboard', | |
| * ), | |
| * ); | |
| * | |
| * @since 0.1.0 | |
| * | |
| * @return array<int, array{label: string, url: string, type?: string}> | |
| * ), | |
| * array( | |
| * 'label' => __( 'Dashboard', 'ai' ), | |
| * 'url' => admin_url( 'admin.php?page=ai-mcp' ), | |
| * ), | |
| * ); | |
| * | |
| * @since 0.1.0 | |
| * | |
| * @return array<int, array{label: string, url: string}> |
| <script> | ||
| ( function() { | ||
| document.querySelectorAll( '.ai-experiments__settings-toggle' ).forEach( function( btn ) { | ||
| btn.addEventListener( 'click', function() { | ||
| var expanded = btn.getAttribute( 'aria-expanded' ) === 'true'; | ||
| var drawerId = btn.getAttribute( 'aria-controls' ); | ||
| var drawer = document.getElementById( drawerId ); | ||
| if ( drawer ) { | ||
| btn.setAttribute( 'aria-expanded', String( ! expanded ) ); | ||
| if ( expanded ) { | ||
| drawer.setAttribute( 'hidden', '' ); | ||
| } else { | ||
| drawer.removeAttribute( 'hidden' ); | ||
| } | ||
| } | ||
| } ); | ||
| } ); | ||
| } )(); | ||
| </script> |
There was a problem hiding this comment.
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.
| <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 ?> |
There was a problem hiding this comment.
The phpcs:ignore comment disables output escaping without justification. While get_ai_icon_svg() returns SVG markup that needs to be unescaped, consider adding a comment explaining why this is safe (e.g., "Safe: SVG content is from a trusted static file, not user input").
| <?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. ?> |
| background: #fff; | ||
| border: 0.0625rem solid #ddd; | ||
| border-radius: 0.125rem; | ||
| border-radius: 4px; |
There was a problem hiding this comment.
The border-radius value was changed from '0.125rem' (2px) to '4px'. This introduces an inconsistency in unit usage - the rest of the file uses rem units for measurements. Consider using '0.25rem' instead of '4px' to maintain consistent relative units throughout the stylesheet.
| border-radius: 4px; | |
| border-radius: 0.25rem; |
| if ( empty( $action['label'] ) || empty( $action['url'] ) ) { | ||
| continue; | ||
| } | ||
| $links[] = sprintf( | ||
| '<a href="%s">%s</a>', | ||
| esc_url( $action['url'] ), | ||
| esc_html( $action['label'] ) |
There was a problem hiding this comment.
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 ) |
|
Note that we'll likely need another iteration on this as the admin reskin in WP 7.0 continues onward as we'll want to closely align to those updated UI components to ensure AIE presents as a first party plugin for WPORG. |
|
@Jameswlepage do you think the work here can close both #35 and #103? |
2 participants
Summary
This PR enhances the AI Experiments settings page with improved UI and extensibility:
_common.scssfor reuse across AI admin pages (MCP, Request Logs, etc.)Screenshots
The experiments page now has:
Technical Changes
Abstract_Experiment: Addedget_entry_points()andhas_settings()methods for UI extensibilitySettings_Page.php: New header structure, grid layout, entry points display, and accordion drawer with inline JShelpers.php: Addedget_ai_icon_svg()andget_ai_icon_data_uri()helpers for icon rendering_common.scss: New shared styles for admin headers and white page backgroundssettings/index.scss: Updated styles for grid layout, entry point links, settings toggle, and drawer animationDependency Note
The following feature PRs will need to be rebased onto this PR (or updated after merge) as they also modify the experiments page:
These PRs currently include similar header styling changes that would become redundant after this PR merges. They should be updated to:
_common.scssget_entry_points()method to add their entry point linkshas_settings()method if they have custom settingsTest Plan