Connect inline banner to the content suggestions modal flow#23156
Conversation
…ggestions" button
…oved outline handling and modal state management
…anagement and remove unnecessary props
…proving feature modal interactions
JorPV
added
the
changelog: non-user-facing
There was a problem hiding this comment.
Pull request overview
Connects the inline Content Planner banner’s “Get content suggestions” CTA to the existing Content Planner modal flow by centralizing modal state in a shared WordPress data store (usable across the block editor iframe boundary).
Changes:
- Introduces a
yoast-seo/content-plannerWP data store to control FeatureModal open/close state and whether to skip the approve step. - Refactors the modal architecture so both the sidebar button and inline banner dispatch into the same shared modal instance (rendered via
ContentPlannerEditorPlugin). - Updates/extends unit tests around modal flow behavior, initialization, and store state.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/js/src/ai-content-planner/store/index.js | Adds a small WP data store for FeatureModal UI state (open + skipApprove). |
| packages/js/src/ai-content-planner/constants.js | Introduces FEATURE_MODAL_STORE constant for the store name. |
| packages/js/src/ai-content-planner/initialize.js | Registers the store and renders a shared FeatureModal from the editor plugin; removes banner block after outline add (stub behavior). |
| packages/js/src/ai-content-planner/components/feature-modal.js | Adds initialStatus support and reuses isEmptyCanvas to skip replace confirmation; extracts getOutlineHandler. |
| packages/js/src/ai-content-planner/block.js | Wires inline banner button to openModal(true) (skip approve) via the shared store. |
| packages/js/src/ai-content-planner/components/inline-banner.js | Adds onClick prop and attaches it to the CTA button. |
| packages/js/src/ai-content-planner/components/content-planner-editor-item.js | Changes sidebar button to dispatch openModal(false) instead of owning a local modal instance. |
| packages/js/src/ai-content-planner/containers/content-planner-editor-item.js | Removes withSelect HOC and exports the component directly (modal state is now global). |
| packages/js/tests/ai-content-planner/components/feature-modal.test.js | Extends tests to cover initialStatus and replace-confirmation skipping behavior. |
| packages/js/tests/ai-content-planner/initialize.test.js | Updates mocks and adds assertions for rendering the modal based on store state. |
| packages/js/tests/ai-content-planner/store/index.test.js | Adds store tests (but currently re-implements store logic inline). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { createReduxStore, createRegistry } from "@wordpress/data"; | ||
| import { FEATURE_MODAL_STORE } from "../../../src/ai-content-planner/constants"; | ||
|
|
||
| describe( "content planner store", () => { | ||
| let registry; | ||
|
|
||
| beforeEach( () => { | ||
| registry = createRegistry(); | ||
| const store = createReduxStore( FEATURE_MODAL_STORE, { | ||
| reducer( state = { isOpen: false, skipApprove: false }, action ) { | ||
| switch ( action.type ) { | ||
| case "OPEN_MODAL": | ||
| return { ...state, isOpen: true, skipApprove: Boolean( action.skipApprove ) }; | ||
| case "CLOSE_MODAL": | ||
| return { isOpen: false, skipApprove: false }; | ||
| default: | ||
| return state; | ||
| } | ||
| }, | ||
| actions: { | ||
| openModal( skipApprove = false ) { | ||
| return { type: "OPEN_MODAL", skipApprove }; | ||
| }, | ||
| closeModal() { | ||
| return { type: "CLOSE_MODAL" }; | ||
| }, | ||
| }, | ||
| selectors: { | ||
| selectIsModalOpen( state ) { | ||
| return state.isOpen; | ||
| }, | ||
| selectShouldSkipApprove( state ) { | ||
| return state.skipApprove; | ||
| }, | ||
| }, | ||
| } ); |
There was a problem hiding this comment.
This test re-implements the store (reducer/actions/selectors) inline instead of importing the production store from src/ai-content-planner/store. As a result, it can pass even if the real store implementation changes or breaks. Consider exporting the store configuration (or a createStore helper) from the source module and registering that in the test registry so the tests exercise the actual code.
| import { createReduxStore, createRegistry } from "@wordpress/data"; | |
| import { FEATURE_MODAL_STORE } from "../../../src/ai-content-planner/constants"; | |
| describe( "content planner store", () => { | |
| let registry; | |
| beforeEach( () => { | |
| registry = createRegistry(); | |
| const store = createReduxStore( FEATURE_MODAL_STORE, { | |
| reducer( state = { isOpen: false, skipApprove: false }, action ) { | |
| switch ( action.type ) { | |
| case "OPEN_MODAL": | |
| return { ...state, isOpen: true, skipApprove: Boolean( action.skipApprove ) }; | |
| case "CLOSE_MODAL": | |
| return { isOpen: false, skipApprove: false }; | |
| default: | |
| return state; | |
| } | |
| }, | |
| actions: { | |
| openModal( skipApprove = false ) { | |
| return { type: "OPEN_MODAL", skipApprove }; | |
| }, | |
| closeModal() { | |
| return { type: "CLOSE_MODAL" }; | |
| }, | |
| }, | |
| selectors: { | |
| selectIsModalOpen( state ) { | |
| return state.isOpen; | |
| }, | |
| selectShouldSkipApprove( state ) { | |
| return state.skipApprove; | |
| }, | |
| }, | |
| } ); | |
| import { createRegistry } from "@wordpress/data"; | |
| import { FEATURE_MODAL_STORE } from "../../../src/ai-content-planner/constants"; | |
| import store from "../../../src/ai-content-planner/store"; | |
| describe( "content planner store", () => { | |
| let registry; | |
| beforeEach( () => { | |
| registry = createRegistry(); |
| * FeatureModal controlled by the content planner store. | ||
| * | ||
| * @returns {null} Renders nothing. | ||
| * @returns {JSX.Element|null} The FeatureModal when open, otherwise null. |
There was a problem hiding this comment.
The JSDoc for ContentPlannerEditorPlugin says it returns null when the modal is closed, but the component always returns a <FeatureModal ... /> element (with isOpen controlling its visibility). Update the return documentation to match the actual return value/behavior.
| * @returns {JSX.Element|null} The FeatureModal when open, otherwise null. | |
| * @returns {JSX.Element} The FeatureModal element, with visibility controlled by the isOpen prop. |
| /** | ||
| * Returns the appropriate handler for adding an outline to the post. | ||
| * When the canvas is empty, skips the replace confirmation and applies directly. | ||
| * | ||
| * @param {boolean} isEmptyCanvas Whether the post has content or not. | ||
| * @param {Function} onConfirm The handler that applies the outline directly. | ||
| * @param {Function} onRequest The handler that shows the replace confirmation first. | ||
| * @returns {Function} The appropriate outline handler. | ||
| */ | ||
| const getOutlineHandler = ( isEmptyCanvas, onConfirm, onRequest ) => isEmptyCanvas ? onConfirm : onRequest; |
There was a problem hiding this comment.
getOutlineHandler’s JSDoc for isEmptyCanvas says "Whether the post has content or not", but the boolean is used as "canvas is empty" (true => skip confirmation). Please adjust the parameter description to avoid inverted/ambiguous meaning.
Coverage Report for CI Build 96Coverage increased (+0.009%) to 53.406%Details
Uncovered ChangesCoverage Regressions3 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
…ts to utilize the exported store
…isEmptyPost' for improved clarity and consistency in modal handling
| * | ||
| * @type {string} | ||
| */ | ||
| export const FEATURE_MODAL_STORE = "yoast-seo/content-planner"; |
There was a problem hiding this comment.
If it's a store for the content planner, should we name this constant as CONTENT_PLANNER_STORE ?
Will we need to to save the results from the requests in the store? For example, user can navigate from the outline back to the content suggestion and pick up a different suggestion. we can save it with useState in the feature modal file, just keeping in mind future usage of that store.
There was a problem hiding this comment.
We don't need to save request results in this store, the FeatureModal already handles that flow internally with useState indeed.
Once the blocks PR #23117 is merged, we will have two dedicated stores.
CONTENT_PLANNER_STORE→ UI state (modal open/close, skipApprove, and potentially more UI flags)yoast-seo/post-planner→ data state (suggestions list, selected outline, API loading/error)
| }; | ||
| } ), | ||
| ] )( ContentPlannerEditorItem ); | ||
| export default ContentPlannerEditorItem; |
There was a problem hiding this comment.
| export default ContentPlannerEditorItem; | |
| import { ContentPlannerEditorItem } from "../components/content-planner-editor-item"; | |
| import { compose } from "@wordpress/compose"; | |
| import { withDispatch } from "@wordpress/data"; | |
| import { FEATURE_MODAL_STORE } from "../constants"; | |
| export default compose( [ | |
| withDispatch( ( dispatch ) => { | |
| const { openModal } = dispatch( FEATURE_MODAL_STORE ); | |
| return { | |
| openModal, | |
| }; | |
| } ), | |
| ] )( ContentPlannerEditorItem ); |
Then in the component you just pass it as a prop:
export const ContentPlannerEditorItem = ( { location, openModal } ) => {
const handleClick = useCallback( () => {
openModal( false );
}, [ openModal ] );
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
…nnect-inline-banner-to-display-the-content-suggestion-modal
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
| }, [ isOpen ] ); | ||
| setCameFromApproveModal( initialStatus === FEATURE_MODAL_STATUS.contentSuggestionsLoading ); |
There was a problem hiding this comment.
When opening the modal with initialStatus === contentSuggestionsLoading (the skip-approve flow), cameFromApproveModal is set to true. That makes SuggestionsPanel apply the approve→suggestions cross-fade classes (including a 300ms delay), even though the approve step is being skipped, which can produce a brief empty modal before the skeleton loaders appear. For the skip-approve entry point, set cameFromApproveModal to false (or adjust the transition logic) so the loading view renders immediately.
| setCameFromApproveModal( initialStatus === FEATURE_MODAL_STATUS.contentSuggestionsLoading ); | |
| setCameFromApproveModal( false ); |
…seo into Connect-inline-banner-to-display-the-content-suggestion-modal
…register it and updated tests
|
CR 🚧 Consider store structre after #23117 was merged. |
… constants and remove unnecessary modal store registration
vraja-pro
left a comment
vraja-pro
left a comment
There was a problem hiding this comment.
CR 🚧
- I removed getSuggestionsEnterTransition
- We need some cleanup of the modal store
vraja-pro
deleted the
Connect-inline-banner-to-display-the-content-suggestion-modal
branch
Context
The inline outline banner (
yoast/content-planner-bannerblock) auto-inserts on new empty posts with a "Get content suggestions" button that was not wired to any action. This PR connects that button to the existingFeatureModalflow (Suggestions → Outline → Add blocks) as per de design, creating a shared modal architecture controlled via a WordPress data store that works across the block editor iframe boundary.Summary
Relevant technical choices
@wordpress/datastores are shared across the iframe boundary. A lightweight store was chosen over DOM events or React context, which don't cross the iframe.FeatureModalinstance: Both the sidebar button and inline banner dispatch to the same store, which theContentPlannerEditorPluginsubscribes to. This prevents two modal instances from opening simultaneously.getOutlineHandlerhelper: Extracted to a standalone function to keep theFeatureModalcomponent's cyclomatic complexity within the ESLint maximum of 6.isEmptyCanvasrenamed toisEmptyPost: Renamed across all components and tests to align with the existingisNewPostnaming convention. The prop is used to skip the replace confirmation when the post is empty — semantically correct since the banner only appears on empty posts.onAddOutlineis still a stub: The actual block insertion logic will come from PR 1101 content planner create the content suggestions blocks #23117. Currently,onAddOutlineonly removes the banner block from the editor.Test instructions for the acceptance test before the PR gets merged
Relevant test scenarios
The inline banner only appears in the Block Editor. Verify the sidebar button still works correctly in both Block Editor and Elementor.
Test instructions for QA when the code is in the RC
Impact check
ContentPlannerEditorPlugin)@wordpress/datastore registrationOther environments
Documentation
Quality assurance
Innovation
Fixes https://github.com/Yoast/reserved-tasks/issues/1147