Admin Settings Screen Implementation #51
Conversation
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @[email protected]. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
Ref34t
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #51 +/- ##
============================================
+ Coverage 60.94% 64.13% +3.19%
- Complexity 81 185 +104
============================================
Files 10 17 +7
Lines 425 870 +445
============================================
+ Hits 259 558 +299
- Misses 166 312 +146
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:
|
Ref34t
force-pushed
the
feature/admin-settings-screen-issue-25
branch
from
5d3ada5 to
72b4e56
Compare
jeffpaul
changed the title
| {section.description} | ||
| </p> | ||
| ) : null} | ||
| </div> |
There was a problem hiding this comment.
Just noting that the design of these controls doesn't allow for long descriptions that might detail requirements, have an HTML list of items, or anything HTML.
Screenshot
The fallback method allows for much more control over what is displayed. Looking at the Content Translator demo in the screenshot... without JS, using the render_settings_section() fallback way, you would be able to do something like this example code to explain what the feature does:
<p>
<?php esc_html_e( 'When enabled, this feature provides a block that allows site visitors to translate page content into multiple languages using AI.', 'jp-wp-ai' ); ?>
</p>
<p>
<strong><?php esc_html_e( 'Supported Languages:', 'jp-wp-ai' ); ?></strong>
</p>
<ul style="list-style: disc; margin-left: 20px;">
<?php foreach ( self::SUPPORTED_LANGUAGES as $name ) : ?>
<li><?php echo esc_html( $name ); ?></li>
<?php endforeach; ?>
</ul>
<p>
<em><?php esc_html_e( 'Add the Content Translator block to any post or page to enable front-end translation.', 'jp-wp-ai' ); ?></em>
</p>
I can actually see it flash quickly before the React app overwrites the screen. As far as I can tell, it's not possible to do any extended description with the existing React app in its current state.
I think any experiment more than an example is going to need more space to explain things.
|
@Jameswlepage @JasonTheAdams @felixarntz @swissspidy @dkotter with #61 resolved and merged in, getting y'alls input / reviews / approvals on this PR will help unblock feature development in the plugin; so if you would please chime in here that would be great, thanks! |
This... is a huge amount of code for something as simple as a settings page 😳 At first glance there's a ton of unused boilerplate code with methods that aren't used except in tests, or reimplement existing core functions such as |
…e-25 # Conflicts: # includes/Abstracts/Abstract_Feature.php # includes/Feature_Loader.php # includes/Features/Example_Feature/Example_Feature.php # package-lock.json # package.json
- Change register() to register_enabled_hooks() to avoid overriding final method - Implement required abstract method register_enabled_hooks() - Ensures Title_Generation follows the feature lifecycle pattern
|
Streamlined and refractored the boilerplate to minimize the size of the PR without losing any functionality, also resolved merge conflicts |
|
|
||
| ## Architecture Overview | ||
|
|
||
| The plugin follows a modular, feature-based architecture: |
|
I'm going to close this PR in favor of #84. This PR does a good job of going after a formal settings registration system, but we're simply not at the point where something like this is necessary. We need something really simple right now that we can improve on later, as we get a clearer sense of what we want the settings to become. Thank you for your effort on this, @Ref34t! It's sincerely appreciated and helped prod the conversation. |
7 participants
Summary
Builds a complete admin settings screen with React UI for managing AI experimental features.
Key Components
Architecture
Feature Development Pattern
Features extend Abstract_Feature and follow these rules:
load_feature_metadata()returning id, label, descriptionai_register_settings_sectionshookis_enabled()before registering functional hooksProvides_Settings_Sectiontrait for settings panelsSee Example_Feature for complete reference implementation.
Files Added
Resolves #25