Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/block-components/icon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ export const Icon = props => {
return
}

// Don't use page icons for multicolor icons
// because we target svg elements with the :nth-of-type() selector to apply the multicolor styles.
if ( getAttribute( 'iconColorType' ) === 'multicolor' ) {
return
}
Comment on lines 207 to 218
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add cleanup when iconColorType changes to/from multicolor.

The early return prevents page-icon processing for multicolor icons. However, if iconColorType changes from a regular type to multicolor during the icon's lifecycle, the icon will remain in the page-icons store without being removed. Similarly, changing from multicolor to a regular type won't trigger page-icon processing.

Consider adding getAttribute( 'iconColorType' ) to the useEffect dependency array and handling the transition:

-	}, [ _icon ] )
+	}, [ _icon, getAttribute( 'iconColorType' ) ] )

Additionally, add cleanup logic before the early return:

 		// Don't use page icons for multicolor icons
 		// because we target svg elements with the :nth-of-type() selector to apply the multicolor styles.
 		if ( getAttribute( 'iconColorType' ) === 'multicolor' ) {
+			// Clean up if this icon was previously in the page-icons store
+			if ( processedIconRef.current === _icon && _icon ) {
+				dispatch( 'stackable/page-icons' ).removePageIcon( _icon )
+				processedIconRef.current = null
+				setIcon( _icon ) // Use the original icon directly
+				lastIconValueRef.current = _icon
+			}
 			return
 		}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/block-components/icon/index.js around lines 205 to 209, the effect
currently returns early for iconColorType === 'multicolor' but doesn't clean up
or respond to transitions; add getAttribute('iconColorType') to the useEffect
dependency array and inside the effect detect transitions: when switching to
'multicolor' call the existing cleanup/remove-from-page-icons logic before
returning, and when switching from 'multicolor' to a regular type ensure you
call the page-icon processing/registration routine; ensure the cleanup function
is invoked on unmount or before early returns so page-icons store stays
consistent.


// Check if icon exists in pageIcons Map
// The Map structure is: [SVG string (key), { id: iconId, count: number } (value)]
if ( _icon ) {
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/page-icons/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ pageIconsWrapper?.setAttribute( 'id', 'stk-page-icons' )
domReady( () => {
if ( pageIconsWrapper ) {
pageIconsWrapper.setAttribute( 'id', 'stk-page-icons' )
pageIconsWrapper.setAttribute( 'style', 'display: none;' )
// Don't use `display: none` to hide the page icons because it prevents gradients from being applied.
pageIconsWrapper.setAttribute( 'style', 'position: absolute; top: 0; left: -9999px; width: 0; height: 0; visibility: hidden;' )
createRoot( pageIconsWrapper ).render( <PageIcons /> )
}
} )
Expand Down
Loading