Replace content analysis buttons with ui-library Button#23051
Replace content analysis buttons with ui-library Button#23051JorPV wants to merge 9 commits intoyst-root-clean-up-from-pluginfrom
Conversation
260fee1 to
6c684de
Compare
|
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. |
6c684de to
5b7749f
Compare
5b7749f to
ed385e2
Compare
There was a problem hiding this comment.
Pull request overview
This PR continues the UI migration away from legacy @yoast/components by replacing content analysis “highlight” and “edit” buttons with @yoast/ui-library Button + @heroicons/react icons, and updates the analysis-report package dependencies accordingly.
Changes:
- Replace legacy content analysis mark/edit icon buttons with
@yoast/ui-libraryButtonimplementations (including tooltip handling). - Add
@heroicons/react,classnames, and@yoast/ui-librarydependencies to@yoast/analysis-report. - Minor cleanup: update some JSDoc defaults and remove several
yst-rootwrapper elements across UI components.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/js/src/settings/components/formik-user-select-field.js | JSDoc default value for className. |
| packages/js/src/settings/components/formik-page-select-field.js | JSDoc default value for className. |
| packages/js/src/settings/components/formik-media-select-field.js | JSDoc default value for className. |
| packages/js/src/settings/components/formik-indexable-page-select-field.js | JSDoc default value for className. |
| packages/js/src/installation-success.js | Removes yst-root class from the page root wrapper. |
| packages/js/src/components/modals/KeywordUpsell.js | Removes yst-root wrapper divs around upsell badges. |
| packages/js/src/components/modals/InternalLinkingSuggestionsUpsell.js | Removes yst-root wrapper divs around upsell badges. |
| packages/js/src/components/contentBlocks/ContentBlocks.js | Removes yst-root wrapper around “New” badge label. |
| packages/js/src/components/contentBlocks/ContentBlock.js | Removes yst-root wrapper around upsell badge. |
| packages/js/src/components/contentAnalysis/WooSeoAnalysisUpsellAd.js | Removes yst-root wrapper. |
| packages/js/src/components/contentAnalysis/SeoAnalysis.js | Stops passing legacy tooltip className props into Results. |
| packages/js/src/components/contentAnalysis/Results.js | Adds custom mark button factory using ui-library Button + tooltip + upsell badge. |
| packages/js/src/components/contentAnalysis/ReadabilityAnalysis.js | Stops passing legacy tooltip className prop into Results. |
| packages/js/src/components/contentAnalysis/PremiumSeoAnalysisUpsellAd.js | Replaces outer yst-root div with fragment. |
| packages/js/src/components/contentAnalysis/InclusiveLanguageAnalysis.js | Stops passing legacy tooltip className prop into Results. |
| packages/js/src/components/WincherSEOPerformanceModal.js | Removes yst-root wrapper. |
| packages/js/src/components/BlackFridayPromotion.js | Replaces outer yst-root div with fragment. |
| packages/analysis-report/src/AnalysisResult.js | Replaces legacy icon button components with ui-library Button + heroicons and adds tooltips. |
| packages/analysis-report/package.json | Adds deps required for the new ui-library + heroicons button implementations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| aria-label={ ariaLabel } | ||
| onPointerEnter={ () => setIsTooltipOpen( true ) } | ||
| onPointerLeave={ () => setIsTooltipOpen( false ) } | ||
| > | ||
| <EyeIcon className="yst-w-4 yst-h-4" /> | ||
| </Button> | ||
| { isTooltipOpen && ! isPressed && ( | ||
| <Tooltip position="left"> | ||
| { ariaLabel } | ||
| </Tooltip> | ||
| ) } |
There was a problem hiding this comment.
The tooltip here is driven by pointer events only (onPointerEnter/onPointerLeave) and renders the Tooltip element directly. The ui-library provides TooltipContainer/TooltipTrigger/TooltipWithContext to handle focus/blur and Escape-to-close behavior, plus aria-describedby linkage. Consider using those helpers (or add equivalent focus/blur + aria-describedby) so the tooltip is accessible via keyboard navigation as well.
| import { EyeIcon } from "@heroicons/react/outline"; | ||
| import { PencilIcon } from "@heroicons/react/outline"; |
There was a problem hiding this comment.
The PR description mentions using @heroicons/react/solid icons, but EyeIcon/PencilIcon are imported from @heroicons/react/outline here. Please align the code with the stated choice (or update the PR description) to avoid confusion during review/QA.
| export function InstallationSuccessPage() { | ||
| return ( | ||
| <div className="yst-root yst-my-auto yst-flex yst-flex-col yst-min-h-[84vh] yst-py-12 yst-justify-center"> | ||
| <div className="yst-my-auto yst-flex yst-flex-col yst-min-h-[84vh] yst-py-12 yst-justify-center"> |
There was a problem hiding this comment.
InstallationSuccessPage no longer renders a .yst-root wrapper. Our Tailwind base overrides in css/src/tailwind.css are scoped under .yst-root (e.g., WP focus/inputs resets), so this page may now miss those scoped base styles. Consider restoring the yst-root class on the top-level element or wrapping the page content in an element that provides .yst-root to keep styling consistent with other Yoast admin UIs.
| <div className="yst-my-auto yst-flex yst-flex-col yst-min-h-[84vh] yst-py-12 yst-justify-center"> | |
| <div className="yst-root yst-my-auto yst-flex yst-flex-col yst-min-h-[84vh] yst-py-12 yst-justify-center"> |
| onPointerEnter={ () => setIsTooltipOpen( true ) } | ||
| onPointerLeave={ () => setIsTooltipOpen( false ) } | ||
| > | ||
| <EyeIcon className="yst-w-4 yst-h-4" /> | ||
| </Button> | ||
| { isTooltipOpen && ! isPressed && ( | ||
| <Tooltip position="left"> | ||
| { ariaLabel } | ||
| </Tooltip> | ||
| ) } |
There was a problem hiding this comment.
The new tooltip behavior is implemented via onPointerEnter/onPointerLeave and conditional rendering of <Tooltip>. This bypasses the ui-library’s TooltipContainer/TooltipTrigger/TooltipWithContext helpers (which handle focus/blur, Escape-to-close, and aria-describedby). Consider switching to the tooltip-container components, or at least add onFocus/onBlur + proper aria-describedby wiring so keyboard users get the same tooltip affordance as pointer users.
| import { Badge, Button, Root, Tooltip } from "@yoast/ui-library"; | ||
| import { EyeIcon } from "@heroicons/react/outline"; | ||
| import { LockClosedIcon } from "@heroicons/react/solid"; |
There was a problem hiding this comment.
The PR description states the new buttons use @heroicons/react/solid, but this implementation imports EyeIcon from @heroicons/react/outline. Either update the implementation to match the intended icon set, or update the PR description/acceptance expectations so they reflect the actual icon variant used.
| const MarkButton = ( { | ||
| ariaLabel, | ||
| id, | ||
| className, | ||
| status, | ||
| onClick, | ||
| isPressed, | ||
| } ) => { | ||
| return <IconButtonToggle | ||
| marksButtonStatus={ status } | ||
| className={ className } | ||
| onClick={ onClick } | ||
| id={ id } | ||
| icon="eye" | ||
| pressed={ isPressed } | ||
| ariaLabel={ ariaLabel } | ||
| />; | ||
| const [ isTooltipOpen, setIsTooltipOpen ] = useState( false ); | ||
|
|
||
| return <Root> | ||
| <div className="yst-relative yst-inline-flex"> | ||
| <Button | ||
| variant={ isPressed ? "primary" : "secondary" } | ||
| size="small" | ||
| className={ classNames( className, "yst-px-2 yst-rounded-lg yst-shadow-none yst-border-0 focus:yst-z-10" ) } | ||
| onClick={ onClick } | ||
| id={ id } | ||
| disabled={ status === "disabled" } | ||
| aria-pressed={ isPressed } | ||
| aria-label={ ariaLabel } | ||
| onPointerEnter={ () => setIsTooltipOpen( true ) } | ||
| onPointerLeave={ () => setIsTooltipOpen( false ) } | ||
| > | ||
| <EyeIcon className="yst-w-4 yst-h-4" /> | ||
| </Button> | ||
| { isTooltipOpen && ! isPressed && ( | ||
| <Tooltip position="left"> | ||
| { ariaLabel } | ||
| </Tooltip> | ||
| ) } | ||
| </div> | ||
| </Root>; | ||
| }; |
There was a problem hiding this comment.
AnalysisResult has Jest snapshot coverage (and ContentAnalysis snapshots include these buttons). Since the mark/edit button markup changed substantially (IconButtonToggle/IconCTAEditButton -> ui-library Button + heroicons), the existing snapshots in packages/analysis-report/tests/__snapshots__ will no longer match. Please update the affected snapshots (run the analysis-report Jest suite with snapshot update) so CI reflects the new intended output.
dd350ae to
844b8a5
Compare
|
/build-zip |
1 similar comment
|
/build-zip |
|
📦 Plugin zip built successfully! Download it from the workflow run. |
|
/build-zip |
a704e3e to
ad07334
Compare
|
📦 Plugin zip built successfully! Download it from the workflow run. |
07c5dd5 to
7534f81
Compare
6014ffa to
4238822
Compare
|
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. |
7534f81 to
f92816d
Compare
4238822 to
6972f35
Compare
ad95a65 to
e92c533
Compare
Replace IconButtonToggle (highlight) and IconCTAEditButton (edit) from @yoast/components with Button from @yoast/ui-library, using EyeIcon and PencilIcon from @heroicons/react/solid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nalysis buttons Move EyeIcon and PencilIcon from Button children to the new icon prop, with sizing handled by CSS class (.yst-button--icon) instead of inline classes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…onent class The @layer components block strips plain CSS and arbitrary value classes are not generated by the plugin's Tailwind build. Use standard yst-w-5/yst-h-5 (20px) and yst-w-4/yst-h-4 (16px) utility classes directly in the JSX. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Merge duplicate @heroicons/react/outline imports - Use parameter defaults instead of defaultProps for function components - Replace arrow functions in JSX props with useCallback handlers - Update analysis-report test snapshots for new button markup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rning - Remove className="yst-root" from analysis-report and search-metadata-previews snapshots to match the base branch Root component that no longer outputs yst-root class - Add eslint-disable complexity for MarkButtonWithUpsell component to stay within @yoast/wordpress-seo max-warnings limit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dius Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e92c533 to
e518d46
Compare
Context
As part of the effort to move away from legacy
@yoast/components, this PR replaces the Highlight (mark) button and Edit button in content analysis results with theButtoncomponent from@yoast/ui-library, using@heroicons/reacticons (EyeIcon,PencilIcon). It also replaces the legacy CSS tooltip classes with theTooltipContainer/TooltipWithContextcomponents from@yoast/ui-library, providing keyboard-accessible tooltips.Summary
This PR can be summarized in the following changelog entry:
IconButtonToggleandIconCTAEditButtoncomponents with@yoast/ui-libraryButton,TooltipContainer, and@heroicons/reacticons in content analysis results.Relevant technical choices:
Button replacement
IconButtonToggle(highlight button) withButtonvariant="secondary"/"primary" size="small" +EyeIconfrom@heroicons/react/outlineIconCTAEditButton(edit button) withButtonvariant="secondary" size="small" +PencilIconfrom@heroicons/react/outlineyst-px-2 yst-rounded-md yst-shadow-none yst-border-0to match the AI optimize button appearancevariant="secondary"(unpressed) andvariant="primary"(pressed)Tooltip replacement
TooltipContainer/useTooltipContext()/TooltipWithContextfrom@yoast/ui-libraryMarkButtonInnerandEditButtonInnercomponents consumeuseTooltipContext()forshow/hideonPointerEnter/onPointerLeave) and keyboard (onFocus/onBlur)aria-describedbywiring links buttons to their tooltipsTooltipContainerprovides Escape key dismissal automatically<Root>from@yoast/ui-libraryto ensure the Tailwind preflight CSS (.yst-root ::before { border-width: 0 }) is properly scoped, which is required for the tooltip arrow to render correctlyComponent restructuring
MarkButtonInner+MarkButtonwrapper pattern inAnalysisResult.jsEditButtonInnercomponent inAnalysisResult.jsMarkButtonInner+MarkButtonWithUpsellwrapper pattern inResults.jsTooltipWithContextin tooltip-container for test compatibilitymarksButtonClassName=""andeditButtonClassName=""props fromSeoAnalysis.js,ReadabilityAnalysis.js, andInclusiveLanguageAnalysis.js(the defaults inResults.jsare already"")Dependencies
@heroicons/react,@yoast/ui-library, andclassnamesas dependencies to@yoast/analysis-reportpackage.jsonTest instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Relevant test scenarios
Test instructions for QA when the code is in the RC
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
Documentation
Quality assurance
grunt build:imagesand commited the results, if my PR introduces new images or SVGs.Innovation
innovationlabel.