Use toggle UI, auto-save, and inline DataForm settings for experiments#376
Use toggle UI, auto-save, and inline DataForm settings for experiments#376
Conversation
Use the DataForm 'toggle' field type and ToggleControl component for a more intuitive on/off UI on the AI settings page.
Remove manual save button clicks from test helpers since settings now auto-save on toggle.
|
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #376 +/- ##
=============================================
+ Coverage 62.63% 63.81% +1.17%
- Complexity 680 684 +4
=============================================
Files 49 49
Lines 3501 3529 +28
=============================================
+ Hits 2193 2252 +59
+ Misses 1308 1277 -31
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:
|
dkotter
left a comment
There was a problem hiding this comment.
Tested and works well for me, noting this also seems to solve the problem reported in #352, which is great.
There are E2E failures here with Content Classification, I'm assuming because the individual settings it needs no longer display, so I think fine to ignore that until we have a PR up to fix that.
That said, I do wonder how this will scale for features that need their own settings (like Content Classification). Is the plan that anytime you change an individual setting we save immediately, similar to how these toggles work? Seems like in that scenario having a save button still may be best
|
lol Darin 🥇, JP 🥈 |
ToggleControl associates its label differently than CheckboxControl
in the accessibility tree, so getByRole('checkbox', { name }) fails.
Use getByLabel which works reliably with both control types.
Allow experiments to declare custom settings field definitions via get_settings_fields() for DataForm rendering. Add get_settings_fields_metadata() to resolve short field IDs to full WordPress option names for REST API consumption.
Replace render_settings_fields() HTML with get_settings_fields() field definitions. Add show_in_rest with proper schemas to register_settings() so custom fields are accessible via the WordPress REST API and core-data store.
Pass each feature's custom settings field definitions to the settings page script data so the frontend can render DataForms for experiment-specific settings.
Add InlineFeatureSettings component that renders a DataForm with custom fields below the experiment toggle when enabled. Uses local state with a Save button that only appears when changes are pending. The sub-settings appear inline within the same card as the toggle.
Navigate to the new settings page URL and use label-based selectors with snackbar confirmation instead of the old form submission approach.
Cover get_settings_fields(), get_settings_fields_metadata(), and settingsFields in bootstrap feature metadata output. Includes tests for Content_Classification field declarations and show_in_rest.
The enableExperiment helper uses page.getByLabel() which matches against the toggle's display label, not the slug. Updated the constant to use 'Content Classification' matching all other tests.
Move the settings data lookup to a ref so the callback identity stays stable across saves, avoiding unnecessary DataForm re-renders caused by the data dependency changing on every settings update.
Reset localEdits when the feature id changes so stale keys from a previous field definition do not persist into the new one.
Replace coordinated mutable state (savingRef, dataRef, localEdits, editComponentsRef) with core-data's built-in dirty tracking. Toggle auto-save now uses __experimentalSaveSpecifiedEntityEdits to persist only the toggled key, so rapid clicks are never dropped and inline drafts are never flushed. InlineFeatureSettings writes directly to the entity record store and saves only its own field IDs. Update E2E helpers to assert snackbar text instead of test ID.
|
Content Classification settings work as expected, though the horizontal line and ALL CAPS for sub-experiment sections feels off from a UI perspective. Any chance we can remove the separator, switch to Title Case, and left align the sub-experiment settings with the experiment title & description? |
The inline settings Save button was clickable while saving because it
only had isBusy without disabled. Also, isSavingEntityRecord('root',
'site') fired for any site entity save including toggle auto-saves,
causing false spinner activation.
Replace the store-level isSaving with local useState so the button
only reflects the inline settings save, and add disabled={isSaving}
to prevent concurrent requests.
…a11y Output DataForm-compatible field definitions from PHP (use 'text' type, nest min/max under isValid) so the frontend can pass them through without transformation. Remove parseSettingsField and the type-mapping useMemo. Replace the module-level Map component cache with a single stable FeatureToggleWithSettings component that looks up its feature via an immutable FEATURES_BY_SETTING Map. Add aria-label to the inline Save button so screen readers can distinguish between multiple feature settings forms.
…dent - Remove border-top separator between toggle and sub-settings - Override Emotion text-transform: uppercase on BaseControl and InputControl labels with !important - Indent sub-settings with padding-left to align under toggle label - Constrain form width with max-width to prevent overly wide inputs
Hi @jeffpaul, I tried to address your feedback: |
Extract MIN_SUGGESTIONS and MAX_SUGGESTIONS class constants in Content_Classification to eliminate duplicated magic numbers across register_settings, get_settings_fields, and sanitize_max_suggestions. Move the DisabledToggle no-op onChange handler to a module-level constant to avoid allocating a new function on every render.
dkotter
left a comment
There was a problem hiding this comment.
There may still be work being done here but just noting there's a failing eslint check and E2E tests are also still failing
Hi @dkotter it was from in progress work, everything is green now. |




Summary
get_settings_fields()onAbstract_Feature, rendered as inline DataForms below the experiment toggle when enabledshow_in_restsupportTest plan
/wp/v2/settings/REST API returnswpai_feature_content-classification_field_strategyandwpai_feature_content-classification_field_max_suggestionsnpm run test:e2e -- --grep "Plugin settings"andnpm run test:e2e -- --grep "Content Classification"