From 1049d6e4741b6e9e7238bfa39e21b99666c86bf4 Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 2 Apr 2026 20:30:04 +0100 Subject: [PATCH 1/8] feat(advertising): replace placements UI with inline expandable cards - Replace action card toggles with inline expandable CardForm cards - Add CardForm component to packages/components/src with ESC support - Add hasHeaderBorder prop to CoreCard for seamless form expansion - Add 12-column responsive grid support (applies at 1054px+) - Improve PlacementControl to auto-derive provider when only one exists - Fix stick_to_top toggle to use immutable state update - Only close edit panel and show snackbar when update succeeds --- packages/components/src/card-form/README.md | 105 ++++++ packages/components/src/card-form/index.tsx | 88 +++++ packages/components/src/card-form/style.scss | 20 ++ packages/components/src/card/core-card.js | 12 +- packages/components/src/card/style-core.scss | 44 ++- packages/components/src/grid/style.scss | 5 + packages/components/src/index.js | 1 + .../components/placement-control/index.js | 95 ++--- src/wizards/advertising/style.scss | 8 + .../advertising/views/placements/index.js | 331 ++++++++++++------ 10 files changed, 534 insertions(+), 175 deletions(-) create mode 100644 packages/components/src/card-form/README.md create mode 100644 packages/components/src/card-form/index.tsx create mode 100644 packages/components/src/card-form/style.scss diff --git a/packages/components/src/card-form/README.md b/packages/components/src/card-form/README.md new file mode 100644 index 0000000000..4bda9be812 --- /dev/null +++ b/packages/components/src/card-form/README.md @@ -0,0 +1,105 @@ +# CardForm + +A card component for presenting a named setting or feature with an expandable inline form. When open, the card body reveals children (controls, fields, action buttons) and the header border is removed for a seamless look. Intended for lists of items that can each be independently enabled, edited, or configured without leaving the page. + +## Layout rules + +- Stack multiple `CardForm` cards inside a `` — they are designed to appear as a list. +- The `actions` slot sits to the right of the badge. Keep it to one button; if you need multiple actions, use an `HStack` with `expanded={ false }`. +- The form body (`children`) is only mounted when `isOpen` is `true`. + +## States + +| State | `isOpen` | `badge` | `actions` example | +|---|---|---|---| +| **Disabled** | `false` | None | "Enable" (secondary) | +| **Enabling** | `true` | None | "Cancel" (tertiary) | +| **Enabled** | `false` | Success badge | "Edit" (tertiary) | +| **Editing** | `true` | Success badge | "Cancel" (tertiary) | + +## Basic usage — enable/edit pattern + +```tsx +import { __ } from '@wordpress/i18n'; +import { useState } from '@wordpress/element'; +import { Button } from '@wordpress/components'; +import { CardForm } from '../../../../../packages/components/src'; + +const [ isOpen, setIsOpen ] = useState( false ); +const [ isEnabled, setIsEnabled ] = useState( false ); + +const handleClose = () => setIsOpen( false ); + + isOpen ? handleClose() : setIsOpen( true ) }> + { isOpen ? __( 'Cancel', 'newspack-plugin' ) : __( 'Edit', 'newspack-plugin' ) } + + ) : ( + + ) + } + isOpen={ isOpen } + onRequestClose={ handleClose } +> + { /* form controls */ } + + +``` + +## With a custom badge level + +The `badge` prop accepts any `BadgeLevel`. Use `warning` or `error` to communicate a degraded state. + +```tsx +{ __( 'Edit', 'newspack-plugin' ) } } + isOpen={ false } +/> +``` + +## Without a badge + +Omit `badge` (or pass `undefined`) to show no badge at all. + +```tsx + + { __( 'Enable', 'newspack-plugin' ) } + + } + isOpen={ false } +/> +``` + +## Props + +| Prop | Type | Default | Description | +|---|---|---|---| +| `title` | `string` | — | Card heading (**required**) | +| `description` | `string` | — | Supporting text below the title | +| `badge` | `{ text: string; level?: BadgeLevel }` | — | Badge shown next to the actions slot. Omit or pass `undefined` to hide. | +| `actions` | `React.ReactNode` | — | JSX rendered in the header action area (buttons, dropdowns, etc.) | +| `isOpen` | `boolean` | `false` | When `true`, renders `children` in the card body and removes the header border | +| `onRequestClose` | `() => void` | — | Called when the user presses Escape while the form is open | +| `className` | `string` | — | Additional class name applied to the card element | +| `children` | `React.ReactNode` | — | Form content rendered inside the card body when `isOpen` is `true` | + +### `BadgeLevel` + +```ts +type BadgeLevel = 'default' | 'info' | 'success' | 'warning' | 'error'; +``` diff --git a/packages/components/src/card-form/index.tsx b/packages/components/src/card-form/index.tsx new file mode 100644 index 0000000000..c499a95e8c --- /dev/null +++ b/packages/components/src/card-form/index.tsx @@ -0,0 +1,88 @@ +/** + * Card Form component. + * + * A card with an expandable inline form — title, description, optional badge, + * and an actions slot in the header. When `isOpen` is true, children are + * rendered in the card body and the header border is removed for a seamless look. + */ + +/** + * External dependencies + */ +import classnames from 'classnames'; + +/** + * WordPress dependencies + */ +import { useEffect } from '@wordpress/element'; +import { __experimentalHStack as HStack, __experimentalVStack as VStack } from '@wordpress/components'; // eslint-disable-line @wordpress/no-unsafe-wp-apis + +/** + * Internal dependencies + */ +import Badge from '../badge'; +import Card from '../card'; +import './style.scss'; + +type BadgeLevel = 'default' | 'info' | 'success' | 'warning' | 'error'; + +type CardFormProps = { + title: string; + description?: string; + badge?: { + text: string; + level?: BadgeLevel; + }; + /** JSX rendered in the header action area (buttons, etc.). */ + actions?: React.ReactNode; + /** When true, children are shown and the header border is removed. */ + isOpen?: boolean; + /** Called when the user presses Escape while the form is open. */ + onRequestClose?: () => void; + className?: string; + children?: React.ReactNode; +}; + +const CardForm = ( { title, description, badge, actions, isOpen = false, onRequestClose, className, children }: CardFormProps ) => { + useEffect( () => { + if ( ! isOpen || ! onRequestClose ) { + return; + } + const handleKeyDown = ( event: KeyboardEvent ) => { + if ( event.key === 'Escape' ) { + onRequestClose(); + } + }; + document.addEventListener( 'keydown', handleKeyDown ); + return () => document.removeEventListener( 'keydown', handleKeyDown ); + }, [ isOpen, onRequestClose ] ); + + return ( + + +

{ title }

+ { description &&

{ description }

} +
+ + { badge && } + { actions } + + + ), + } } + > + { isOpen && children } +
+ ); +}; + +export default CardForm; diff --git a/packages/components/src/card-form/style.scss b/packages/components/src/card-form/style.scss new file mode 100644 index 0000000000..eb246c04a4 --- /dev/null +++ b/packages/components/src/card-form/style.scss @@ -0,0 +1,20 @@ +/** + * CardForm + */ + +@use "~@wordpress/base-styles/colors" as wp-colors; +@use "~@wordpress/base-styles/variables" as wp; + +.newspack-card-form { + &__title { + font-size: wp.$font-size-large; + font-weight: 600; + line-height: wp.$font-line-height-large; + } + + &__description { + color: wp-colors.$gray-700; + font-size: wp.$font-size-medium; + line-height: wp.$font-line-height-medium; + } +} diff --git a/packages/components/src/card/core-card.js b/packages/components/src/card/core-card.js index 7ccbeb3413..be5a62ccea 100644 --- a/packages/components/src/card/core-card.js +++ b/packages/components/src/card/core-card.js @@ -47,6 +47,7 @@ const CoreCard = ( { noMargin, children = null, hasGreyHeader, + hasHeaderBorder = true, ...otherProps } ) => { const classes = classNames( @@ -81,7 +82,11 @@ const CoreCard = ( { { ( header || icon ) && ( ) } { children && ( -
+
{ children }
) } diff --git a/packages/components/src/card/style-core.scss b/packages/components/src/card/style-core.scss index 47aad36865..a629fc468d 100644 --- a/packages/components/src/card/style-core.scss +++ b/packages/components/src/card/style-core.scss @@ -3,26 +3,27 @@ */ @use "~@wordpress/base-styles/colors" as wp-colors; +@use "~@wordpress/base-styles/variables" as wp-vars; @use "../../../colors/colors.module" as colors; .newspack-card--core, .newspack-wizard .newspack-card--core { - background: white; + background: wp-colors.$white; position: relative; transition: background-color 125ms ease-in-out, box-shadow 125ms ease-in-out, color 125ms ease-in-out; svg { transition: fill 125ms ease-in-out; } &__icon { - border-radius: 50%; + border-radius: wp-vars.$radius-round; display: grid; - height: 48px; + height: wp-vars.$grid-unit-60; place-items: center; - width: 48px; + width: wp-vars.$grid-unit-60; svg { fill: var(--wp-admin-theme-color); - height: 40px; - width: 40px; + height: wp-vars.$grid-unit-50; + width: wp-vars.$grid-unit-50; } .newspack-card--core__has-icon-background-color & { background: var(--wp-admin-theme-color-lighter-10); @@ -35,15 +36,22 @@ h4, h5, h6 { - font-size: 14px; - font-weight: 500; + font-size: wp-vars.$font-size-large; + font-weight: 600; } - .newspack-card--core__icon { - height: 40px; - width: 40px; - svg { - height: 24px; - width: 24px; + .newspack-card--core { + &__header-content { + * { + line-height: wp-vars.$font-line-height-small; + } + } + &__icon { + height: wp-vars.$grid-unit-50; + width: wp-vars.$grid-unit-50; + svg { + height: wp-vars.$grid-unit-30; + width: wp-vars.$grid-unit-30; + } } } } @@ -108,6 +116,14 @@ } } } + &__header--no-border { + border-bottom: none !important; + } + &__body--no-header-border { + > div:not(.components-card__body):not(.components-card__media):not(.components-card__divider) { + padding-top: 0 !important; + } + } &__header--has-actions { .newspack-card--core__header { padding-right: 70px; diff --git a/packages/components/src/grid/style.scss b/packages/components/src/grid/style.scss index 164e9a7c40..445d63a830 100644 --- a/packages/components/src/grid/style.scss +++ b/packages/components/src/grid/style.scss @@ -99,6 +99,11 @@ grid-template-columns: repeat(3, 1fr); } + // Columns 12 + &__columns-12 { + grid-template-columns: repeat(12, 1fr); + } + // Columns 4 &__columns-4 { grid-template-columns: repeat(4, 1fr); diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 149d89e540..c175fc0f88 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -8,6 +8,7 @@ export { default as Button } from './button'; export { default as BoxContrast } from './box-contrast'; export { default as Card } from './card'; export { default as CardFeature } from './card-feature'; +export { default as CardForm } from './card-form'; export { default as CardSettingsGroup } from './card-settings-group'; export { default as CardSortableList } from './card-sortable-list'; export { default as CategoryAutocomplete } from './category-autocomplete'; diff --git a/src/wizards/advertising/components/placement-control/index.js b/src/wizards/advertising/components/placement-control/index.js index b07f33f641..df7395398a 100644 --- a/src/wizards/advertising/components/placement-control/index.js +++ b/src/wizards/advertising/components/placement-control/index.js @@ -7,11 +7,12 @@ */ import { Fragment, useState, useEffect, useMemo } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; +import { __experimentalVStack as VStack } from '@wordpress/components'; // eslint-disable-line @wordpress/no-unsafe-wp-apis /** * Internal dependencies */ -import { Grid, Notice, SelectControl, TextControl } from '../../../../../packages/components/src'; +import { Notice, SelectControl, TextControl } from '../../../../../packages/components/src'; /** * Get select options from object of ad units. @@ -120,65 +121,71 @@ const PlacementControl = ( { return ; } + const showProviderSelect = providers.length > 1; + const effectiveProvider = showProviderSelect ? placementProvider : providers[ 0 ]; + return ( - - onChange( { ...value, provider } ) } - disabled={ disabled } - /> + + { showProviderSelect && ( + onChange( { ...value, provider } ) } + disabled={ disabled } + /> + ) } { onChange( { ...value, ad_unit: data, + ...( ! showProviderSelect && { provider: effectiveProvider?.id } ), } ); } } disabled={ disabled } { ...props } /> - - { placementProvider?.id === 'gam' && - Object.keys( bidders ).map( bidderKey => { - const bidder = bidders[ bidderKey ]; - // translators: %s: bidder name. - const bidderLabel = sprintf( __( '%s Placement ID', 'newspack-plugin' ), bidder.name ); - return ( - { - onChange( { - ...value, - bidders_ids: { - ...value.bidders_ids, - [ bidderKey ]: data, - }, - } ); - } } - { ...props } - /> - ); - } ) } - { placementProvider?.id === 'gam' && - Object.keys( biddersErrors ).map( bidderKey => { - if ( biddersErrors[ bidderKey ] ) { + { effectiveProvider?.id === 'gam' && + Object.keys( bidders ).map( bidderKey => { + const bidder = bidders[ bidderKey ]; + // translators: %s: bidder name. + const bidderLabel = sprintf( __( '%s Placement ID', 'newspack-plugin' ), bidder.name ); return ( - - { biddersErrors[ bidderKey ] } - + { + onChange( { + ...value, + bidders_ids: { + ...value.bidders_ids, + [ bidderKey ]: data, + }, + } ); + } } + { ...props } + /> ); - } - return null; - } ) } + } ) } + { effectiveProvider?.id === 'gam' && + Object.keys( biddersErrors ).map( bidderKey => { + if ( biddersErrors[ bidderKey ] ) { + return ( + + { biddersErrors[ bidderKey ] } + + ); + } + return null; + } ) } + ); }; diff --git a/src/wizards/advertising/style.scss b/src/wizards/advertising/style.scss index fd44a3141b..410f29699e 100644 --- a/src/wizards/advertising/style.scss +++ b/src/wizards/advertising/style.scss @@ -1,3 +1,11 @@ +.newspack-wizard-ads-placements__snackbar { + bottom: 16px; + left: 50%; + position: fixed; + transform: translateX( -50% ); + z-index: 99999; +} + .newspack-ads-display-ads { .newspack-button-card .newspack-notice { margin: 24px 0 0; diff --git a/src/wizards/advertising/views/placements/index.js b/src/wizards/advertising/views/placements/index.js index e5ab190b95..9b2fb7ba2f 100644 --- a/src/wizards/advertising/views/placements/index.js +++ b/src/wizards/advertising/views/placements/index.js @@ -6,21 +6,19 @@ * External dependencies */ import classnames from 'classnames'; -import set from 'lodash/set'; /** * WordPress dependencies */ import apiFetch from '@wordpress/api-fetch'; -import { Fragment, useState, useEffect } from '@wordpress/element'; +import { Fragment, useState, useEffect, createPortal } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; -import { settings } from '@wordpress/icons'; -import { ToggleControl } from '@wordpress/components'; +import { __experimentalHStack as HStack, __experimentalVStack as VStack, Snackbar, ToggleControl } from '@wordpress/components'; // eslint-disable-line @wordpress/no-unsafe-wp-apis /** * Internal dependencies */ -import { ActionCard, Button, Card, Modal, Notice, withWizardScreen } from '../../../../../packages/components/src'; +import { Button, CardForm, Grid, Notice, withWizardScreen } from '../../../../../packages/components/src'; import PlacementControl from '../../components/placement-control'; /** @@ -32,9 +30,12 @@ const Placements = () => { const [ error, setError ] = useState( null ); const [ providers, setProviders ] = useState( [] ); const [ editingPlacement, setEditingPlacement ] = useState( null ); + const [ isEnabling, setIsEnabling ] = useState( false ); + const [ originalData, setOriginalData ] = useState( null ); const [ placements, setPlacements ] = useState( {} ); const [ bidders, setBidders ] = useState( {} ); const [ biddersError, setBiddersError ] = useState( null ); + const [ notices, setNotices ] = useState( [] ); const placementsApiFetch = async options => { try { @@ -51,6 +52,7 @@ const Placements = () => { method: value ? 'POST' : 'DELETE', } ); if ( value ) { + setIsEnabling( true ); setEditingPlacement( placement ); } }; @@ -79,12 +81,20 @@ const Placements = () => { }; const updatePlacement = async placementKey => { setInFlight( true ); - await placementsApiFetch( { - path: `/newspack-ads/v1/placements/${ placementKey }`, - method: 'POST', - data: placements[ placementKey ].data, - } ); + let success = false; + try { + await apiFetch( { + path: `/newspack-ads/v1/placements/${ placementKey }`, + method: 'POST', + data: placements[ placementKey ].data, + } ); + success = true; + setError( null ); + } catch ( err ) { + setError( err ); + } setInFlight( false ); + return success; }; const isEnabled = placementKey => { return placements[ placementKey ].data?.enabled; @@ -113,121 +123,212 @@ const Placements = () => { fetchData(); }, [] ); - // Silently refetch placements data when exiting edit modal. + const cancelEditing = async () => { + if ( isEnabling && editingPlacement ) { + await handlePlacementToggle( editingPlacement )( false ); + } + setIsEnabling( false ); + setOriginalData( null ); + setEditingPlacement( null ); + }; + + // Silently refetch placements data when exiting edit panel. useEffect( () => { if ( ! editingPlacement && initialized ) { placementsApiFetch( { path: '/newspack-ads/v1/placements' } ); } }, [ editingPlacement ] ); - const placement = editingPlacement ? placements[ editingPlacement ] : null; - return ( -

{ __( 'Placements', 'newspack-plugin' ) }

{ ! inFlight && ! providers.length && } -
- { Object.keys( placements ).map( key => { - return ( - setEditingPlacement( key ) } - icon={ settings } - label={ __( 'Placement settings', 'newspack-plugin' ) } - tooltipPosition="bottom center" - /> - ) : null - } - /> - ); - } ) } -
- { editingPlacement && placement && ( - setEditingPlacement( null ) } + +

{ __( 'Placements', 'newspack-plugin' ) }

+ - { error && } - { biddersError && } - { isEnabled( editingPlacement ) && placement.hook_name && ( - - ) } - { placement.hooks && - Object.keys( placement.hooks ).map( hookKey => { - const hook = { - hookKey, - ...placement.hooks[ hookKey ], - }; - return ( - - - - ); - } ) } - { placement.supports?.indexOf( 'stick_to_top' ) > -1 && ( - { - setPlacements( set( { ...placements }, [ editingPlacement, 'data', 'stick_to_top' ], value ) ); - } } - /> - ) } - - - - -
- ) } + { Object.keys( placements ).map( key => { + const placement = placements[ key ]; + const enabled = isEnabled( key ); + const isEditing = editingPlacement === key; + const hasChanges = JSON.stringify( placement.data ) !== JSON.stringify( originalData ); + let hasAdUnit = true; + if ( placement.hook_name ) { + hasAdUnit = !! placement.data?.ad_unit; + } else if ( placement.hooks ) { + hasAdUnit = Object.keys( placement.hooks ).every( hookKey => !! placement.data?.hooks?.[ hookKey ]?.ad_unit ); + } + + return ( + { + if ( isEditing ) { + cancelEditing(); + } else { + setOriginalData( placement.data ); + setEditingPlacement( key ); + } + } } + > + { isEditing ? __( 'Cancel', 'newspack-plugin' ) : __( 'Edit', 'newspack-plugin' ) } + + ) : ( + + ) + } + isOpen={ isEditing } + onRequestClose={ cancelEditing } + className={ classnames( 'newspack-wizard-ads-placement', { + 'newspack-wizard-ads-placement--enabled': enabled, + } ) } + > + + { error && } + { biddersError && } + { ( enabled || isEnabling ) && placement.hook_name && ( + + ) } + { placement.hooks && + Object.keys( placement.hooks ).map( hookKey => { + const hook = { + hookKey, + ...placement.hooks[ hookKey ], + }; + return ( + + ); + } ) } + { placement.supports?.indexOf( 'stick_to_top' ) > -1 && ( + { + setPlacements( { + ...placements, + [ key ]: { + ...placements[ key ], + data: { + ...placements[ key ].data, + stick_to_top: value, + }, + }, + } ); + } } + /> + ) } + + + { ! isEnabling && ( + + ) } + + + + ); + } ) } + + + { notices.length > 0 && + createPortal( +
+ { notices.map( notice => ( + setNotices( prev => prev.filter( n => n.id !== notice.id ) ) }> + { notice.content } + + ) ) } +
, + document.getElementById( 'wpbody' ) ?? document.body + ) }
); }; From 999fa19997e26b8b6b65a3ef8a146279a26de6e5 Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 2 Apr 2026 20:36:19 +0100 Subject: [PATCH 2/8] fix: move css --- packages/components/src/grid/style.scss | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/components/src/grid/style.scss b/packages/components/src/grid/style.scss index 445d63a830..43f8a94670 100644 --- a/packages/components/src/grid/style.scss +++ b/packages/components/src/grid/style.scss @@ -99,11 +99,6 @@ grid-template-columns: repeat(3, 1fr); } - // Columns 12 - &__columns-12 { - grid-template-columns: repeat(12, 1fr); - } - // Columns 4 &__columns-4 { grid-template-columns: repeat(4, 1fr); @@ -122,6 +117,11 @@ grid-template-columns: repeat(6, 1fr); } + // Columns 12 + &__columns-12 { + grid-template-columns: repeat(12, 1fr); + } + // Gutter 48 &__gutter-48 { grid-gap: 48px; @@ -222,4 +222,4 @@ &__tbody + &__tbody { margin-top: -32px; } -} \ No newline at end of file +} From 793d605339b45591cc33b2fbc303c09eca819937 Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 2 Apr 2026 20:40:51 +0100 Subject: [PATCH 3/8] fix: address second round of Copilot review feedback - Guard handlePlacementToggle with inFlight and return success flag - Gate disable close/snackbar on successful response - Disable other cards' actions while a placement is being edited - Derive placementAdUnit from effectiveProvider in PlacementControl - Move font-weight: 600 to base header-content rule, remove from is-small --- packages/components/src/card/style-core.scss | 3 +- .../components/placement-control/index.js | 9 +++--- .../advertising/views/placements/index.js | 30 ++++++++++++++----- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/packages/components/src/card/style-core.scss b/packages/components/src/card/style-core.scss index a629fc468d..b3ee839d01 100644 --- a/packages/components/src/card/style-core.scss +++ b/packages/components/src/card/style-core.scss @@ -37,7 +37,6 @@ h5, h6 { font-size: wp-vars.$font-size-large; - font-weight: 600; } .newspack-card--core { &__header-content { @@ -159,7 +158,7 @@ h6 { align-items: center; color: wp-colors.$gray-900; - font-weight: 500; + font-weight: 600; display: flex; gap: 8px; a { diff --git a/src/wizards/advertising/components/placement-control/index.js b/src/wizards/advertising/components/placement-control/index.js index df7395398a..efb25a9dbd 100644 --- a/src/wizards/advertising/components/placement-control/index.js +++ b/src/wizards/advertising/components/placement-control/index.js @@ -89,13 +89,15 @@ const PlacementControl = ( { const [ biddersErrors, setBiddersErrors ] = useState( {} ); // Ensure incoming value is available otherwise reset to empty values. + const showProviderSelect = providers.length > 1; const placementProvider = useMemo( () => ( value.provider ? providers.find( provider => provider?.id === value.provider ) : null ), [ providers, value.provider ] ); + const effectiveProvider = showProviderSelect ? placementProvider : providers[ 0 ]; const placementAdUnit = useMemo( - () => ( value.ad_unit ? ( placementProvider?.units || [] ).find( u => u.value === value.ad_unit ) : null ), - [ placementProvider, value.ad_unit ] + () => ( value.ad_unit ? ( effectiveProvider?.units || [] ).find( u => u.value === value.ad_unit ) : null ), + [ effectiveProvider, value.ad_unit ] ); useEffect( () => { @@ -121,9 +123,6 @@ const PlacementControl = ( { return ; } - const showProviderSelect = providers.length > 1; - const effectiveProvider = showProviderSelect ? placementProvider : providers[ 0 ]; - return ( diff --git a/src/wizards/advertising/views/placements/index.js b/src/wizards/advertising/views/placements/index.js index 9b2fb7ba2f..6e551feb51 100644 --- a/src/wizards/advertising/views/placements/index.js +++ b/src/wizards/advertising/views/placements/index.js @@ -47,14 +47,25 @@ const Placements = () => { } }; const handlePlacementToggle = placement => async value => { - await placementsApiFetch( { - path: `/newspack-ads/v1/placements/${ placement }`, - method: value ? 'POST' : 'DELETE', - } ); - if ( value ) { + setInFlight( true ); + let success = false; + try { + const data = await apiFetch( { + path: `/newspack-ads/v1/placements/${ placement }`, + method: value ? 'POST' : 'DELETE', + } ); + setPlacements( data ); + setError( null ); + success = true; + } catch ( err ) { + setError( err ); + } + setInFlight( false ); + if ( success && value ) { setIsEnabling( true ); setEditingPlacement( placement ); } + return success; }; const handlePlacementChange = ( placementKey, hookKey ) => value => { const placementData = placements[ placementKey ]?.data; @@ -179,7 +190,7 @@ const Placements = () => { + ) : ( + + ) + } + isOpen={ this.state.cardFormOpen } + onRequestClose={ () => this.setState( { cardFormOpen: false } ) } + > + + {} } /> + + + + + { __( 'Enable', 'newspack-plugin' ) } + + } + isOpen={ false } + /> + +

{ __( 'Badge levels', 'newspack-plugin' ) }

+ + { [ 'success', 'info', 'warning', 'error' ].map( level => ( + + { __( 'Edit', 'newspack-plugin' ) } + + } + isOpen={ false } + /> + ) ) } + +

{ __( 'Newspack Icons', 'newspack-plugin' ) }

From b1e9d96e210e5e3aad70cce66878c78f1f3609cc Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 16 Apr 2026 08:56:53 +0100 Subject: [PATCH 5/8] fix(advertising): guard cancel path + scope dirty-check + simplify notices - cancelEditing now only closes the panel when the disable request succeeds on the Enabling path, and reverts placements[key].data to originalData on the edit path so other cards do not see dirty values - hasChanges is only computed for the editing card and uses lodash.isEqual to avoid key-order footguns with JSON.stringify - notices state collapses to a single notice with a timestamp key so rapid-fire actions no longer collide on React keys - add bidders to the placement-control useEffect dep array so biddersErrors does not go stale once bidders resolve async --- .../components/placement-control/index.js | 2 +- .../advertising/views/placements/index.js | 44 ++++++++++--------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/wizards/advertising/components/placement-control/index.js b/src/wizards/advertising/components/placement-control/index.js index efb25a9dbd..7d366b6b2b 100644 --- a/src/wizards/advertising/components/placement-control/index.js +++ b/src/wizards/advertising/components/placement-control/index.js @@ -117,7 +117,7 @@ const PlacementControl = ( { ); } ); setBiddersErrors( errors ); - }, [ placementProvider, placementAdUnit ] ); + }, [ placementProvider, placementAdUnit, bidders ] ); if ( ! providers.length ) { return ; diff --git a/src/wizards/advertising/views/placements/index.js b/src/wizards/advertising/views/placements/index.js index 6e551feb51..df820b0f71 100644 --- a/src/wizards/advertising/views/placements/index.js +++ b/src/wizards/advertising/views/placements/index.js @@ -6,6 +6,7 @@ * External dependencies */ import classnames from 'classnames'; +import isEqual from 'lodash/isEqual'; /** * WordPress dependencies @@ -35,7 +36,7 @@ const Placements = () => { const [ placements, setPlacements ] = useState( {} ); const [ bidders, setBidders ] = useState( {} ); const [ biddersError, setBiddersError ] = useState( null ); - const [ notices, setNotices ] = useState( [] ); + const [ notice, setNotice ] = useState( null ); const placementsApiFetch = async options => { try { @@ -136,7 +137,20 @@ const Placements = () => { const cancelEditing = async () => { if ( isEnabling && editingPlacement ) { - await handlePlacementToggle( editingPlacement )( false ); + const success = await handlePlacementToggle( editingPlacement )( false ); + if ( ! success ) { + return; + } + } else if ( editingPlacement && originalData ) { + // Revert dirty edits so other cards' hasChanges doesn't see them + // before the silent refetch completes. + setPlacements( { + ...placements, + [ editingPlacement ]: { + ...placements[ editingPlacement ], + data: originalData, + }, + } ); } setIsEnabling( false ); setOriginalData( null ); @@ -167,7 +181,7 @@ const Placements = () => { const placement = placements[ key ]; const enabled = isEnabled( key ); const isEditing = editingPlacement === key; - const hasChanges = JSON.stringify( placement.data ) !== JSON.stringify( originalData ); + const hasChanges = isEditing && ! isEqual( placement.data, originalData ); let hasAdUnit = true; if ( placement.hook_name ) { hasAdUnit = !! placement.data?.ad_unit; @@ -288,12 +302,7 @@ const Placements = () => { // translators: %s: placement name. const updatedContent = sprintf( __( '%s updated.', 'newspack-plugin' ), name ); const savedContent = isEnabling ? enabledContent : updatedContent; - setNotices( [ - { - id: 'placement-saved', - content: savedContent, - }, - ] ); + setNotice( { id: Date.now(), content: savedContent } ); } } > { isEnabling ? __( 'Enable', 'newspack-plugin' ) : __( 'Update', 'newspack-plugin' ) } @@ -314,12 +323,7 @@ const Placements = () => { setEditingPlacement( null ); // translators: %s: placement name. const disabledContent = sprintf( __( '%s disabled.', 'newspack-plugin' ), name ); - setNotices( [ - { - id: 'placement-disabled', - content: disabledContent, - }, - ] ); + setNotice( { id: Date.now(), content: disabledContent } ); } } > { __( 'Disable', 'newspack-plugin' ) } @@ -332,14 +336,12 @@ const Placements = () => { } ) } - { notices.length > 0 && + { notice && createPortal(

- { notices.map( notice => ( - setNotices( prev => prev.filter( n => n.id !== notice.id ) ) }> - { notice.content } - - ) ) } + setNotice( null ) }> + { notice.content } +
, document.getElementById( 'wpbody' ) ?? document.body ) } From 24969ccd429f7a3c75db2dd54d565729f05c0782 Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 16 Apr 2026 08:57:03 +0100 Subject: [PATCH 6/8] feat(card-form): add focus, ARIA, and titleLevel; scope ESC to body - move focus into the body on open and restore it on close - render the body as a labelled region and link it to the title - scope the Escape listener to the open body so multiple open cards do not all close on a single keypress and callers can preventDefault from inner controls without tripping the close - add a titleLevel prop so consumers can pick the heading that fits their document outline - export BadgeLevel from Badge and drop the local duplicate in CardForm - boost the CardForm title weight selector so it beats the CoreCard heading rule without !important, keeping the 600 scoped to CardForm --- packages/components/src/badge/index.tsx | 4 +- packages/components/src/card-form/README.md | 10 +++- packages/components/src/card-form/index.tsx | 58 +++++++++++++++++--- packages/components/src/card-form/style.scss | 14 +++-- 4 files changed, 69 insertions(+), 17 deletions(-) diff --git a/packages/components/src/badge/index.tsx b/packages/components/src/badge/index.tsx index d54649d788..a782861ab4 100644 --- a/packages/components/src/badge/index.tsx +++ b/packages/components/src/badge/index.tsx @@ -8,9 +8,11 @@ import './style.scss'; */ import classnames from 'classnames'; +export type BadgeLevel = 'default' | 'info' | 'success' | 'warning' | 'error'; + type BadgeProps = { text: string; - level?: 'default' | 'info' | 'success' | 'warning' | 'error'; + level?: BadgeLevel; }; /** diff --git a/packages/components/src/card-form/README.md b/packages/components/src/card-form/README.md index 4bda9be812..6a7636b025 100644 --- a/packages/components/src/card-form/README.md +++ b/packages/components/src/card-form/README.md @@ -94,10 +94,18 @@ Omit `badge` (or pass `undefined`) to show no badge at all. | `badge` | `{ text: string; level?: BadgeLevel }` | — | Badge shown next to the actions slot. Omit or pass `undefined` to hide. | | `actions` | `React.ReactNode` | — | JSX rendered in the header action area (buttons, dropdowns, etc.) | | `isOpen` | `boolean` | `false` | When `true`, renders `children` in the card body and removes the header border | -| `onRequestClose` | `() => void` | — | Called when the user presses Escape while the form is open | +| `onRequestClose` | `() => void` | — | Called when the user presses Escape while focus is inside the open form | +| `titleLevel` | `1 \| 2 \| 3 \| 4 \| 5 \| 6` | `3` | Heading level rendered for `title`. Pick the level that fits the surrounding document outline. | | `className` | `string` | — | Additional class name applied to the card element | | `children` | `React.ReactNode` | — | Form content rendered inside the card body when `isOpen` is `true` | +## Accessibility + +- The body is rendered as a `role="region"` labelled by the title, so assistive tech announces it as a named region when focus enters. +- On open, focus moves to the first focusable element in the body (or to the region itself if none exist). On close, focus is restored to whatever was focused before opening — typically the trigger button. +- The Escape listener is scoped to the open form's body, so multiple open cards do not all close on a single keypress. If an inner control needs to consume Escape (for example, to close its own menu), call `event.preventDefault()` and CardForm will ignore it. + + ### `BadgeLevel` ```ts diff --git a/packages/components/src/card-form/index.tsx b/packages/components/src/card-form/index.tsx index c499a95e8c..8376a742eb 100644 --- a/packages/components/src/card-form/index.tsx +++ b/packages/components/src/card-form/index.tsx @@ -14,17 +14,18 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useEffect } from '@wordpress/element'; +import { useEffect, useRef, createElement } from '@wordpress/element'; +import { useInstanceId } from '@wordpress/compose'; import { __experimentalHStack as HStack, __experimentalVStack as VStack } from '@wordpress/components'; // eslint-disable-line @wordpress/no-unsafe-wp-apis /** * Internal dependencies */ -import Badge from '../badge'; +import Badge, { BadgeLevel } from '../badge'; import Card from '../card'; import './style.scss'; -type BadgeLevel = 'default' | 'info' | 'success' | 'warning' | 'error'; +type HeadingLevel = 1 | 2 | 3 | 4 | 5 | 6; type CardFormProps = { title: string; @@ -39,24 +40,59 @@ type CardFormProps = { isOpen?: boolean; /** Called when the user presses Escape while the form is open. */ onRequestClose?: () => void; + /** Heading level for the title. Defaults to 3. */ + titleLevel?: HeadingLevel; className?: string; children?: React.ReactNode; }; -const CardForm = ( { title, description, badge, actions, isOpen = false, onRequestClose, className, children }: CardFormProps ) => { +const CardForm = ( { title, description, badge, actions, isOpen = false, onRequestClose, titleLevel = 3, className, children }: CardFormProps ) => { + const bodyRef = useRef< HTMLDivElement | null >( null ); + const previousActiveRef = useRef< HTMLElement | null >( null ); + const instanceId = useInstanceId( CardForm, 'newspack-card-form' ); + const titleId = `${ instanceId }__title`; + const bodyId = `${ instanceId }__body`; + + // Scope Escape handling to the open form's body, so multiple open CardForms + // don't all close on a single keypress, and callers can preventDefault from + // inner controls (e.g. select menus) without tripping the close. useEffect( () => { if ( ! isOpen || ! onRequestClose ) { return; } + const node = bodyRef.current; + if ( ! node ) { + return; + } const handleKeyDown = ( event: KeyboardEvent ) => { - if ( event.key === 'Escape' ) { + if ( event.key === 'Escape' && ! event.defaultPrevented ) { onRequestClose(); } }; - document.addEventListener( 'keydown', handleKeyDown ); - return () => document.removeEventListener( 'keydown', handleKeyDown ); + node.addEventListener( 'keydown', handleKeyDown ); + return () => node.removeEventListener( 'keydown', handleKeyDown ); }, [ isOpen, onRequestClose ] ); + // Move focus into the body on open and restore it to the trigger on close. + useEffect( () => { + if ( ! isOpen ) { + return; + } + const node = bodyRef.current; + if ( node ) { + previousActiveRef.current = ( node.ownerDocument?.activeElement ?? null ) as HTMLElement | null; + const focusable = node.querySelector< HTMLElement >( + 'button:not([disabled]), [href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])' + ); + ( focusable ?? node ).focus(); + } + return () => { + previousActiveRef.current?.focus?.(); + }; + }, [ isOpen ] ); + + const titleTag = `h${ titleLevel }`; + return ( -

{ title }

+ { createElement( titleTag, { id: titleId, className: 'newspack-card-form__title' }, title ) } { description &&

{ description }

}
@@ -80,7 +116,11 @@ const CardForm = ( { title, description, badge, actions, isOpen = false, onReque ), } } > - { isOpen && children } + { isOpen && ( +
+ { children } +
+ ) }
); }; diff --git a/packages/components/src/card-form/style.scss b/packages/components/src/card-form/style.scss index eb246c04a4..1b5cc6d7cc 100644 --- a/packages/components/src/card-form/style.scss +++ b/packages/components/src/card-form/style.scss @@ -6,15 +6,17 @@ @use "~@wordpress/base-styles/variables" as wp; .newspack-card-form { - &__title { - font-size: wp.$font-size-large; - font-weight: 600; - line-height: wp.$font-line-height-large; - } - &__description { color: wp-colors.$gray-700; font-size: wp.$font-size-medium; line-height: wp.$font-line-height-medium; } } + +// Scoped inside __header-content to beat the CoreCard h1..h6 heading weight +// selector without resorting to !important. +.newspack-card--core__header-content .newspack-card-form__title { + font-size: wp.$font-size-large; + font-weight: 600; + line-height: wp.$font-line-height-large; +} From 783115e2a7baf235c02bd21809120bfea759c7a2 Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 16 Apr 2026 08:57:09 +0100 Subject: [PATCH 7/8] fix(card): replace !important with specificity and revert heading weight - drop !important on header--no-border and body--no-header-border in favour of two-class selectors, so consumers can still override - revert the global __header-content heading font-weight back to 500 so CoreCard callers outside placements are not silently restyled; the 600 weight is now scoped to CardForm titles only --- packages/components/src/card/style-core.scss | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/components/src/card/style-core.scss b/packages/components/src/card/style-core.scss index b3ee839d01..a39d791b99 100644 --- a/packages/components/src/card/style-core.scss +++ b/packages/components/src/card/style-core.scss @@ -115,12 +115,14 @@ } } } - &__header--no-border { - border-bottom: none !important; + // Two classes beat the single-class .components-card__header border-bottom + // rule from WP Core without needing !important. + &__header.newspack-card--core__header--no-border { + border-bottom: none; } - &__body--no-header-border { + &__body.newspack-card--core__body--no-header-border { > div:not(.components-card__body):not(.components-card__media):not(.components-card__divider) { - padding-top: 0 !important; + padding-top: 0; } } &__header--has-actions { @@ -158,7 +160,7 @@ h6 { align-items: center; color: wp-colors.$gray-900; - font-weight: 600; + font-weight: 500; display: flex; gap: 8px; a { From cfd7d951840f3f7bd694afa0ae7e8d0d5eeed278 Mon Sep 17 00:00:00 2001 From: Thomas Guillot Date: Thu, 16 Apr 2026 09:00:44 +0100 Subject: [PATCH 8/8] fix(card): remove !important from header-border modifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops !important on header--no-border and body--no-header-border in favour of two-class selectors, so consumers can still override without escalating further. The global h1-h6 font-weight: 600 in __header-content stays as-is — that is intentional alignment with WP Core card headers. --- packages/components/src/card-form/style.scss | 14 ++++++-------- packages/components/src/card/style-core.scss | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/components/src/card-form/style.scss b/packages/components/src/card-form/style.scss index 1b5cc6d7cc..eb246c04a4 100644 --- a/packages/components/src/card-form/style.scss +++ b/packages/components/src/card-form/style.scss @@ -6,17 +6,15 @@ @use "~@wordpress/base-styles/variables" as wp; .newspack-card-form { + &__title { + font-size: wp.$font-size-large; + font-weight: 600; + line-height: wp.$font-line-height-large; + } + &__description { color: wp-colors.$gray-700; font-size: wp.$font-size-medium; line-height: wp.$font-line-height-medium; } } - -// Scoped inside __header-content to beat the CoreCard h1..h6 heading weight -// selector without resorting to !important. -.newspack-card--core__header-content .newspack-card-form__title { - font-size: wp.$font-size-large; - font-weight: 600; - line-height: wp.$font-line-height-large; -} diff --git a/packages/components/src/card/style-core.scss b/packages/components/src/card/style-core.scss index a39d791b99..1d2b256f42 100644 --- a/packages/components/src/card/style-core.scss +++ b/packages/components/src/card/style-core.scss @@ -160,7 +160,7 @@ h6 { align-items: center; color: wp-colors.$gray-900; - font-weight: 500; + font-weight: 600; display: flex; gap: 8px; a {