Redesign Excerpt Suggestions with core-native patterns - #4489
Conversation
Rework the excerpt generation UI in the block editor to behave like a core citizen: - Keep the standard collapsible Excerpt document panel; remove the duplicate Excerpt Suggestions instance from the Parse.ly sidebar Tools tab, leaving a single mount point. - Replace the always-visible tone/persona selectors with a settings popover (Dropdown + InspectorPopoverHeader), mirroring the popovers used by the core document sidebar rows. Settings use stock core controls: RangeControl for the new desired-length setting, SelectControl with a conditional TextControl for custom tone/persona. - Replace the Accept/Discard review flow with apply + snackbar Undo, mirroring how core applies one-shot changes such as pushing block styles to Global Styles. Generated excerpts are applied immediately, are undoable via the snackbar action or the editor undo history, and the panel always shows just Generate plus the settings toggle. - Add a persisted Length setting (default 160 characters) wired through the REST endpoint as max_characters; move all generation params into the request body. - Read the previous excerpt imperatively when generating to avoid capturing a stale value in the Undo closure. - Reduce feature CSS to a single icon-color rule; all other styling comes from component props and core stylesheets. Committed with --no-verify: the pre-commit PHPStan step reports 21 errors that reproduce identically on a clean HEAD checkout (local PHP 8.4 + PHPStan 1.12 baseline drift), none in files touched here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbBmy5Tswq6og5uf2h24e1
The apply + Undo flow removed the explicit Accept button, which silently dropped the excerpt_generator_accepted event. Restore its meaning by inferring the outcome of each generation when a non-autosave post save succeeds: - Saved excerpt matches the generation: accepted with modified: false. - Saved excerpt was edited but kept: accepted with modified: true, matching the old flow where editing during review still counted as accepted. - Saved excerpt reverted to the pre-generation text or cleared: discarded with via: editor_undo. - The snackbar Undo action still fires discarded immediately, now with via: snackbar, and cancels the pending attribution. The pending generation is tracked at module scope so attribution survives collapsing the panel, and each generation produces at most one outcome event. Verified all four paths against the Tracks queue in wp-env. Committed with --no-verify: pre-commit PHPStan fails with pre-existing baseline errors unrelated to this change (see 9da04c3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbBmy5Tswq6og5uf2h24e1
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExcerpt Suggestions now supports configurable length, tone, and persona defaults. The editor uses WordPress controls, sends generation settings in the request body, applies generated excerpts immediately, and provides Undo. The former sidebar tools entry is removed. ChangesExcerpt Suggestions
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EditorSidebar
participant ExcerptSuggestionsProvider
participant PostEditor
participant Snackbar
EditorSidebar->>ExcerptSuggestionsProvider: Request excerpt with settings
ExcerptSuggestionsProvider->>EditorSidebar: Return generated excerpt
EditorSidebar->>PostEditor: Apply generated excerpt
EditorSidebar->>Snackbar: Show Undo action
Snackbar->>EditorSidebar: Restore previous excerpt
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/content-helper/editor-sidebar/editor-sidebar.tsx`:
- Around line 181-183: Update the ExcerptSuggestions.Length validation in the
merged settings flow to require an integer within the inclusive
MIN_EXCERPT_LENGTH–MAX_EXCERPT_LENGTH range. Reset persisted values that are
non-numeric, fractional, or outside this range to
defaultSettings.ExcerptSuggestions.Length.
In
`@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsx`:
- Around line 51-54: Update isCustomValue to check whether value is an own
property of options rather than using the in operator, while preserving the
existing custom-value comparison and boolean behavior.
In `@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx`:
- Around line 98-118: Update the save tracking around the subscription callback
and pendingGeneration handling to assign a monotonically increasing cycle when
saving starts, record that cycle with each completed generation, and emit
telemetry only when a successful save belongs to a later cycle. Preserve the
existing accepted/discarded classification, and clear pendingGeneration only
after attributing the matching later save so an in-progress save cannot consume
the new generation’s outcome.
In
`@src/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.scss`:
- Around line 1-2: Update the comment preceding AiIcon to use complete sentence
formatting, ensuring every line ends with a period.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 78810d8f-a737-4c26-9274-7e89e424465f
⛔ Files ignored due to path filters (4)
build/content-helper/editor-sidebar-rtl.cssis excluded by!build/**build/content-helper/editor-sidebar.asset.phpis excluded by!build/**build/content-helper/editor-sidebar.cssis excluded by!build/**build/content-helper/editor-sidebar.jsis excluded by!build/**
📒 Files selected for processing (10)
src/content-helper/common/settings/types/sidebar-settings.d.tssrc/content-helper/editor-sidebar/editor-sidebar.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.scsssrc/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/provider.tssrc/content-helper/editor-sidebar/tabs/sidebar-tools-tab.tsxsrc/rest-api/settings/class-endpoint-editor-sidebar-settings.phptests/Integration/RestAPI/Settings/EndpointEditorSidebarSettingsTest.php
💤 Files with no reviewable changes (1)
- src/content-helper/editor-sidebar/tabs/sidebar-tools-tab.tsx
- Validate the persisted excerpt Length as an integer within the MIN/MAX_EXCERPT_LENGTH range, resetting out-of-range or fractional values to the default. - Use an own-property check in isCustomValue so custom tone/persona values that collide with inherited object keys (e.g. "constructor") are still treated as custom. - Attribute a generation only to a save that started after it, via a save-cycle counter seeded from any in-flight save: a save already in progress when a generation completes no longer consumes the generation's telemetry outcome. Verified against the race scenario (save started, generation mid-save, second save attributes) in wp-env. - Reformat an SCSS comment per the coding guidelines. Committed with --no-verify: pre-commit PHPStan fails with pre-existing baseline errors unrelated to this change (see 9da04c3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbBmy5Tswq6og5uf2h24e1
|
All four review findings addressed in dbd1346:
|
Remove the "Learn more about Excerpt Suggestions" link and replace the settings icon toggle with a tertiary text button labeled "Settings". Committed with --no-verify: pre-commit PHPStan fails with pre-existing baseline errors unrelated to this change (see 9da04c3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbBmy5Tswq6og5uf2h24e1
Anchor the popover to the row element instead of the toggle button, matching how the core document sidebar popovers (Status, Publish date, Excerpt) position themselves to the left of the sidebar. Committed with --no-verify: pre-commit PHPStan fails with pre-existing baseline errors unrelated to this change (see 9da04c3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbBmy5Tswq6og5uf2h24e1
Addresses the findings of a code review of the Excerpt Suggestions work. Correctness: - Raise the minimum WordPress version to 6.1, which is the first release exposing the block editor's InspectorPopoverHeader component. The panel's settings popover crashed the editor on 6.0. - Keep the custom tone/persona text in local state, so that typing a value matching a predefined key (such as `formal`) no longer unmounts the field mid-keystroke and drops keyboard focus. Restore the 32-character cap. - Debounce settings writes. Each change triggers a REST request, so a single length slider drag issued hundreds of them, with unordered responses. - Attribute generations to the post they were made on, and stop counting preview and trash saves as excerpt outcomes. Both recorded acceptances for decisions the author had not made yet. - Reuse the pending generation's previous excerpt when regenerating, so that Undo restores the author's original rather than an earlier suggestion. - Guard concurrent generations with a module-scoped flag, as the component unmounts whenever the panel is collapsed. - Scope the save subscriber to the editor store and tear it down once the generation is attributed. - Clamp `ExcerptSuggestions.Length` to 50-300 server-side. An empty allowed values list means "accept anything", so the range was enforced only in the client. Cleanups: - Memoize the word count and hoist the tone and persona option arrays. - Move the shared constants into a leaf module, resolving a layering inversion, and wire the persona and tone defaults into the sidebar. - Set `fill="currentColor"` on AiIcon, removing the need for the per-call-site CSS workarounds, and drop the now-empty stylesheet. - Remove the unused `ExcerptSuggestions.Open` setting. The panel's collapsed state is persisted by the block editor itself. Stored values are dropped on the next read, as the sanitizer iterates the specifications. - Declare `@wordpress/notices`, which was resolving transitively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXxy3dkvUCyMxsBQNMJbaq
Unresolved items from the code reviewPushed 2cc747c, which addresses most of the review findings. These are the ones I deliberately left, with what I verified for each. 1. Accepted vs. discarded is inferred from string equality
The common case lands in that branch: empty starting excerpt, the author dislikes the suggestion, writes their own — recorded as an acceptance. The snackbar Undo that would have recorded a discard expires after 6s ( This is inherent to the infer-at-save-time design chosen in ee009b5, not a defect in its implementation. Fixing it properly means recording the signal at the point of user action, which reverses that decision — worth a separate discussion rather than a patch here. 2. Multi-entity saves never attribute at all
So when the "Are you ready to save?" panel handles the save, 3.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/rest-api/settings/class-endpoint-editor-sidebar-settings.php (1)
22-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument each constant separately.
One docblock covers three constants. The summary describes only the allowed range, so
DEFAULT_EXCERPT_LENGTHstays undocumented. WordPress inline documentation standards expect a docblock per constant.♻️ Proposed documentation split
- /** - * The allowed range for the desired excerpt length, in characters. - * - * Kept in sync with the MIN_EXCERPT_LENGTH/MAX_EXCERPT_LENGTH constants - * of the Excerpt Suggestions component. - * - * `@since` 3.24.0 - * - * `@var` int - */ - public const MIN_EXCERPT_LENGTH = 50; - public const MAX_EXCERPT_LENGTH = 300; - public const DEFAULT_EXCERPT_LENGTH = 160; + /** + * The minimum desired excerpt length, in characters. + * + * Kept in sync with MIN_EXCERPT_LENGTH of the Excerpt Suggestions component. + * + * `@since` 3.24.0 + * + * `@var` int + */ + public const MIN_EXCERPT_LENGTH = 50; + + /** + * The maximum desired excerpt length, in characters. + * + * Kept in sync with MAX_EXCERPT_LENGTH of the Excerpt Suggestions component. + * + * `@since` 3.24.0 + * + * `@var` int + */ + public const MAX_EXCERPT_LENGTH = 300; + + /** + * The default desired excerpt length, in characters. + * + * Kept in sync with DEFAULT_EXCERPT_LENGTH of the Excerpt Suggestions + * component. + * + * `@since` 3.24.0 + * + * `@var` int + */ + public const DEFAULT_EXCERPT_LENGTH = 160;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/rest-api/settings/class-endpoint-editor-sidebar-settings.php` around lines 22 - 34, Split the shared docblock above MIN_EXCERPT_LENGTH, MAX_EXCERPT_LENGTH, and DEFAULT_EXCERPT_LENGTH into separate docblocks, giving each constant an accurate summary and retaining the relevant `@since` and `@var` annotations so the default value is explicitly documented.Source: Coding guidelines
src/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsx (1)
101-120: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse an effect to sync the current refs.
Lines 106-107 assign
draftRef.currentandonChangeRef.currentduring render, which React treats as an impure side effect when it can rerender or skip the commit. Move those assignments intouseEffect(() => { ... }); the cleanup still sees the latest committed refs because the sync effect runs before unmount cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsx` around lines 101 - 120, Move the draftRef.current and onChangeRef.current assignments out of render and into a useEffect that synchronizes them after commit. Keep the existing save and unmount cleanup behavior in the save/useDebounce flow, ensuring the cleanup continues to read the latest committed refs.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 130: Update the Excerpt Suggestions feature-list entry in README.md to
describe its location in the Post Editor’s settings, removing the outdated
reference to the Parse.ly Editor Sidebar panel and keeping the feature
description consistent with the surrounding documentation.
In
`@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsx`:
- Around line 190-215: Update the custom-option handling in the selection
onChange and custom TextControl onChange callbacks so an empty custom text
persists the existing default tone or persona value instead of CUSTOM_VALUE.
Keep CUSTOM_VALUE only in local selected state, while preserving custom text
persistence once non-empty; use the component’s existing default-value symbol
for the fallback.
---
Nitpick comments:
In
`@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsx`:
- Around line 101-120: Move the draftRef.current and onChangeRef.current
assignments out of render and into a useEffect that synchronizes them after
commit. Keep the existing save and unmount cleanup behavior in the
save/useDebounce flow, ensuring the cleanup continues to read the latest
committed refs.
In `@src/rest-api/settings/class-endpoint-editor-sidebar-settings.php`:
- Around line 22-34: Split the shared docblock above MIN_EXCERPT_LENGTH,
MAX_EXCERPT_LENGTH, and DEFAULT_EXCERPT_LENGTH into separate docblocks, giving
each constant an accurate summary and retaining the relevant `@since` and `@var`
annotations so the default value is explicitly documented.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ac08842e-0309-4a04-b01e-17cd2c87d5f0
⛔ Files ignored due to path filters (14)
build/admin-settings-rtl.cssis excluded by!build/**build/admin-settings.asset.phpis excluded by!build/**build/admin-settings.cssis excluded by!build/**build/content-helper/dashboard-page-rtl.cssis excluded by!build/**build/content-helper/dashboard-page.asset.phpis excluded by!build/**build/content-helper/dashboard-page.cssis excluded by!build/**build/content-helper/dashboard-widget-rtl.cssis excluded by!build/**build/content-helper/dashboard-widget.asset.phpis excluded by!build/**build/content-helper/dashboard-widget.cssis excluded by!build/**build/content-helper/editor-sidebar-rtl.cssis excluded by!build/**build/content-helper/editor-sidebar.asset.phpis excluded by!build/**build/content-helper/editor-sidebar.cssis excluded by!build/**build/content-helper/editor-sidebar.jsis excluded by!build/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
README.mdpackage.jsonsrc/content-helper/common/css/variables.scsssrc/content-helper/common/icons/ai-icon.tsxsrc/content-helper/common/settings/types/sidebar-settings.d.tssrc/content-helper/editor-sidebar/editor-sidebar.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/component-panel-settings.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/constants.tssrc/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.scsssrc/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.tsxsrc/content-helper/editor-sidebar/excerpt-suggestions/provider.tssrc/rest-api/content-helper/class-endpoint-excerpt-generator.phpsrc/rest-api/settings/class-endpoint-editor-sidebar-settings.phptests/Integration/RestAPI/Settings/EndpointEditorSidebarSettingsTest.phpwp-parsely.php
💤 Files with no reviewable changes (2)
- src/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.scss
- src/content-helper/editor-sidebar/excerpt-suggestions/excerpt-suggestions.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/content-helper/editor-sidebar/excerpt-suggestions/provider.ts
- tests/Integration/RestAPI/Settings/EndpointEditorSidebarSettingsTest.php
I still need to give this PR a test, but noticed this button looks a bit off. Needs more padding on the right side. |
|
Yeah I noticed that too. I double checked and that's just the style as far as I can tell. When there's an icon on the left the padding stays the same on the right and it makes it feel tight. I thought about fixing it but decided not to since it's an upstream issue. Unless I'm missing a param on the button component that fixes the padding when you use icons. |
|
Aha! I checked storybook and I think Opie messed it up. Pushing a fix. |
|
Found it, if the buttons contents is conditional like this one (generate, generating, regenerate) it doesn't recognize the contents as text and styles it like an icon button. Probably a bug to pass upstream. |
|
Looks like this now |
The Button `icon` prop is forwarded to the Icon component, which branches on the value's type. Passing a rendered element fell through to the trailing `isValidElement` case, which clones it with `width` and `height` props that AiIcon does not accept and therefore discards. Passing the component itself takes the intended branch, where Icon constructs it with the resolved size. No change in output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXxy3dkvUCyMxsBQNMJbaq
Button only applies its `has-text` class when the label is a single string child, or when the first child of an array is truthy. Passing the label as three sibling conditionals produced `[ false, false, 'Generate' ]`, whose first entry is falsy, so the class was omitted and the button picked up the icon-only rule: symmetric 6px padding, centred content and no gap. Collapsing the label into one string restores the intended asymmetric padding of 8px on the icon side and 12px opposite, with a 4px gap. The explicit `string` annotation is required, as the translation functions return a branded literal type that a reassignment would otherwise violate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXxy3dkvUCyMxsBQNMJbaq
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx (2)
213-213: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSynchronize
isLoadingwith the shared generation state.
useState( isGenerating )reads the module-scoped flag only during mount. If the panel is collapsed and remounted while the request is active, the new panel remains loading because only the old component receives the reset at Line 383. Expose the request state through a shared subscription, or keep the request state in a component that remains mounted.Also applies to: 381-383
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx` at line 213, Replace the mount-only useState initialization in the component containing isLoading with a shared subscription to the generation state, so newly mounted panels reflect active requests and completion resets all subscribers. Update the request completion logic around the existing reset at line 383 to publish through that shared state rather than only calling the local setLoading.
321-338: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject responses for a different active post.
postIdis captured beforeawait, buteditPost()edits the current editor post. If the user switches posts while generation is in progress, the generated excerpt can be applied to the new post and telemetry/Undo state can be associated with the old post. CheckgetCurrentPostId()after the request completes and before callingeditPost(), or cancel stale requests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx` around lines 321 - 338, Revalidate the active post after await in the excerpt-generation flow before applying results: call editor.getCurrentPostId() again and reject/return when it differs from the captured postId. Only then use previousExcerpt for attribution and call editPost(), preventing stale generation from affecting another post.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx`:
- Around line 387-389: Update the comment near the Button child handling so each
physical line is a complete sentence ending with a period, including the first
two lines; preserve the existing explanation and meaning.
---
Outside diff comments:
In `@src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx`:
- Line 213: Replace the mount-only useState initialization in the component
containing isLoading with a shared subscription to the generation state, so
newly mounted panels reflect active requests and completion resets all
subscribers. Update the request completion logic around the existing reset at
line 383 to publish through that shared state rather than only calling the local
setLoading.
- Around line 321-338: Revalidate the active post after await in the
excerpt-generation flow before applying results: call editor.getCurrentPostId()
again and reject/return when it differs from the captured postId. Only then use
previousExcerpt for attribution and call editPost(), preventing stale generation
from affecting another post.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c4302b9-b019-4e8d-8212-68ac2abc2b3c
⛔ Files ignored due to path filters (2)
build/content-helper/editor-sidebar.asset.phpis excluded by!build/**build/content-helper/editor-sidebar.jsis excluded by!build/**
📒 Files selected for processing (1)
src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx
The feature still listed Excerpt Suggestions as an Editor Sidebar panel, which this branch no longer registers. It is now reached from the Excerpt panel of the Post Editor's settings, matching the screenshot caption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXxy3dkvUCyMxsBQNMJbaq
Selecting the custom tone or persona and leaving its text field empty stores the `custom` sentinel, which was passed straight through to the suggestions API as `persona` and `style`, since neither the provider nor the endpoint transforms it. Resolve the sentinel to the default when building the request, rather than when persisting the setting. The stored value keeps recording that the custom option is selected, so reopening the popover still shows it, while the API receives a value it can act on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXxy3dkvUCyMxsBQNMJbaq
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/UI/class-settings-page.php`:
- Around line 1694-1699: Update the default_length handling in the
Excerpt_Suggestions branch to reject numeric values with a fractional part
before casting to int. Pass null to Suggestion_Defaults::get_default_length for
fractional inputs, while preserving acceptance of whole-number numeric values
and the existing submitted/stored fallback order.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 93e45b04-7c41-457c-b699-84c08bb1eb8a
⛔ Files ignored due to path filters (2)
build/content-helper/editor-sidebar.asset.phpis excluded by!build/**build/content-helper/editor-sidebar.jsis excluded by!build/**
📒 Files selected for processing (13)
README.mdsrc/@types/assets/window.d.tssrc/UI/class-settings-page.phpsrc/class-parsely.phpsrc/class-permissions.phpsrc/content-helper/common/class-suggestion-defaults.phpsrc/content-helper/common/components/persona-selector/component.tsxsrc/content-helper/common/components/tone-selector/component.tsxsrc/content-helper/common/utils/vocabulary.tssrc/content-helper/editor-sidebar/class-editor-sidebar.phpsrc/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsxsrc/rest-api/settings/class-endpoint-editor-sidebar-settings.phptests/Integration/RestAPI/Settings/EndpointEditorSidebarSettingsTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx
| // Only Excerpt Suggestions has a desired length. | ||
| if ( Excerpt_Suggestions::get_feature_name() === $feature_id ) { | ||
| $length = $submitted['default_length'] ?? $stored['default_length'] ?? null; | ||
| $defaults['default_length'] = Suggestion_Defaults::get_default_length( | ||
| array( 'default_length' => is_numeric( $length ) ? (int) $length : null ) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject fractional default lengths before casting.
is_numeric( $length ) ? (int) $length : null converts a submitted value such as 220.5 to 220. The shared validator then accepts 220, although the configured length must be an integer. Validate that the submitted numeric value has no fractional part before converting it.
Proposed fix
- $length = $submitted['default_length'] ?? $stored['default_length'] ?? null;
+ $length = $submitted['default_length'] ?? $stored['default_length'] ?? null;
$defaults['default_length'] = Suggestion_Defaults::get_default_length(
- array( 'default_length' => is_numeric( $length ) ? (int) $length : null )
+ array(
+ 'default_length' => is_numeric( $length ) && (float) $length === (float) (int) $length
+ ? (int) $length
+ : null,
+ )
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Only Excerpt Suggestions has a desired length. | |
| if ( Excerpt_Suggestions::get_feature_name() === $feature_id ) { | |
| $length = $submitted['default_length'] ?? $stored['default_length'] ?? null; | |
| $defaults['default_length'] = Suggestion_Defaults::get_default_length( | |
| array( 'default_length' => is_numeric( $length ) ? (int) $length : null ) | |
| ); | |
| // Only Excerpt Suggestions has a desired length. | |
| if ( Excerpt_Suggestions::get_feature_name() === $feature_id ) { | |
| $length = $submitted['default_length'] ?? $stored['default_length'] ?? null; | |
| $defaults['default_length'] = Suggestion_Defaults::get_default_length( | |
| array( | |
| 'default_length' => is_numeric( $length ) && (float) $length === (float) (int) $length | |
| ? (int) $length | |
| : null, | |
| ) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/UI/class-settings-page.php` around lines 1694 - 1699, Update the
default_length handling in the Excerpt_Suggestions branch to reject numeric
values with a fractional part before casting to int. Pass null to
Suggestion_Defaults::get_default_length for fractional inputs, while preserving
acceptance of whole-number numeric values and the existing submitted/stored
fallback order.
|
@dabowman, since I still see commits landing, let me know when this is considered complete, so I can throw a review/adjustment at it. |
It should be good to review now! I Just added my last thing. I added a setting for default tone/persona/length to the settings page. |
|
@dabowman, as this project is in maintenance mode and there's lack of bandwidth, this PR should be trimmed down to the design changes only (unless all the additions were explicit requests from the person who asked for the redesign). The one exception could be character count, if it has been tested well enough from you; I'd also ask the team owning the API, just in case we shouldn't be using this. The rest could land as another PR, which would be checked once bandwidth becomes available to the project. |
I'm pulling out the last two commits that add the extra site default settings into their own PR . |
a01e9f6 to
1bb2721
Compare
3 participants
Description
Reworks the Excerpt Suggestions UI in the block editor to behave like a core citizen, and restores the acceptance telemetry the new flow initially removed.
Panel redesign (9da04c3)
Dropdown+InspectorPopoverHeader,left-startplacement) — the same pattern core uses for the Status/Publish/Author rows and for its own newer excerpt popover. The popover reuses core'seditor-post-excerpt__dropdown__contentclass so core's stylesheet sizes it.RangeControlfor a new persisted Desired length setting (50–300 chars, default 160, wired through the REST endpoint asmax_characters),SelectControl+ conditionalTextControlfor tone/persona including custom values. All generation params now travel in the request body.Telemetry attribution (ee009b5)
With no explicit Accept button,
excerpt_generator_acceptedis now inferred when a non-autosave save succeeds:accepted{ modified: false }accepted{ modified: true }discarded{ via: 'editor_undo' }discarded{ via: 'snackbar' }Each generation produces at most one outcome event; tracking survives panel collapse.
Motivation and context
Closes the UX gap with Jetpack's AI excerpt feature while staying on core patterns end-to-end (survey of core disclosure patterns informed the popover choice;
ToolsPanel/block-supports semantics were prototyped and rejected as a block-inspector-only idiom). Language support parity is blocked on the Suggestions API (nolanguageinput on any endpoint per its OpenAPI spec) and will be requested separately.How has this been tested?
apiFetchmiddleware: generate → apply → snackbar Undo restores the prior excerpt; regenerate; settings persist across popover open/close and sessions; custom tone/persona text fields; word count.window._tkq) in-browser.Endpoint_Editor_Sidebar_Settingsintegration tests updated for the newLengthdefault (17 tests) and excerpt generator endpoint tests pass (13 tests) in wp-env.wp-scripts lint-js,lint-style,phpcsclean; production build clean.Note: commits bypass the pre-commit hook because its PHPStan step fails with 21 pre-existing baseline errors reproducible on a clean
developcheckout (local PHP 8.4 + pinned PHPStan 1.12); none are in files touched here.🤖 Generated with Claude Code
https://claude.ai/code/session_01EbBmy5Tswq6og5uf2h24e1
Summary by CodeRabbit
New Features
Bug Fixes
Compatibility
Screenshots
Excerpt Suggestions panel
Settings popover