feat(advertising): replace placements UI with inline expandable cards#4625
Open
thomasguillot wants to merge 4 commits intotrunkfrom
Open
feat(advertising): replace placements UI with inline expandable cards#4625thomasguillot wants to merge 4 commits intotrunkfrom
thomasguillot wants to merge 4 commits intotrunkfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Advertising Placements screen UI by replacing the toggle + modal workflow with inline, expandable placement cards, adding snackbar confirmations, and enhancing shared UI components to support the new layout.
Changes:
- Replaces modal-based placement settings with inline expandable cards, including enable/edit/cancel flows and snackbar notifications.
- Updates
PlacementControlto hide the provider selector when only one provider is available. - Extends shared UI components: adds 12-column support to
Grid, addshasHeaderBordertoCoreCard, and alignsstyle-core.scsswith WordPress base-style variables.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/wizards/advertising/views/placements/index.js |
Implements the new inline card UI for placements, including enable/edit flows and snackbar notifications. |
src/wizards/advertising/style.scss |
Adds fixed-position snackbar container styling for placement action confirmations. |
src/wizards/advertising/components/placement-control/index.js |
Hides provider select when only one provider exists; refactors layout to a vertical stack. |
packages/components/src/grid/style.scss |
Adds a 12-column grid variant used by the updated placements layout. |
packages/components/src/card/style-core.scss |
Switches to WP base-style variables and adds styles to support borderless headers during inline expansion. |
packages/components/src/card/core-card.js |
Adds hasHeaderBorder prop to allow removing header border/padding for seamless inline expansion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ea6b68b to
9179c61
Compare
- 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
27f4cdb to
1049d6e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/wizards/advertising/views/placements/index.js:48
placementsApiFetchcatches fetch errors but never rethrows/returns a status. Callers can’t reliably tell whether a request failed, which leads to follow-up UI actions (open panel, close panel, snackbars) running even when the API call errored. Consider returning a boolean (or rethrowing) so callers can gate subsequent state changes on success.
const placementsApiFetch = async options => {
try {
const data = await apiFetch( options );
setPlacements( data );
setError( null );
} catch ( err ) {
setError( err );
}
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
Replaces the action card toggle + modal pattern on the Advertising Placements screen with an inline expandable card UI.
Each placement card now supports:
#wpbody) on Enable, Update, and DisableAlso:
CardFormcomponent topackages/components/srcwith title/description/badge/actions/children slots, built-in ESC handling viaonRequestClose, README, and examples in the components demoPlacementControlto derive the effective provider when only one is available, fixing empty ad unit options and missing GAM bidder fields in single-provider setupshasHeaderBorderprop toCoreCardfor seamless inline form expansionGridcomponent (responsive: applies at 1054px+)stick_to_toptoggle to use an immutable state updatefont-weightfor card headings to600at the base__header-contentlevelHow to test the changes in this Pull Request:
Other information: