Add dynamic banner colors and intent badge tooltips to content planner modals#23152
Conversation
JorPV
added
the
changelog: enhancement
Coverage Report for CI Build 9232Coverage decreased (-6.6%) to 46.971%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements intent-aware UI styling and explanations in the AI Content Planner modals by centralizing intent configuration and using it to drive badge/callout styling and tooltip copy.
Changes:
- Extended the shared
intentBadgemapping with per-intent callout colors and tooltip strings, plus a new “Transactional” intent. - Updated Content Suggestions modal intent badges to show hover tooltips.
- Updated Content Outline modal intent callout to use intent-specific banner colors and show a tooltip for the intent badge.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/js/src/ai-content-planner/components/intent-badge.js | Centralizes tooltip text and callout color classes per intent; adds transactional intent config. |
| packages/js/src/ai-content-planner/components/content-suggestions-modal.js | Adds tooltip behavior to intent badges in the suggestions list (and a placeholder transactional suggestion). |
| packages/js/src/ai-content-planner/components/content-outline-modal.js | Adds intent-colored callout styling and extracts an intent badge + tooltip helper component. |
| packages/js/tests/ai-content-planner/components/content-outline-modal.test.js | Expands intent coverage to include the new transactional intent and adjusts unknown-intent test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [ isTooltipVisible, setIsTooltipVisible ] = useState( false ); | ||
| const handleMouseEnter = useCallback( () => setIsTooltipVisible( true ), [] ); | ||
| const handleMouseLeave = useCallback( () => setIsTooltipVisible( false ), [] ); | ||
| const tooltipId = `suggestion-intent-tooltip-${ intent }-${ title }`; |
There was a problem hiding this comment.
tooltipId is built from title, which contains spaces (and may contain other characters) and produces an invalid/fragile HTML id value. This can break aria-describedby and any selector-based logic. Use a sanitized/slugged value or a stable generated id (e.g. React useId/instance id) instead of interpolating the raw title string.
| <Badge | ||
| className={ classNames( "yst-relative yst-flex yst-items-center yst-gap-1 yst-w-fit yst-text-xs yst-cursor-default", badge.classes ) } | ||
| aria-describedby={ tooltipId } | ||
| onMouseEnter={ handleMouseEnter } | ||
| onMouseLeave={ handleMouseLeave } |
There was a problem hiding this comment.
Same ARIA/a11y issue as in the suggestions modal: aria-describedby is set unconditionally while the tooltip is conditionally mounted, so the trigger often points to a missing element. Also the tooltip is only shown on mouse enter/leave, so it’s not discoverable via keyboard. Consider switching to TooltipContainer + TooltipTrigger + TooltipWithContext from @yoast/ui-library (keeps tooltip in DOM and supports focus/ESC) or make aria-describedby conditional and add focus/blur support.
| it( "renders the correct badge for transactional intent", () => { | ||
| renderModal( { suggestion: { ...defaultSuggestion, intent: "transactional" } } ); | ||
| expect( screen.getByText( "transactional" ) ).toBeInTheDocument(); | ||
| expect( screen.getByText( "Transactional" ) ).toBeInTheDocument(); | ||
| } ); |
There was a problem hiding this comment.
This PR introduces new intent behavior (tooltip text + dynamic callout colors per intent), but the tests here only assert the badge label. Consider adding assertions that the intent tooltip content becomes visible on hover/focus and that the callout container uses the intent-specific classes, so regressions in tooltip wiring or styling are caught.
| <button type="button" onClick={ handleClick } className="yst-text-start yst-w-full yst-rounded-md yst-border yst-border-slate-200 yst-mb-4 yst-p-4 yst-shadow-sm focus:yst-outline focus:yst-outline-2 focus:yst-outline-offset-2 focus:yst-outline-primary-500"> | ||
| { intentBadge[ intent ] ? ( | ||
| <Badge className={ classNames( "yst-flex yst-items-center yst-gap-1 yst-w-fit yst-mb-2 yst-text-xs", intentBadge[ intent ].classes ) }> | ||
| <Icon className={ classNames( "yst-w-3 ", intentBadge[ intent ].classes ) } { ...svgAriaProps } /> { intentBadge[ intent ].label } | ||
| { badge ? ( | ||
| <Badge | ||
| className={ classNames( "yst-relative yst-flex yst-items-center yst-gap-1 yst-w-fit yst-mb-2 yst-text-xs", badge.classes ) } | ||
| aria-describedby={ tooltipId } | ||
| onMouseEnter={ handleMouseEnter } | ||
| onMouseLeave={ handleMouseLeave } |
There was a problem hiding this comment.
aria-describedby is applied to the Badge, but the focusable element here is the surrounding <button>—so keyboard/screen reader users won’t get the tooltip description, and the tooltip itself is only mounted on mouse hover. Additionally, because the tooltip is conditionally rendered, aria-describedby often points to a non-existent id. Prefer the ui-library TooltipContainer + TooltipTrigger + TooltipWithContext (focus + hover, tooltip stays in DOM), or move aria-describedby/show-hide handling to the actual focusable trigger and only reference an id that exists.
|
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. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…d hover with the new IntentBadge/IntentCallout architecture Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JorPV
changed the title
JorPV
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 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 ( | ||
| <Badge | ||
| className={ classNames( "yst-relative yst-flex yst-items-center yst-gap-1 yst-w-fit yst-cursor-default", badge.classes, className ) } | ||
| aria-describedby={ tooltipId } | ||
| onMouseEnter={ handleMouseEnter } | ||
| onMouseLeave={ handleMouseLeave } | ||
| > | ||
| <Icon className={ classNames( "yst-w-3", badge.classes ) } { ...svgAriaProps } /> { badge.label } | ||
| { isTooltipVisible && <Tooltip id={ tooltipId } className="yst-max-w-48 yst-z-50 yst-text-center" position={ tooltipPosition }>{ badge.tooltip }</Tooltip> } | ||
| </Badge> |
There was a problem hiding this comment.
The new tooltip behavior (show/hide + intent-specific tooltip copy) and the intent-driven callout styling are introduced/changed here, but there are no unit tests asserting tooltip rendering on hover (or that the correct tooltip text appears for each intent). Consider extending the existing modal/component tests to simulate hover and assert the tooltip content, to prevent regressions.
| const [ isTooltipVisible, setIsTooltipVisible ] = useState( false ); | ||
| const handleMouseEnter = useCallback( () => setIsTooltipVisible( true ), [] ); | ||
| const handleMouseLeave = useCallback( () => setIsTooltipVisible( false ), [] ); | ||
| const tooltipId = `intent-tooltip-${ intent }`; |
There was a problem hiding this comment.
tooltipId is derived only from intent (e.g. intent-tooltip-informational), which will produce duplicate IDs when multiple badges of the same intent are rendered (e.g. in the suggestions list). This breaks DOM id uniqueness and can confuse ARIA relationships. Generate a per-instance stable unique id (e.g. via useInstanceId from @wordpress/compose or a useRef-backed unique suffix) and incorporate it into the tooltip id.
| className={ classNames( "yst-relative yst-flex yst-items-center yst-gap-1 yst-w-fit yst-cursor-default", badge.classes, className ) } | ||
| aria-describedby={ tooltipId } | ||
| onMouseEnter={ handleMouseEnter } | ||
| onMouseLeave={ handleMouseLeave } | ||
| > | ||
| <Icon className={ classNames( "yst-w-3", badge.classes ) } { ...svgAriaProps } /> { badge.label } | ||
| { isTooltipVisible && <Tooltip id={ tooltipId } className="yst-max-w-48 yst-z-50 yst-text-center" position={ tooltipPosition }>{ badge.tooltip }</Tooltip> } |
There was a problem hiding this comment.
aria-describedby is always set to tooltipId, but the element with that id is only rendered conditionally when the tooltip is visible. When the tooltip is hidden, the referenced element doesn't exist, so assistive tech won't have a valid description target. Consider only setting aria-describedby while the tooltip is rendered, or render a persistently-present (visually hidden / offscreen) description node so the reference is always valid.
…dby, tests - Use React useId to generate per-instance tooltip ids so rendering multiple badges with the same intent no longer produces duplicate DOM ids. - Only set aria-describedby while the tooltip is rendered, so the reference always points to an existing element. - Add intent-badge.test.js covering label rendering, hover show/hide, aria-describedby toggling and unique-id generation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Context
This PR implements dynamic background colors for the "Why this content?" callout banner in the Content Outline modal and adds hover tooltips to intent badges in both the Content Suggestions and Content Outline modals. Previously the callout always used hardcoded blue colors regardless of intent type. Now the colors adapt to match the intent (blue for Informational, violet for Navigational, yellow for Commercial, green for Transactional). The tooltips explaining what each intent type means are also added on hover. Additionally, suggestion cards in the Content Suggestions modal now have a hover state (subtle background tint, darker border, underlined title) matching the design.
Design reference: Figma – Content suggestions
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices
intent-badge.js: Extended the existing mapping withcalloutClasses,calloutTextClasses, andtooltipfields per intent type, keeping all intent-related styling and copy in one place.position="bottom-right"for its tooltip because the callout sits at the top of a scrollable container withoverflow-y: auto— an absolutely-positioned tooltip above the badge would be clipped by the parent's overflow. The Content Suggestions modal usesposition="top-right"since badges there have enough room above them.top-right/bottom-rightanchors the tooltip from the badge's left edge so it extends rightward into available space, preventing it from overflowing the modal's left boundary.IntentBadgeWithTooltipcomponent: Extracted a dedicated component in the outline modal to keep theIntentCalloutcomponent's cyclomatic complexity within the ESLint limit.yst-max-w-48to force the tooltip text to wrap into multiple lines (matching the Figma design), preventing a single wide line from overflowing the modal.yst-groupon the button withgroup-hover:yst-underlineon the title, combined withhover:yst-bg-slate-50andhover:yst-border-slate-300plusyst-transition-colorsfor a smooth effect.intentBadgemapping withShoppingCartIconand green color scheme. A placeholder transactional suggestion was added to the suggestions list for testing.Test instructions for the acceptance test before the PR gets merged
Relevant test scenarios
Test instructions for QA when the code is in the RC
Impact check
intent-badge.js)content-suggestions-modal.js)content-outline-modal.js)Other environments
Documentation
Quality assurance
Innovation
Fixes https://github.com/Yoast/reserved-tasks/issues/1145