-
Notifications
You must be signed in to change notification settings - Fork 13
Admin Settings Screen Implementation #51
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: trunk
Are you sure you want to change the base?
Changes from 27 commits
72b4e56
013f8e7
379e44f
0fc0a70
7774324
a7eacdc
bfc6154
0194419
08f883c
89394cc
876955d
cf77782
db48a13
b6740e2
6922346
cf7fc73
3035c5f
f11fdf4
20441b0
2aca4bb
87c57b3
48b2b28
d2cc9a5
bedc00a
092ee3b
3bd52bf
0d6537b
b6155d5
ce9c0d0
b40d86b
e799aae
33fe657
af8ef99
dd1c67f
1b1afb8
9d3121a
d5e2afc
b9c502c
fbe3665
57ee557
52a83e7
b7e6e43
716750e
77b7d22
d42255b
229ed80
ba48de1
8f44aad
092d239
6a56e7e
3784468
16ab9e0
5e0f0da
b9714ee
4d40ed4
07bbc1c
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 |
|---|---|---|
|
|
@@ -13,8 +13,8 @@ node_modules/ | |
| .DS_Store | ||
|
|
||
| # Build | ||
| /build/ | ||
| /dist/ | ||
| /build/ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems this change just swaps the position of these two, any reason for that?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there was a commit that removed the build directory and then in a later commit it was added it back, just in a slightly different location
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. resolved |
||
|
|
||
| # Testing | ||
| /tests/wordpress/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,7 +77,7 @@ | |
| "scripts": { | ||
| "format": "phpcbf --standard=phpcs.xml.dist", | ||
| "lint": "phpcs --standard=phpcs.xml.dist", | ||
| "phpstan": "phpstan analyse --memory-limit=1G", | ||
| "stan": "phpstan analyse --memory-limit=1G", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason for this naming change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to have the same way as the rest. can be reverted if it doesn't make sense for you |
||
| "test": "phpunit --strict-coverage" | ||
| }, | ||
| "scripts-descriptions": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,7 +71,7 @@ ai/ | |
| │ └── Example_Feature/ # Each feature in own directory | ||
| │ ├── Example_Feature.php | ||
| │ └── README.md | ||
| ├── admin/ # Admin interface (planned) | ||
| ├── admin/ # Admin settings services, controllers, assets | ||
|
||
| ├── assets/ # CSS, JS, images | ||
| ├── docs/ # Documentation | ||
| │ ├── DEVELOPER_GUIDE.md # This guide | ||
|
|
@@ -213,6 +213,81 @@ Examples of how to use the feature. | |
| Any settings or filters available. | ||
| ``` | ||
|
|
||
| ## Admin Settings Architecture | ||
|
|
||
| The admin settings screen allows site administrators to manage AI Experiments globally and per feature. The PHP services live under `includes/Admin/` and the React application under `src/`. | ||
|
|
||
| ``` | ||
| includes/ | ||
| ├── Admin/ | ||
| │ ├── Admin_Settings_Page.php # Registers the options page and fallback markup | ||
| │ ├── Settings_Page_Assets.php # Enqueues the React bundle when viewing the page | ||
| │ ├── Settings_Payload_Builder.php # Builds the data passed to the React app | ||
| │ └── Settings/ | ||
| │ ├── Feature_Toggles.php # Persists per-feature enable/disable state | ||
| │ ├── Settings_Renderer.php # Renders the fallback UI for the toggle section | ||
| │ ├── Settings_Registry.php # Registry of settings sections registered by features | ||
| │ ├── Settings_Section.php # Immutable value object describing a section | ||
| │ ├── Settings_Service.php # Coordinates registration of toggles, sections, and page | ||
| │ └── Settings_Toggle.php # Manages the global experiments option | ||
| └── Features/ | ||
| └── Traits/ | ||
| └── Provides_Settings_Section.php # Helper trait for feature-owned sections | ||
|
|
||
| src/ | ||
| ├── index.tsx # React entry point mounted on the admin page | ||
| ├── components/ | ||
| │ ├── App.tsx # Top-level application component | ||
| │ ├── FeatureSection.tsx # Card UI for per-feature toggles | ||
| │ └── ToggleSection.tsx # Card UI for the global toggle | ||
| ├── types.ts # Shared payload types | ||
| ├── style.scss # Styles for the settings screen | ||
| └── global.d.ts # Ambient declaration for the payload on window | ||
| ``` | ||
|
|
||
| `includes/bootstrap.php` wires the settings services on the `init` hook via `initialize_admin_settings()`. That function: | ||
|
|
||
| 1. Instantiates the toggle, registry, renderer, payload builder, page assets handler, and admin page controller. | ||
| 2. Registers the shared `Feature_Toggles` service on the `ai_feature_toggles_service` filter so features can receive it through dependency injection. | ||
| 3. Calls `Settings_Service::register()` to hook the global toggle option, expose REST fields, register the admin menu, and trigger section registration with `ai_register_settings_sections`. | ||
|
|
||
| Feature settings panels should be registered inside the `ai_register_settings_sections` hook. The `Provides_Settings_Section` trait streamlines the process: | ||
|
|
||
| ```php | ||
| class Example_Feature extends Abstract_Feature { | ||
| use Provides_Settings_Section; | ||
|
|
||
| public function register(): void { | ||
| add_action( 'ai_register_settings_sections', array( $this, 'register_settings_sections' ) ); | ||
|
|
||
| if ( ! $this->is_enabled() ) { | ||
| return; | ||
| } | ||
|
|
||
| // Register functional hooks only when enabled. | ||
| } | ||
|
|
||
| public function register_settings_sections( Settings_Registry $registry ): void { | ||
| $this->register_feature_settings_section( | ||
| $registry, | ||
| 'example-feature', | ||
| __( 'Example Feature', 'ai' ), | ||
| array( $this, 'render_settings_section' ), | ||
| array( | ||
| 'description' => __( 'Demonstration controls for the example feature.', 'ai' ), | ||
| 'priority' => 20, | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| public function render_settings_section( Settings_Toggle $toggle, Settings_Section $section ): void { | ||
| // Output fallback markup when JavaScript is unavailable. | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| `Settings_Payload_Builder` serializes the registry into a payload consumed by the React app. Each section’s `enabled` state reflects persisted data from `Feature_Toggles`, ensuring the UI mirrors stored values immediately. | ||
|
|
||
| ### Conditional Features | ||
|
|
||
| If your feature has requirements (PHP extensions, other plugins, etc.), implement validation in your constructor: | ||
|
|
@@ -251,21 +326,21 @@ add_action( 'ai_register_features', function( $registry ) { | |
|
|
||
| ### Filtering Default Features | ||
|
|
||
| Modify the list of default feature classes before they are instantiated: | ||
| Modify the list of default feature instances before they are registered: | ||
|
|
||
| ```php | ||
| add_filter( 'ai_default_feature_classes', function( $feature_classes ) { | ||
| add_filter( 'ai_default_features', function( $features, $feature_toggles ) { | ||
| // Add a custom feature | ||
| $feature_classes[] = 'My_Namespace\My_Custom_Feature'; | ||
|
|
||
| // Remove a default feature | ||
| $key = array_search( 'WordPress\AI\Features\Example_Feature\Example_Feature', $feature_classes ); | ||
| if ( false !== $key ) { | ||
| unset( $feature_classes[ $key ] ); | ||
| } | ||
| $features[] = new My_Namespace\My_Custom_Feature( $feature_toggles ); | ||
|
|
||
| return $feature_classes; | ||
| } ); | ||
| // Remove the bundled Example Feature | ||
| return array_filter( | ||
| $features, | ||
| static function ( $feature ) { | ||
| return ! $feature instanceof WordPress\AI\Features\Example_Feature\Example_Feature; | ||
| } | ||
| ); | ||
| }, 10, 2 ); | ||
| ``` | ||
|
|
||
| ### Disabling a Feature | ||
|
|
@@ -371,4 +446,4 @@ GPL-2.0-or-later | |
|
|
||
| --- | ||
|
|
||
| <br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p> | ||
| <br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,63 @@ | |
| * | ||
| * Provides common functionality for all features including enable/disable state. | ||
| * | ||
| * ## Rules for Creating Features: | ||
| * | ||
| * 1. **Implement load_feature_metadata()**: Return an array with 'id', 'label', and 'description'. | ||
| * | ||
| * 2. **Always register settings sections**: In register(), hook into 'ai_register_settings_sections' | ||
| * so your feature appears in the admin UI even when disabled. | ||
| * | ||
| * 3. **Check is_enabled() before functional hooks**: Only register functional hooks (like actions, | ||
| * filters, REST routes) if is_enabled() returns true. This allows users to disable features. | ||
| * | ||
| * 4. **Pass services as parameters**: Don't use global singletons or service locators. Accept | ||
| * services as method parameters (e.g., Settings_Registry passed to register_settings_sections()). | ||
| * | ||
| * 5. **Use Provides_Settings_Section trait**: To add a settings panel, use this trait and pass | ||
| * the registry as the first parameter to register_feature_settings_section(). | ||
| * | ||
| * ## Example: | ||
| * | ||
| * ```php | ||
| * class My_Feature extends Abstract_Feature { | ||
| * use Provides_Settings_Section; | ||
| * | ||
| * protected function load_feature_metadata(): array { | ||
| * return array( | ||
| * 'id' => 'my-feature', | ||
| * 'label' => __( 'My Feature', 'ai' ), | ||
| * 'description' => __( 'Description of my feature.', 'ai' ), | ||
| * ); | ||
| * } | ||
| * | ||
| * public function register(): void { | ||
| * // Always register settings sections. | ||
| * add_action( 'ai_register_settings_sections', array( $this, 'register_settings_sections' ) ); | ||
| * | ||
| * // Only register functional hooks if enabled. | ||
| * if ( ! $this->is_enabled() ) { | ||
| * return; | ||
| * } | ||
| * | ||
| * add_action( 'init', array( $this, 'my_hook' ) ); | ||
| * } | ||
| * | ||
| * public function register_settings_sections( Settings_Registry $registry ): void { | ||
| * $this->register_feature_settings_section( | ||
| * $registry, // Pass as parameter | ||
| * 'my-feature', | ||
| * __( 'My Feature', 'ai' ), | ||
| * array( $this, 'render_settings' ) | ||
| * ); | ||
| * } | ||
| * | ||
| * public function render_settings( Settings_Toggle $toggle, Settings_Section $section ): void { | ||
| * // Render settings UI. | ||
| * } | ||
| * } | ||
| * ``` | ||
| * | ||
| * @since 0.1.0 | ||
| */ | ||
| abstract class Abstract_Feature implements Feature { | ||
|
|
@@ -50,17 +107,28 @@ abstract class Abstract_Feature implements Feature { | |
| */ | ||
| private $enabled = true; | ||
|
|
||
| /** | ||
| * Feature toggles service. | ||
| * | ||
| * @since 0.1.0 | ||
| * @var \WordPress\AI\Admin\Settings\Feature_Toggles|null | ||
| */ | ||
| private $feature_toggles = null; | ||
|
|
||
| /** | ||
| * Constructor. | ||
| * | ||
| * Loads feature metadata and initializes properties. | ||
| * | ||
| * @since 0.1.0 | ||
| * | ||
| * @param \WordPress\AI\Admin\Settings\Feature_Toggles|null $feature_toggles Optional. Feature toggles service for checking enabled state. | ||
| * | ||
| * @throws \WordPress\AI\Exception\Invalid_Feature_Metadata_Exception If feature metadata is invalid. | ||
| */ | ||
| final public function __construct() { | ||
| $metadata = $this->load_feature_metadata(); | ||
| final public function __construct( ?\WordPress\AI\Admin\Settings\Feature_Toggles $feature_toggles = null ) { | ||
| $this->feature_toggles = $feature_toggles; | ||
| $metadata = $this->load_feature_metadata(); | ||
|
|
||
| if ( empty( $metadata['id'] ) ) { | ||
| throw new Invalid_Feature_Metadata_Exception( | ||
|
|
@@ -132,12 +200,19 @@ public function get_description(): string { | |
| /** | ||
| * Checks if feature is enabled. | ||
| * | ||
| * Uses injected Feature_Toggles service to check persisted toggle state. | ||
| * Falls back to default enabled state if service not available. | ||
| * | ||
| * @since 0.1.0 | ||
| * | ||
| * @return bool True if enabled, false otherwise. | ||
| */ | ||
| final public function is_enabled(): bool { | ||
| $enabled = $this->enabled; | ||
| if ( null !== $this->feature_toggles ) { | ||
| $enabled = $this->feature_toggles->is_feature_enabled( $this->id ); | ||
| } else { | ||
| $enabled = $this->enabled; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit confused on the enablement checks. We have:
It's hard to know what the source of truth is. Do have needs for this complexity already? I wonder if the UI could store the enabled/disabled options somewhere and just use those with the existing filter and avoid all the new classes? Admittedly I don't have a full picture of the next steps and/or plans for this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for calling that out! I’ve flattened the enablement flow so there’s now a single source of truth:
|
||
|
|
||
| /** | ||
| * Filters the enabled status for a specific feature. | ||
|
|
||
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.
Looking at the changes here, it seems to avoid permission issues we're adding a comment instead of updating the PR description. I have a separate PR (#65) that fixes permission issues. I'd suggest we remove these changes from this PR and can continue discussion on that other PR to keep things more focused here
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.
Note I've brought the code changes here that ensure we have a PR description over to #65, though I didn't bring over the changes that add a comment instead of updating the PR description
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.
reverted here.