feat: getting started onboarding + style guide #3571
Conversation
🤖 Pull request artifacts
|
- accordion - icon list, icon list item - countdown
- table of contents - posts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/plugins/theme-block-style-inheritance/index.php (3)
36-41: register_setting type/sanitization/default mismatch.Type is boolean but sanitize_text_field and default '' are inconsistent. Use a boolean sanitizer and default.
- array( - 'type' => 'boolean', - 'description' => __( 'Inherit block style from theme.json', STACKABLE_I18N ), - 'sanitize_callback' => 'sanitize_text_field', - 'show_in_rest' => true, - 'default' => '', - ) + array( + 'type' => 'boolean', + 'description' => __( 'Inherit block style from theme.json', STACKABLE_I18N ), + 'sanitize_callback' => 'rest_sanitize_boolean', + 'show_in_rest' => true, + 'default' => false, + )
51-54: Store boolean true, not the string 'true'.This prevents type confusion when read later.
- update_option( 'stackable_disable_block_style_inheritance', 'true' ); + update_option( 'stackable_disable_block_style_inheritance', true );
82-84: Return the incoming CSS when bailing out.Returning null can break filters expecting a string.
- if ( ! isset( $raw_data[ 'styles' ] ) ) { - return; - } + if ( ! isset( $raw_data[ 'styles' ] ) ) { + return $current_css; + }src/util/block-styles.js (1)
28-33: Bug: attributes are only removed if truthy.Falsy values (''/false/0) won’t be deleted, skewing “modified” detection.
- attrsToRemove.forEach( attr => { - if ( blockAttrs[ attr ] ) { - delete blockAttrs[ attr ] - } - } ) + attrsToRemove.forEach( attr => { + if ( attr in blockAttrs ) { + delete blockAttrs[ attr ] + } + } )src/plugins/global-settings/utils/use-block-layout-editor-loader.js (3)
86-90: Fix invalid CSS nesting for parent-hover rules (scoped rules are being wrapped inside :root).getValue returns a full selector block (e.g., ".stk--is-hovered.stk-XYZ { … }") that later gets concatenated inside ":root { … }", producing invalid CSS like ":root { .stk--is-hovered… { … } }".
Refactor to keep selector-scoped rules out of :root and media-wrapped :root blocks.
Apply this diff:
@@ - const deviceCss = { + const deviceCss = { desktop: [], tablet: [], mobile: [], } + + // Rules that are selector-scoped (e.g., parent-hover), not custom-property declarations on :root. + const scopedCss = { + desktop: [], + tablet: [], + mobile: [], + } @@ - if ( currentHoverState === 'parent-hover' && state === 'parent-hover' && blockUniqueId && parentHoverBlock ) { - style = `.stk--is-hovered.stk-${ blockUniqueId }{ ${ style } }` - } + // Do not wrap in a selector here; push to scopedCss at the call site. @@ - const unit = getUnit( property, state ) + const unit = getUnit( property, device ) @@ - deviceCss[ device ].push( getValue( property, device, hoverState, value, unit ) ) + const decl = getValue( property, device, hoverState, value, unit ) + if ( hoverState === 'parent-hover' && currentHoverState === 'parent-hover' && blockUniqueId && parentHoverBlock ) { + scopedCss[ device ].push( `.stk--is-hovered.stk-${ blockUniqueId }{ ${ decl } }` ) + } else { + deviceCss[ device ].push( decl ) + } @@ - if ( property === 'button-icon-size' || property === 'icon-size' ) { - deviceCss[ device ].push( getValue( `custom-${ property }`, device, hoverState, value, unit ) ) - } + if ( property === 'button-icon-size' || property === 'icon-size' ) { + const customDecl = getValue( `custom-${ property }`, device, hoverState, value, unit ) + if ( hoverState === 'parent-hover' && currentHoverState === 'parent-hover' && blockUniqueId && parentHoverBlock ) { + scopedCss[ device ].push( `.stk--is-hovered.stk-${ blockUniqueId }{ ${ customDecl } }` ) + } else { + deviceCss[ device ].push( customDecl ) + } + } @@ - if ( deviceCss.desktop.length > 0 ) { - css += `:root { ${ compact( deviceCss.desktop ).join( '' ) }}` - } + if ( deviceCss.desktop.length > 0 ) { + css += `:root { ${ compact( deviceCss.desktop ).join( '' ) }}` + } + if ( scopedCss.desktop.length > 0 ) { + css += compact( scopedCss.desktop ).join( '' ) + } @@ - if ( deviceCss.tablet.length > 0 ) { - css += `@media screen and (max-width: ${ breakDesktop - 1 }px){ :root { ${ compact( deviceCss.tablet ).join( '' ) }}}` - } + if ( deviceCss.tablet.length > 0 || scopedCss.tablet.length > 0 ) { + css += `@media screen and (max-width: ${ breakDesktop - 1 }px){` + + `${ deviceCss.tablet.length ? `:root { ${ compact( deviceCss.tablet ).join( '' ) }}` : '' }` + + `${ scopedCss.tablet.length ? compact( scopedCss.tablet ).join( '' ) : '' }` + + `}` + } @@ - if ( deviceCss.mobile.length > 0 ) { - css += `@media screen and (max-width: ${ breakTablet - 1 }px){:root { ${ compact( deviceCss.mobile ).join( '' ) }}}` - } + if ( deviceCss.mobile.length > 0 || scopedCss.mobile.length > 0 ) { + css += `@media screen and (max-width: ${ breakTablet - 1 }px){` + + `${ deviceCss.mobile.length ? `:root { ${ compact( deviceCss.mobile ).join( '' ) }}` : '' }` + + `${ scopedCss.mobile.length ? compact( scopedCss.mobile ).join( '' ) : '' }` + + `}` + }Also applies to: 121-131
46-49: Units for hover states default to "px" due to wrong argument; pass device not state.getUnit expects a device key (desktop/tablet/mobile), but it receives the raw state (e.g., "desktopHover"), so hover units fall back to "px".
Apply this diff:
- const getUnit = ( property, state ) => { - return blockLayouts[ property ][ `${ state }Unit` ] ?? 'px' + const getUnit = ( property, device ) => { + return blockLayouts[ property ][ `${ device }Unit` ] ?? 'px' } @@ - const unit = getUnit( property, state ) + const unit = getUnit( property, device )Also applies to: 104-107
34-37: Honor returnCss on empty input.When blockLayouts is empty and returnCss is true, the function should return an empty string and avoid calling setStyles.
Apply this diff:
if ( Object.keys( blockLayouts ).length === 0 ) { - setStyles( '' ) - return + if ( returnCss ) { + return '' + } + setStyles( '' ) + return } @@ - if ( returnCss ) { - return css - } + if ( returnCss ) { + return css + }Also applies to: 133-136
🧹 Nitpick comments (51)
src/plugins/global-settings/editor.scss (1)
35-38: Button centering may not take effect without flex; avoid!importantif possible
justify-contentonly applies to flex/grid containers. If this class is applied to a non-flex element, the rule won’t work. Also consider dropping!importantby tightening selector specificity.Apply:
.ugb-global-settings__preview-button { width: 100%; - justify-content: center !important; + display: flex; + justify-content: center; + align-items: center; }src/components/style-guide/images/readme.txt (1)
1-2: Good on adding attributions; include license + file mapping for auditsAdd license (e.g., “Pexels License”), local filenames, and access date to simplify asset compliance checks.
Example:
hero-bg.webp — Pexels (Daniel Frank), https://..., accessed 2025‑09‑10, Pexels License media-text.webp — Pexels (cottonbro studio), https://..., accessed 2025‑09‑10, Pexels License.config/rules.js (1)
42-42: Broaden image regex (jpeg/avif) and make it case‑insensitiveFuture-proof the loader by supporting
.jpegand.avif, and add/i.- test: /\.(png|jpg|gif|webp)$/, + test: /\.(png|jpe?g|gif|webp|avif)$/i,Optional: consider migrating from file-loader to webpack 5 Asset Modules when convenient.
src/init.php (1)
343-344: LGTM: exposinghomeUrlto JS is usefulNo security concerns;
wp_localize_scriptJSON-encodes values. Optionally, align withsrcUrlby usinguntrailingslashit( home_url() )if callers concatenate paths.src/plugins/global-settings/block-styles/index.js (1)
22-47: Filter contract changed—preserve back-compat for third-partiesPreviously,
stackable.global-settings.inspector.global-block-styles.controlreceivedisOpen. Removing it may break existing integrations. Pass a second boolean arg (fallback) to keep arity, and optionally add a third arg for future context.- { isPro && applyFilters( 'stackable.global-settings.inspector.global-block-styles.control', Fragment ) } + { isPro && applyFilters( + 'stackable.global-settings.inspector.global-block-styles.control', + Fragment, + false, // back-compat for previous `isOpen` + {} // future context (e.g., panel props) if needed + ) }Document deprecation of
isOpenin the filter’s JSDoc and point consumers to the new context arg if/when provided.src/plugins/guided-modal-tour-trigger/index.js (1)
13-13: Minor: fix typo in comment.- // Check th + // Check the query param and trigger the toursrc/components/design-library-list/util.js (1)
240-253: Add comment clarifying placeholder cycling
Above theArray.fromcall, annotate that whennumItemsexceedsremainingPosts.length, placeholders are cycled via modulus:} else { + // Cycle through `postsPlaceholder` when more items are needed than available posts const needed = numItems const postsToUse = [ ...remainingPosts, ...Array.from( { length: needed - remainingPosts.length }, ( _, i ) => postsPlaceholder[ i % postsPlaceholder.length ] // reuse placeholders if numberOfItems > placeholders.length ), ] currentPosts = postsToUse.slice( 0, numItems ) }src/components/guided-modal-tour/tour-steps.js (4)
124-129: Tighten grammar in user-facing copy.Minor clarity tweak.
Apply this diff:
- description: __( 'Optionally, you can turn on backgrounds, change color schemes, to customize the library in real-time.', i18n ), + description: __( 'Optionally, turn on backgrounds or change color schemes to customize the library in real time.', i18n ),
182-185: Avoid empty translatable strings.Empty
description: ''creates a useless msgid for translators and inconsistent UI. Omit the key or add a meaningful placeholder.Apply this diff to omit:
- description: '', // Not yet available.
194-196: Avoid empty translatable strings.Same concern as above for the design-system picker step.
Apply this diff to omit:
- description: '', // Not yet available.
52-55: Condition helpers: consider URLSearchParams for robustness.Using
.includes('tour=…')may produce false positives (e.g.,notour=). URLSearchParams is safer.Apply this diff pattern where applicable:
- return window?.location?.search?.includes( 'tour=design-system' ) ? true : null + return new URLSearchParams( window?.location?.search || '' ).get( 'tour' ) === 'design-system' ? true : nullsrc/components/style-guide/editor.scss (3)
42-51: Duplicate margin-bottom; later value wins.Two margin-bottom declarations; keep one (32px).
Apply this diff:
.ugb-style-guide__section-subheading { font-family: $guide-font-family; font-size: 25.63px; font-weight: 400; - margin-bottom: 1em; color: #999; border-bottom: 1px solid #eee; margin-bottom: 32px;
378-382: Redundant corner radius overrides.Setting top-left/right to 16px may fight the variable radius. Let the variable drive all corners.
Apply this diff:
.ugb-style-guide__preview { border-radius: var(--stk-block-background-border-radius, 16px); - border-top-left-radius: 16px; - border-top-right-radius: 16px; overflow: hidden;
81-85: Transition breadth may affect perf.
transition: all …on large containers can be costly. Limit to the properties you actually animate.Happy to propose a narrowed transition list if you confirm which properties animate here.
src/components/guided-modal-tour/editor.scss (2)
21-24: Remove unused theme color vars or use them.These vars are set but unused; drop them or apply consistently.
Apply this diff (if unused):
- --wp-admin-theme-color: #f00069; - --wp-admin-theme-color-darker-10: #e0003c; - --wp-admin-theme-color-darker-20: #cb0044;
169-176: Respect prefers-reduced-motion for glow animation.Offer a reduced-motion path to improve accessibility.
Add this at file end:
@media (prefers-reduced-motion: reduce) { .ugb-tour-modal__glow, .ugb-tour-modal__glow--medium, .ugb-tour-modal__glow--large { animation: none !important; } }src/welcome/getting-started.scss (3)
30-37: Use a CSS variable for brand color for consistency.Align with
--stk-welcome-primaryused elsewhere.Apply this diff:
.tag { text-align: center; - color: #f00069; - border: 1px solid #f0006933; + color: var(--stk-welcome-primary, #f00069); + border: 1px solid color-mix(in srgb, var(--stk-welcome-primary, #f00069) 20%, transparent); padding: 4px 16px; border-radius: 30px; - background-color: #f0006933; + background-color: color-mix(in srgb, var(--stk-welcome-primary, #f00069) 20%, transparent); }
5-11: Add reduced-motion fallback to s-shadow.Avoid hover elevation animation for users who prefer reduced motion.
Append inside the mixin’s scope:
@media (prefers-reduced-motion: reduce) { &, &:hover { transition: none !important; box-shadow: rgba(0, 0, 0, 0.08) 0 0 0 1px; } }
276-290: Avoid nth-child for semantic icon colors.nth-child ties visuals to DOM order; prefer modifier classes (e.g., .s-quick-button--import, --tour, --docs).
I can refactor selectors if you confirm the intended card identities.
package.json (1)
17-29: Sanity-check bundle impact.Ensure confetti and html-to-image are lazy-loaded only where needed to keep editor bundle lean.
If not already, switch to dynamic imports where these are used.
src/util/styles/index.js (1)
299-304: Harden getPropertyLabel for non-camelCase, acronyms, and unsafe inputs.Covers snake/kebab case, consecutive capitals (e.g., CSSVariable), trims, and guards null/undefined.
Apply:
-export const getPropertyLabel = property => { - const result = property.replace( /([a-z])([A-Z])/g, '$1 $2' ) - .replace( /^([a-z])/, match => match.toUpperCase() ) - return result -} +export const getPropertyLabel = ( property = '' ) => { + const str = String( property ) + .replace( /[_\-\s]+/g, ' ' ) // snake/kebab -> space + .replace( /([A-Z]+)([A-Z][a-z])/g, '$1 $2' ) // split acronyms + .replace( /([a-z])([A-Z])/g, '$1 $2' ) // camelCase -> space + .replace( /\s+/g, ' ' ) + .trim() + return str ? str[0].toUpperCase() + str.slice( 1 ) : '' +}src/plugins/design-library-button/design-library-button.js (1)
15-15: Gate/lazy-mount the editor tour to avoid unnecessary work.GuidedModalTour self-gates, but mounting it on every editor load still computes steps/conditions. Consider a cheap guard (e.g., URL flag, a global “tours enabled” setting, or lazy import) before rendering to save cycles on most loads.
Also applies to: 45-53
src/plugins/global-settings/typography/index.js (1)
154-157: Clear throttled timers on unmount to avoid leaks.Add a cleanup effect to clear saveTypographyThrottle, saveSelectedFontPairThrottle, and saveCustomFontPairsThrottle.
Apply:
let saveTypographyThrottle = null let saveSelectedFontPairThrottle = null let saveCustomFontPairsThrottle = null +// Cleanup throttles on unmount +useEffect( () => () => { + clearTimeout( saveTypographyThrottle ) + clearTimeout( saveSelectedFontPairThrottle ) + clearTimeout( saveCustomFontPairsThrottle ) +}, [] )src/plugins/global-settings/block-styles/editor-loader.js (1)
4-24: Simplify and harden CSS assembly (avoid for..in, handle missing fields).Use map/join, default to empty strings, and keep return flags.
Apply:
-export const renderGlobalBlockStyleStyles = ( setStyles, allBlockStyles, returnSaveCss = false, returnCss = false ) => { - let styles = '' - - for ( const blockName in allBlockStyles ) { - const blockStyles = allBlockStyles[ blockName ] - blockStyles.forEach( blockStyle => { - if ( returnSaveCss ) { - styles += blockStyle.saveCss - return - } - - styles += blockStyle.editCss - } ) - } - - if ( returnCss ) { - return styles - } - - setStyles( styles ) -} +export const renderGlobalBlockStyleStyles = ( setStyles, allBlockStyles, returnSaveCss = false, returnCss = false ) => { + const getCss = bs => ( returnSaveCss ? bs?.saveCss : bs?.editCss ) || '' + const styles = Object.values( allBlockStyles || {} ) + .map( arr => ( arr || [] ).map( getCss ).join( '' ) ) + .join( '' ) + if ( returnCss ) return styles + setStyles( styles ) +}src/util/styles/block-style-generators.js (1)
18-18: Guard webpack-specific require.context for tests and non-webpack builds.
require.contextis undefined in Jest/Node/Vite. Prevent import-time crashes.-export const blockStyleGenerators = fetchBlockStyleGenerators( require.context( '../../block', true, /style\.js$/ ) ) +export const blockStyleGenerators = + typeof require !== 'undefined' && typeof require.context === 'function' + ? fetchBlockStyleGenerators( require.context( '../../block', true, /style\.js$/ ) ) + : {}src/util/typography/description.js (1)
1-4: Consider importing lodash functions via path imports to keep bundle lean.Switch to per-module imports to avoid pulling more of lodash than needed.
-import { - upperFirst, startCase, last, -} from 'lodash' +import upperFirst from 'lodash/upperFirst' +import startCase from 'lodash/startCase' +import last from 'lodash/last'src/plugins/global-settings/typography/typography-picker.js (2)
9-9: Prefer per-method lodash import for smaller bundles.Swap to
lodash/omit.-import { omit } from 'lodash' +import omit from 'lodash/omit'
46-46: Class toggle may miss device-only sizes.
createTypographyDescription( value )uses desktop by default; if only tablet/mobile size is set, the class won’t be added though a description exists in preview.Two options:
- Compute
devicehere (sameuseSelectas inTypographyPreview) and callcreateTypographyDescription( value, device ).- Or be conservative: toggle when any of
fontFamily/fontWeight/textTransform/fontSize/tabletFontSize/mobileFontSizeis present.src/components/color-scheme-preview/index.js (3)
8-16: Label consistency: use “Button Background Color”.Align the label with the key name and other labels.
export const COLOR_SCHEME_PROPERTY_LABELS = { backgroundColor: __( 'Background Color', i18n ), headingColor: __( 'Heading Color', i18n ), textColor: __( 'Text Color', i18n ), linkColor: __( 'Link Color', i18n ), accentColor: __( 'Accent Color', i18n ), - buttonBackgroundColor: __( 'Button Color', i18n ), + buttonBackgroundColor: __( 'Button Background Color', i18n ), buttonTextColor: __( 'Button Text Color', i18n ), buttonOutlineColor: __( 'Button Outline Color', i18n ), }
8-17: Graceful fallback for unknown properties.If a property isn’t in COLOR_SCHEME_PROPERTY_LABELS, UI shows an empty label. Consider exporting a helper that falls back to the raw key or start-cased key.
Example:
+export const getColorSchemePropertyLabel = key => + COLOR_SCHEME_PROPERTY_LABELS[ key ] ?? keyThen use getColorSchemePropertyLabel where labels are rendered.
39-45: A11y: propagate disabled to Button.Button isn’t disabled when isDisabled=true. Add disabled so it’s non-focusable and announced properly.
-const additionalProps = onClick === NOOP ? {} : { onClick } +const additionalProps = onClick === NOOP ? {} : { onClick, disabled: isDisabled }src/components/style-guide/index.js (1)
21-23: Track the TODO.Open a follow-up issue to list remaining tasks and owners so this doesn’t linger.
I can draft the issue with a checklist if you want.
src/plugins/global-settings/index.js (2)
50-55: Memo without props won’t help much; memoize panels instead.React.memo on a prop-less component won’t prevent parent-triggered renders. Memoize the expensive filter result.
-const GlobalSettingsInspector = memo( () => { - const panels = applyFilters( 'stackable.global-settings.inspector', null ) - return <> { panels } </> -} ) +const GlobalSettingsInspector = memo( () => { + const panels = useMemo( + () => applyFilters( 'stackable.global-settings.inspector', null ), + [] + ) + return <>{ panels }</> +} )
90-97: WP components v3: prefer variant over isSecondary.isSecondary may be deprecated; use variant="secondary" and expose toggled state for a11y.
-<Button - isSecondary +<Button + variant="secondary" className="ugb-global-settings__preview-button" onClick={ () => setIsStyleGuideOpen( isOpen => ! isOpen ) } icon={ <Icon icon={ layoutIcon } /> } + aria-pressed={ isStyleGuideOpen } >src/components/style-guide/popover.js (2)
29-34: Make anchoring responsive; recompute width/height on resize.useMemo runs once; sizes won’t update on window/editor resize or late-mount anchor.
- // On load, look for the .interface-interface-skeleton__content and position over it. - const [ editorStylesWrapper, width, height ] = useMemo( () => { - const el = document.querySelector( '.interface-interface-skeleton__content' ) - return [ el, el?.offsetWidth, el?.offsetHeight ] - }, [] ) + // Anchor to the editor content and track its size. + const [ editorStylesWrapper, setEditorStylesWrapper ] = useState( null ) + const [ size, setSize ] = useState( { width: 0, height: 0 } ) + useEffect( () => { + const el = document.querySelector( '.interface-interface-skeleton__content' ) + if ( ! el ) return + setEditorStylesWrapper( el ) + const ro = new ResizeObserver( ( entries ) => { + for ( const entry of entries ) { + const { width, height } = entry.contentRect + setSize( { width, height } ) + } + } ) + ro.observe( el ) + // Initialize immediately. + setSize( { width: el.offsetWidth, height: el.offsetHeight } ) + return () => ro.disconnect() + }, [] )- const style = width && height ? { '--width': `${ width }px`, '--height': `${ height }px` } : {} + const style = size.width && size.height ? { '--width': `${ size.width }px`, '--height': `${ size.height }px` } : {}-import { - useMemo, useState, useRef, -} from '@wordpress/element' +import { + useMemo, useState, useRef, useEffect, +} from '@wordpress/element'Also applies to: 37-37, 16-18
81-84: Prefer non-blocking notices over alert().Consider wp.data notices or a Snackbar to avoid blocking UX.
src/hooks/use-design-system.js (1)
57-59: Ensure createTypographyDescription handles non-strings.fontWeight can be numeric; upstream implementation calls value.includes which would throw on numbers. Please confirm and harden upstream utility (coerce to string or type-check).
I can patch createTypographyDescription to guard non-strings if you’d like.
src/welcome/getting-started.php (1)
15-17: Register settings once (init) instead of twice.Hooking both admin_init and rest_api_init is redundant; registering on init suffices.
src/plugins/global-settings/utils/use-block-layout-editor-loader.js (2)
66-84: Treat empty-string side values as “unset” to avoid emitting invalid "px" tokens.When a side is '', appendUnitIfNeeded yields "px". Use defaults when side is '' or undefined.
Apply this diff:
- const top = value.top !== undefined ? value.top : defaultValue.top - const right = value.right !== undefined ? value.right : defaultValue.right - const bottom = value.bottom !== undefined ? value.bottom : defaultValue.bottom - const left = value.left !== undefined ? value.left : defaultValue.left + const pick = (v, d) => (v === '' || v === undefined ? d : v) + const top = pick( value.top, defaultValue.top ) + const right = pick( value.right, defaultValue.right ) + const bottom = pick( value.bottom, defaultValue.bottom ) + const left = pick( value.left, defaultValue.left )
170-200: Avoid re-adding the same filter; add once and remove on cleanup.addFilter with the same namespace may be invoked multiple times across effects and MutationObserver callbacks. Prefer adding once and removing in cleanup to avoid duplicate handlers.
Apply this diff pattern:
- addFilter( 'stackable.global-styles.classnames', `stackable/global-settings.${ classSuffix }`, classnames => { + const ns = `stackable/global-settings.${ classSuffix }` + addFilter( 'stackable.global-styles.classnames', ns, classnames => { classnames.push( className ) return classnames } ) ... - return () => mo.disconnect() + return () => { + mo.disconnect() + removeFilter?.( 'stackable.global-styles.classnames', ns ) + }Note: removeFilter is available from '@wordpress/hooks' in recent WP versions; import it if present.
src/hooks/use-block-color-schemes.js (1)
100-142: getSortedColorSchemes works; consider memoizing to avoid recalculation.Since it derives from stable selector state, wrap with useMemo (or return a memoized function) to avoid recomputing on each consumer render.
Example:
- const getSortedColorSchemes = () => { + const getSortedColorSchemes = useMemo( () => () => { // existing body - } + }, [ _baseColorScheme, _backgroundModeColorScheme, _containerModeColorScheme, colorSchemesCollection ] )src/components/style-guide/components.js (4)
15-16: Fallback to generated labels when a property label is missing.If a property isn’t in COLOR_SCHEME_PROPERTY_LABELS, UI shows "undefined". Use getPropertyLabel as a fallback.
Apply this diff:
-import { isDarkColor } from '~stackable/util' +import { isDarkColor, getPropertyLabel } from '~stackable/util' @@ - const label = COLOR_SCHEME_PROPERTY_LABELS[ property ] + const label = COLOR_SCHEME_PROPERTY_LABELS[ property ] || getPropertyLabel( property ) @@ - const label = COLOR_SCHEME_PROPERTY_LABELS[ property ] + const label = COLOR_SCHEME_PROPERTY_LABELS[ property ] || getPropertyLabel( property ) @@ - const label = COLOR_SCHEME_PROPERTY_LABELS[ property ] + const label = COLOR_SCHEME_PROPERTY_LABELS[ property ] || getPropertyLabel( property )Also applies to: 77-83, 86-92, 95-102
259-263: Guard against getBlockType returning null.getBlockType can return undefined for unknown block names. Avoid potential runtime errors.
Apply this diff:
- const blockTitle = getBlockType( blockName ).title + const blockType = getBlockType( blockName ) + const blockTitle = blockType?.title || blockName
36-39: Avoid "undefined" in className strings.When className is falsy, template literals produce "undefined". Default to empty string.
Apply this pattern where applicable:
- className={ `ugb-style-guide__color-scheme ugb-style-guide__color-container ugb-style-guide__preview-root ${ className }` } + className={ `ugb-style-guide__color-scheme ugb-style-guide__color-container ugb-style-guide__preview-root ${ className || '' }` }Also applies to: 70-73, 215-216, 233-236, 263-280, 302-303
115-122: Use a stable key for Colors list items.If available, prefer a unique id/slug over the array index to avoid reconciliation glitches when the palette changes.
Example:
- { colors.map( ( color, key ) => { - return <div key={ key } ... + { colors.map( color => { + return <div key={ color.slug || color.name } ...src/components/style-guide/utils.js (3)
305-306: Harden against undefined variations.
getBlockVariationsmay returnundefined. Default to[]to avoid.lengthaccess errors.- const variations = getBlockVariations( blockName ) + const variations = getBlockVariations( blockName ) || []
60-79: Make serialization patches resilient to markup changes.The countdown/tab replacements rely on exact HTML strings and may break with minor markup tweaks. Prefer regex-based, structure-tolerant replacements.
176-184: Avoid hard-coded, collision-prone class suffixes.
stk-5609083/stk-3f9ae3care static. If multiple previews render, these can collide. Consider deriving a per-render unique suffix (e.g., fromcreateUniqueClassor a prop).Also applies to: 191-200
src/components/guided-modal-tour/index.js (3)
232-251: Query within the correct document for delegated/non-click events.Use the same
ownerDocumentforquerySelectorAllas you do for delegated clicks to support shadow DOM/iframes.- } else { - const elements = document.querySelectorAll( nextEventTarget ) + } else { + const elements = ( modalRef.current?.ownerDocument || document ).querySelectorAll( nextEventTarget ) @@ - } else { - const elements = document.querySelectorAll( nextEventTarget ) + } else { + const elements = ( modalRef.current?.ownerDocument || document ).querySelectorAll( nextEventTarget )Also applies to: 254-266
382-388: Prefer Modal’s onRequestClose over manual header listeners.You’ve disabled outside clicks; wiring
onRequestClose={ onClose }preserves Esc/close-button behavior without brittle DOM listeners. Then remove the header-click effect.- // onRequestClose={ onClose } // Do not use onRequestClose, it will cause the tour finish + onRequestClose={ onClose }And remove the header click effect:
- // When unmounted, do not call onClose. So we need to do this handler on our own. - useEffect( () => { - const handleHeaderClick = () => { - onClose() - } - if ( modalRef.current ) { - modalRef.current.querySelector( '.components-modal__header' ).addEventListener( 'click', handleHeaderClick ) - } - return () => { - if ( modalRef.current ) { - modalRef.current.querySelector( '.components-modal__header' ).removeEventListener( 'click', handleHeaderClick ) - } - } - }, [ modalRef.current, onClose ] )Also applies to: 357-371
80-84: Handle Settings.save() promise to avoid unhandled rejections.Network failures can surface as unhandled promise rejections. Swallow or log.
- const settings = new models.Settings( { stackable_guided_tour_states: [ ...guidedTourStates, tourId ] } ) - settings.save() + const settings = new models.Settings( { stackable_guided_tour_states: [ ...guidedTourStates, tourId ] } ) + settings.save().catch( () => {} )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/welcome/images/check.svgis excluded by!**/*.svgsrc/welcome/images/divider.svgis excluded by!**/*.svgsrc/welcome/images/quick-buttons-arrow.svgis excluded by!**/*.svg
📒 Files selected for processing (50)
.config/rules.js(1 hunks).github/workflows/playwright.yml(1 hunks)package.json(1 hunks)plugin.php(1 hunks)src/components/color-scheme-preview/index.js(1 hunks)src/components/design-library-list/design-library-list-item.js(3 hunks)src/components/design-library-list/util.js(4 hunks)src/components/guided-modal-tour/editor.scss(1 hunks)src/components/guided-modal-tour/index.js(1 hunks)src/components/guided-modal-tour/tour-steps.js(1 hunks)src/components/index.js(2 hunks)src/components/modal-design-library/modal.js(8 hunks)src/components/style-guide/components.js(1 hunks)src/components/style-guide/editor.scss(1 hunks)src/components/style-guide/images/readme.txt(1 hunks)src/components/style-guide/index.js(1 hunks)src/components/style-guide/popover.js(1 hunks)src/components/style-guide/utils.js(1 hunks)src/hooks/index.js(1 hunks)src/hooks/use-block-color-schemes.js(8 hunks)src/hooks/use-design-system.js(1 hunks)src/init.php(1 hunks)src/plugins/design-library-button/design-library-button.js(2 hunks)src/plugins/global-settings/block-styles/editor-loader.js(2 hunks)src/plugins/global-settings/block-styles/index.js(2 hunks)src/plugins/global-settings/color-schemes/editor-loader.js(4 hunks)src/plugins/global-settings/colors/index.js(2 hunks)src/plugins/global-settings/editor.scss(1 hunks)src/plugins/global-settings/index.js(3 hunks)src/plugins/global-settings/typography/index.js(5 hunks)src/plugins/global-settings/typography/store.js(1 hunks)src/plugins/global-settings/typography/typography-picker.js(3 hunks)src/plugins/global-settings/utils/index.js(1 hunks)src/plugins/global-settings/utils/use-block-layout-editor-loader.js(3 hunks)src/plugins/guided-modal-tour-trigger/index.js(1 hunks)src/plugins/index.js(1 hunks)src/plugins/theme-block-style-inheritance/index.php(2 hunks)src/util/block-styles.js(1 hunks)src/util/block-templates.js(1 hunks)src/util/blocks.js(1 hunks)src/util/styles/block-style-generators.js(1 hunks)src/util/styles/index.js(2 hunks)src/util/typography/description.js(1 hunks)src/util/typography/index.js(1 hunks)src/welcome/admin.scss(1 hunks)src/welcome/getting-started-banner-bg.scss(0 hunks)src/welcome/getting-started.js(2 hunks)src/welcome/getting-started.php(1 hunks)src/welcome/getting-started.scss(7 hunks)src/welcome/index.php(1 hunks)
💤 Files with no reviewable changes (1)
- src/welcome/getting-started-banner-bg.scss
🧰 Additional context used
🧬 Code graph analysis (24)
src/hooks/use-design-system.js (5)
src/hooks/use-block-color-schemes.js (1)
useSelect(19-217)src/plugins/global-settings/utils/use-block-layout-editor-loader.js (4)
useSelect(141-147)blockLayoutDefaults(155-155)renderGlobalBlockLayoutStyles(23-138)renderGlobalBlockLayoutStyles(23-138)src/plugins/global-settings/block-styles/editor-loader.js (3)
allBlockStyles(27-27)renderGlobalBlockStyleStyles(4-24)renderGlobalBlockStyleStyles(4-24)src/util/typography/description.js (2)
createTypographyDescription(13-54)createTypographyDescription(13-54)src/plugins/global-settings/color-schemes/editor-loader.js (3)
colorSchemes(40-40)renderGlobalColorSchemeStyles(24-120)renderGlobalColorSchemeStyles(24-120)
src/plugins/global-settings/block-styles/index.js (1)
src/components/color-palette-control/index.js (1)
applyFilters(119-119)
src/util/typography/description.js (1)
src/plugins/global-settings/typography/typography-picker.js (1)
description(146-146)
src/components/guided-modal-tour/index.js (1)
src/components/guided-modal-tour/tour-steps.js (2)
TOUR_STEPS(49-199)TOUR_STEPS(49-199)
src/plugins/guided-modal-tour-trigger/index.js (1)
src/higher-order/with-block-wrapper-is-hovered/util.js (1)
window(8-11)
src/components/style-guide/popover.js (3)
src/components/style-guide/index.js (2)
props(23-23)StyleGuide(22-63)src/hooks/use-design-system.js (2)
useDesignSystem(18-129)useDesignSystem(18-129)src/components/popover/index.js (1)
Popover(6-24)
src/plugins/global-settings/typography/store.js (1)
src/plugins/global-settings/typography/index.js (1)
typographySettings(168-168)
src/plugins/design-library-button/design-library-button.js (2)
src/components/guided-modal-tour/index.js (1)
GuidedModalTour(34-95)src/icons/index.js (1)
SVGStackableIcon(106-108)
src/components/modal-design-library/modal.js (2)
src/components/guided-modal-tour/index.js (1)
GuidedModalTour(34-95)src/components/color-scheme-preview/index.js (1)
ColorSchemePreview(32-83)
src/components/style-guide/components.js (4)
src/components/style-guide/utils.js (12)
__(285-287)DefaultButton(173-184)DefaultButton(173-184)DefaultOutlineButton(186-200)DefaultOutlineButton(186-200)RenderBlock(284-299)RenderBlock(284-299)blockName(237-237)getPlaceholders(301-335)getPlaceholders(301-335)attributes(303-303)innerBlocks(302-302)src/util/block-templates.js (2)
LONG_TEXT(5-18)LONG_TEXT(5-18)src/components/color-scheme-preview/index.js (2)
COLOR_SCHEME_PROPERTY_LABELS(8-17)COLOR_SCHEME_PROPERTY_LABELS(8-17)src/util/styles/index.js (3)
color(23-23)isDarkColor(20-55)isDarkColor(20-55)
src/components/color-scheme-preview/index.js (1)
src/components/style-guide/utils.js (1)
__(285-287)
src/util/blocks.js (1)
src/components/style-guide/utils.js (2)
serialized(241-241)attributes(303-303)
src/components/guided-modal-tour/tour-steps.js (1)
src/components/guided-modal-tour/index.js (1)
TOUR_STEPS(45-49)
src/components/design-library-list/design-library-list-item.js (2)
src/components/design-library-list/util.js (1)
DEFAULT_CONTENT(9-9)src/util/blocks.js (2)
cleanSerializedBlock(613-621)cleanSerializedBlock(613-621)
src/components/style-guide/utils.js (5)
src/components/style-guide/index.js (1)
DUMMY_COLOR_SCHEMES(25-34)src/components/design-library-list/util.js (5)
block(201-201)defaultValues(148-148)DEFAULT_CONTENT(9-9)addPlaceholderForPostsBlock(207-277)addPlaceholderForPostsBlock(207-277)src/util/blocks.js (4)
block(324-324)block(467-467)cleanSerializedBlock(613-621)cleanSerializedBlock(613-621)src/components/design-library-list/design-library-list-item.js (2)
DEFAULT_CONTENT(36-36)DEFAULT_CONTENT(36-36)src/util/block-templates.js (2)
PLACEHOLDER_INNER_BLOCKS(149-161)PLACEHOLDER_INNER_BLOCKS(149-161)
src/util/block-templates.js (2)
src/components/style-guide/utils.js (1)
__(285-287)src/block-components/alignment/edit.js (1)
sprintf(85-88)
src/components/style-guide/index.js (3)
src/components/style-guide/utils.js (3)
DUMMY_COLOR_SCHEMES(25-58)DUMMY_COLOR_SCHEMES(25-58)__(285-287)src/hooks/use-design-system.js (2)
designSystemStyles(104-110)previewClassNames(112-119)src/components/style-guide/components.js (10)
ColorSchemes(28-109)ColorSchemes(28-109)Colors(111-125)Colors(111-125)Typography(127-209)Typography(127-209)BlockStyles(211-286)BlockStyles(211-286)WebPreview(288-492)WebPreview(288-492)
src/plugins/global-settings/typography/index.js (1)
src/util/blocks.js (2)
dispatch(104-104)dispatch(153-153)
src/plugins/global-settings/color-schemes/editor-loader.js (1)
src/plugins/global-settings/color-schemes/utils.js (2)
schemeHasValue(29-35)schemeHasValue(29-35)
src/plugins/global-settings/index.js (3)
src/components/panel-advanced-settings/panel-body.js (1)
PanelBody(41-115)src/components/style-guide/popover.js (1)
StyleGuidePopover(25-63)src/components/guided-modal-tour/index.js (1)
GuidedModalTour(34-95)
src/plugins/global-settings/utils/use-block-layout-editor-loader.js (5)
src/plugins/global-settings/buttons-and-icons/index.js (1)
blockLayouts(42-56)src/plugins/global-settings/spacing-and-borders/index.js (1)
blockLayouts(44-58)src/hooks/use-design-system.js (1)
blockLayoutDefaults(68-68)src/plugins/global-settings/color-schemes/editor-loader.js (1)
currentHoverState(131-131)src/plugins/global-settings/utils/use-block-layout-inspector-utils.js (1)
currentHoverState(26-26)
src/plugins/global-settings/typography/typography-picker.js (1)
src/util/typography/description.js (3)
createTypographyDescription(13-54)createTypographyDescription(13-54)description(14-14)
src/components/design-library-list/util.js (2)
src/components/style-guide/utils.js (1)
defaultValues(275-275)src/block/posts/util.js (1)
separator(356-356)
src/hooks/use-block-color-schemes.js (2)
src/plugins/global-settings/color-schemes/utils.js (2)
value(45-45)obj(19-19)src/util/styles/index.js (2)
getPropertyLabel(300-304)getPropertyLabel(300-304)
🔇 Additional comments (55)
src/welcome/index.php (1)
276-276: Removal of.s-narrowwidens the Getting Started body—confirm layout intentDropping
.s-narrowremoves the max-width constraint defined in admin.scss. Verify large screens don’t get overly stretched content and that inner grids still look correct.If needed, reintroduce width via a more specific container class or media queries.
.github/workflows/playwright.yml (1)
81-84: Ensure the option is stored as a PHP array, not a JSON string
wp option update ... --format=jsonaffects output formatting, not input parsing. Usewp evalto update the option as a serialized array so the plugin reads it correctly.- - name: Disable guided tours - run: | - TOUR_STATES='["design-system", "editor", "design-library"]' - wp-env run tests-cli wp option update stackable_guided_tour_states "$TOUR_STATES" --format=json + - name: Disable guided tours + run: | + wp-env run tests-cli wp eval 'update_option( "stackable_guided_tour_states", array( "design-system", "editor", "design-library" ), false );'Likely an incorrect or invalid review comment.
src/plugins/theme-block-style-inheritance/index.php (1)
25-26: Expose setting to JS: good, but ensure boolean normalization downstream.Hook is fine. Make sure the injected value is a real boolean (see comment on add_setting).
src/util/block-styles.js (1)
7-9: Verify attribute keys in block schema
Please manually confirm that insrc/block/table-of-contents/schema.jsthestackable/table-of-contentsblock defines attributestextandheadings, and that insrc/block/icon-list-item/schema.jsthestackable/icon-list-itemblock defines theparentUniqueIdattribute.src/util/blocks.js (1)
612-621: Utility extraction looks good.Centralizing comment stripping reduces duplication and keeps renderPreview lean.
src/components/design-library-list/design-library-list-item.js (3)
16-16: Import refactor is good.Using cleanSerializedBlock from util is preferable to inline regex.
152-152: LGTM: uses cleanSerializedBlock for preview HTML.
36-36: DEFAULT_CONTENT export is correct. Verified that all references import DEFAULT_CONTENT from design-library-list-item.js and none import it from util.js.src/plugins/global-settings/colors/index.js (1)
111-142: Always-render panel: check for perf/UX regressions.With isOpen gating removed, ColorPicker and toggles mount every time. Validate editor performance and whether this matches product intent.
src/plugins/index.js (1)
8-8: LGTM: plugin bootstrap now loads tour trigger.src/plugins/global-settings/typography/store.js (1)
6-41: Solid minimal store for typography settings.Action/selector/reducer shape is clear and sufficient for current usage.
src/plugins/global-settings/color-schemes/editor-loader.js (4)
24-31: LGTM! Sensible API extension with backwards compatibility.The addition of the
returnCssparameter with a default value offalsemaintains backwards compatibility while enabling direct CSS string retrieval. This is a clean way to extend the function's capabilities without breaking existing callers.
83-84: Good defensive programming with the additional guard clause.The extended condition ensures that default container colors are only emitted when there's actually a background color scheme with values present. This prevents unnecessary CSS generation and potential styling issues.
115-118: Clean implementation of the CSS return path.The early return when
returnCssis true is properly placed before thesetStylescall, allowing the function to be used both for direct CSS retrieval and for setting styles through the callback.
138-145: Function call correctly updated to use the new signature.The call site has been properly updated to use the newly exported
renderGlobalColorSchemeStylesfunction, maintaining the same behavior while using the public API.src/components/design-library-list/util.js (4)
7-7: Import statement looks good.The import of
META_SEPARATORSfrom the posts utility module is appropriate for the separator rendering logic.
207-207: Enhanced function signature provides better flexibility.The addition of the optional
imgparameter allows for custom image sources while maintaining backwards compatibility with the defaultnullvalue.
230-230: Proper handling of the metaSeparator attribute.Using the imported
META_SEPARATORSconstant with a sensible default of 'dot' ensures consistent separator rendering across posts.
255-267: Good improvements to the post rendering logic.The changes properly handle custom image sources and add the missing
authorNamereplacement. The use ofreplaceAllformetaSeparatorensures all occurrences are replaced, and the image tag replacement maintains proper styling with width attributes.src/welcome/getting-started.js (8)
9-23: New imports properly support the UI redesign.The addition of SVG assets and WordPress icons aligns well with the new Quick Start interface. The icons are properly imported from the official
@wordpress/iconspackage.
28-29: Good addition of state management for tour completion.Importing
guidedTourStatesandclassNamesenables dynamic button styling based on tour completion status, which enhances the user experience.
117-122: Clean welcome section implementation.The centered welcome message with the tag provides good visual hierarchy. The commented-out alternative title is preserved for potential A/B testing or future iterations.
139-149: Well-implemented tour link with completion tracking.The design-library tour link properly includes URL parameters and uses conditional styling to show completion status. The SVGCheck icon provides clear visual feedback.
176-216: Hidden placeholder sections properly preserved for future features.The TODO comments and
display: nonestyling clearly indicate these are placeholder sections for upcoming features (Site Kit and Design System Picker). This is a good practice for feature planning.
219-225: Getting Started video section properly implemented.The video embed is well-structured with appropriate iframe attributes for YouTube embedding, including necessary permissions for autoplay and fullscreen.
228-228: Visual divider adds good section separation.The SVGDivider component provides clear visual separation between the Quick Start section and Resources.
231-239: Resources section maintains clean structure.The Resources section properly renders the GeneralCard components with the existing props, maintaining the established pattern.
src/components/guided-modal-tour/tour-steps.js (1)
167-172: Verify nextEventTarget points to the actual “Pages” tab.Targeting the modal header looks like a placeholder and could stall progression if clicks never occur there.
Would you like me to update this step to target the real tab selector once it lands and add a safe fallback (e.g., keep Next button enabled)?
src/components/style-guide/editor.scss (1)
282-299: :has() selector support.:has() isn’t universally supported; consider scoping under @supports or providing a fallback to avoid layout regressions.
I can add an @supports wrapper and a simple width fallback if desired.
src/components/guided-modal-tour/editor.scss (2)
1-5: Overlay blocks no clicks (pointer-events: none); confirm UX.This prevents clicking the overlay to dismiss/advance. If intentional, fine; otherwise enable pointer events and handle click-to-close.
37-46: Entrance state is solid.Visible/visible-delayed classes produce smooth scale-in. No issues.
src/welcome/getting-started.scss (1)
211-222: Width: calc(100% + 100px) can cause horizontal scroll.Given
overflow-x: autoon the parent this may be intentional, but consider padding instead to avoid scrollbars.src/hooks/index.js (1)
24-24: Ignore default re‐export suggestion
There is noexport defaultinuse-design-system.js, soexport { default as … }would fail.export * from './use-design-system'already re-exports the nameduseDesignSystem, which must be imported asimport { useDesignSystem } from '~stackable/hooks'Likely an incorrect or invalid review comment.
src/util/typography/index.js (1)
5-5: LGTM: public surface expanded safely.Re-exporting createTypographyDescription is low risk and helpful.
plugin.php (1)
315-323: No frontend impact from global include.The only hook in src/welcome/getting-started.php is add_action('admin_init', …), which only runs in the admin context and doesn’t enqueue any assets on the frontend. No additional is_admin() guard is required.
src/util/styles/index.js (1)
18-18: Public re-export looks good.No issues spotted with exposing blockStyleGenerators.
src/plugins/global-settings/utils/index.js (1)
1-1: Re-export added correctly.Assuming renderGlobalBlockLayoutStyles is a named export in use-block-layout-editor-loader.js, this is fine.
src/components/index.js (2)
59-61: New component exports look correct.No conflicts with existing names.
128-129: Expose COLOR_SCHEME_PROPERTY_LABELS.Good addition for external consumers.
src/components/modal-design-library/modal.js (1)
219-221: Placing GuidedModalTour inside the modal is fine; verify step selectors.Ensure tour steps target elements within the modal context (and not the editor canvas) to avoid mis-highlighting. If steps reach outside, consider a portal/container prop.
src/plugins/global-settings/typography/index.js (2)
14-14: Ensure store side-effect import runs before dispatch usage.Importing './store' at module top is correct; it guarantees the store is registered before later dispatch calls.
201-202: Syncing the public store on load and updates is good.Keeps external consumers consistent with UI state.
Also applies to: 249-250
src/plugins/global-settings/block-styles/editor-loader.js (1)
33-34: Good reuse of the new helper.GlobalBlockStyles now delegates to renderGlobalBlockStyleStyles, keeping logic in one place.
src/plugins/global-settings/typography/typography-picker.js (2)
6-7: Centralizing description generation looks good.Importing and using the shared
createTypographyDescriptionremoves duplication.
146-146: LGTM on per-device description.This aligns the preview text with the active viewport.
src/components/color-scheme-preview/index.js (1)
1-5: LGTM: i18n wiring is correct.Importing i18n from stackable and using __ is consistent with the rest of the codebase.
src/components/style-guide/index.js (2)
36-38: Confirm replaceAll availability or use a polyfill-free alternative.If the build targets environments lacking String.prototype.replaceAll, prefer a split/join.
- .replaceAll( ':root', '&' ) + .split( ':root' ).join( '&' )
41-49: LGTM: theme/style scoping.Inlining THEME_STYLES under .ugb-style-guide__preview and injecting designSystemStyles looks good.
src/plugins/global-settings/index.js (2)
59-60: Verify useSelect deps behavior.Using [] is fine if you only need the initial user id; if the user can change without reload (e.g., switcher), this won’t update. Confirm desired behavior.
75-88: LGTM: capability gating and sidebar integration.Clean toggle flow and capability check before rendering the sidebar.
src/components/style-guide/popover.js (1)
43-44: Confirmplacementis correct
ThePopovercomponent currently uses theplacementprop (withpositiondeprecated but still supported), and for overlay-sized popovers you can rely onplacement="overlay"(optionally using the newanchorprop for custom anchoring). No changes needed.src/hooks/use-design-system.js (1)
104-111: LGTM on CSS composition.The concatenation order (typography → buttons/icons → spacing/borders → color schemes → block styles) looks sane for specificity/overrides.
src/welcome/getting-started.php (1)
51-54: LGTM: localization wiring.Exposing guidedTourStates via localization aligns with the front-end tour orchestration.
src/hooks/use-block-color-schemes.js (2)
60-77: Good normalization: exposes normal/hover/parentHover for desktop.The transition to scheme.normal/hover/parentHover is clear and localized, keeping original scheme intact (via cloneDeep). Looks good.
Also applies to: 79-82
169-183: Nice: switched to getPropertyLabel for palette entries.This improves consistency and i18n neutrality across properties.
| "@wordpress/icons": "^10.27.0", | ||
| "bigpicture": "^2.5.3", | ||
| "canvas-confetti": "^1.9.3", | ||
| "classnames": "^2.2.6", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New deps: verify license and security.
Please check canvas-confetti and html-to-image licenses and advisories before shipping.
Run:
🏁 Script executed:
#!/bin/bash
npm run check-licenses
npm audit --productionLength of output: 836
🏁 Script executed:
#!/bin/bash
set -e
# Locate the scripts section and any check-licenses script in package.json
grep -n '"scripts"' -n package.json -C3 || true
grep -R '"check-licenses"' -n package.json -C2 || true
# Verify that canvas-confetti and html-to-image are declared dependencies
grep -n '"canvas-confetti"' -n package.json -C2 || true
grep -n '"html-to-image"' -n package.json -C2 || true
# If a lockfile exists, run license and audit checks
if [[ -f package-lock.json ]]; then
npm run check-licenses || true
npm audit --production --omit=dev || true
else
echo "No lockfile found; generating one for audit..."
npm i --package-lock-only
npm run check-licenses || true
npm audit --production --omit=dev || true
fiLength of output: 2704
Ensure license checks succeed and remediate vulnerabilities.
- package.json (scripts): install
concurrentlyor adjust thecheck-licensescommand sonpm run check-licensescompletes without “concurrently: not found.” - Manually verify that
canvas-confettiandhtml-to-imagelicenses meet project requirements (e.g. GPL-2 or compatible). - Address the 5 moderate-severity
@babel/runtimevulnerabilities (affecting@wordpress/icons,escape-html, etc.) by runningnpm audit fix(note: this upgrades@wordpress/iconsto 10.9.0, a breaking change) or by pinning/patching dependencies.
🤖 Prompt for AI Agents
In package.json around lines 18-21, the project scripts and dependencies cause
the license check to fail because the check-licenses script depends on
"concurrently" which is not installed, and there are license and vulnerability
issues to remediate; fix by either adding "concurrently" to devDependencies and
running npm install so npm run check-licenses can run, or update the
check-licenses script to not rely on concurrently (e.g., run commands
sequentially or use npm-run-all), then manually verify that canvas-confetti and
html-to-image licenses meet project policy and replace or remove them if they do
not, and resolve the five moderate @babel/runtime vulnerabilities by running npm
audit fix (accepting any dependency upgrades such as upgrading @wordpress/icons)
or by pinning/patching the vulnerable sub-dependencies to safe versions and
re-running the audit until no moderate issues remain.
| useEffect( () => { | ||
| // Create the element. | ||
| const element = document.createElement( 'div' ) | ||
| element.className = `ugb-tour-modal__glow ugb-tour-modal__glow--hidden` | ||
| document.body.appendChild( element ) | ||
|
|
||
| // Keep track of the element. | ||
| glowElementRef.current = element | ||
|
|
||
| return () => { | ||
| glowElementRef.current = null | ||
| element.remove() | ||
| } | ||
| }, [] ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Attach glow element and target lookups to ownerDocument.
Appending to document.body and querying document breaks in iframes. Use ownerDocument consistently.
useEffect( () => {
- // Create the element.
- const element = document.createElement( 'div' )
+ const doc = modalRef.current?.ownerDocument || document
+ // Create the element.
+ const element = doc.createElement( 'div' )
element.className = `ugb-tour-modal__glow ugb-tour-modal__glow--hidden`
- document.body.appendChild( element )
+ doc.body.appendChild( element )
@@
- if ( glowTarget && isVisibleDelayed ) {
+ if ( glowTarget && isVisibleDelayed ) {
// Get the top, left, width, and height of the target.
- const target = document.querySelector( glowTarget )
+ const doc = modalRef.current?.ownerDocument || document
+ const target = doc.querySelector( glowTarget )
if ( target ) {
const targetRect = target.getBoundingClientRect()
@@
- const glowTargetSize = targetRect.width > 300 || targetRect.height > 200 ? 'large'
- : targetRect.width > 300 || targetRect.height > 100 ? 'medium'
+ const glowTargetSize = targetRect.width > 300 || targetRect.height > 200 ? 'large'
+ : targetRect.width > 200 || targetRect.height > 100 ? 'medium'
: 'small'Also applies to: 333-352
| description: __( 'These are hundreds ofpre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ), | ||
| help: createInterpolateElement( __( 'Pick one of the <strong>designs</strong> to continue.', i18n ), { | ||
| strong: <strong />, |
There was a problem hiding this comment.
Fix typo in user-facing copy.
"These are hundreds ofpre-built designs" → "These are hundreds of pre-built designs".
Apply this diff:
- description: __( 'These are hundreds ofpre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ),
+ description: __( 'These are hundreds of pre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: __( 'These are hundreds ofpre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ), | |
| help: createInterpolateElement( __( 'Pick one of the <strong>designs</strong> to continue.', i18n ), { | |
| strong: <strong />, | |
| description: __( 'These are hundreds of pre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ), | |
| help: createInterpolateElement( __( 'Pick one of the <strong>designs</strong> to continue.', i18n ), { | |
| strong: <strong />, |
🤖 Prompt for AI Agents
In src/components/guided-modal-tour/tour-steps.js around lines 114 to 116, fix
the typo in the user-facing description string: change "These are hundreds
ofpre-built designs" to "These are hundreds of pre-built designs" so there is a
space between "hundreds of" and "pre-built"; update the string passed to the
i18n __(...) call accordingly and ensure no other formatting or i18n key changes
are made.
| size: 'medium', | ||
| nextEventTarget: '.ugb-design-library-item', | ||
| offsetX: '-400px', | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
offsetX type should be a number, not a px string.
The docs above specify numbers. Using a string may break positioning math.
Apply this diff:
- offsetX: '-400px',
+ offsetX: -400,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| size: 'medium', | |
| nextEventTarget: '.ugb-design-library-item', | |
| offsetX: '-400px', | |
| }, | |
| size: 'medium', | |
| nextEventTarget: '.ugb-design-library-item', | |
| offsetX: -400, | |
| }, |
🤖 Prompt for AI Agents
In src/components/guided-modal-tour/tour-steps.js around lines 118 to 121, the
offsetX prop is set as the string '-400px' but the component expects a numeric
value; change offsetX to a number (e.g., -400) and ensure any related offset
props use numbers as well, updating any tests or usages that assume a string
type.
| export const createTypographyDescription = ( styleObject, device = 'desktop', separator = ', ' ) => { | ||
| const description = [] | ||
| if ( styleObject.fontFamily ) { | ||
| description.push( getFontFamilyLabel( styleObject.fontFamily ) ) | ||
| } | ||
| if ( styleObject.fontSize ) { | ||
| description.push( `${ styleObject.fontSize }${ styleObject.fontSizeUnit || 'px' }` ) | ||
| } | ||
|
|
||
| // Show the correct font size when in tablet or mobile previews. | ||
| if ( device === 'tablet' && styleObject.tabletFontSize ) { | ||
| if ( styleObject.fontSize ) { | ||
| description.pop() | ||
| } | ||
| description.push( `${ styleObject.tabletFontSize }${ styleObject.tabletFontSizeUnit || 'px' }` ) | ||
| } else if ( device === 'mobile' && ( styleObject.tabletFontSize || styleObject.mobileFontSize ) ) { | ||
| if ( styleObject.fontSize ) { | ||
| description.pop() | ||
| } | ||
| if ( styleObject.mobileFontSize ) { | ||
| description.push( `${ styleObject.mobileFontSize }${ styleObject.mobileFontSizeUnit || 'px' }` ) | ||
| } else { | ||
| description.push( `${ styleObject.tabletFontSize }${ styleObject.tabletFontSizeUnit || 'px' }` ) | ||
| } | ||
| } | ||
|
|
||
| if ( styleObject.fontWeight ) { | ||
| description.push( styleObject.fontWeight ) | ||
| } | ||
| if ( styleObject.textTransform ) { | ||
| description.push( upperFirst( styleObject.textTransform ) ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
0 font sizes are ignored; simplify device font-size selection and make input robust.
Truthiness checks drop 0 values and the pop/replace approach is fragile. Choose the size once per device and push it with the correct unit. Also default styleObject to {}.
-export const createTypographyDescription = ( styleObject, device = 'desktop', separator = ', ' ) => {
+export const createTypographyDescription = ( styleObject = {}, device = 'desktop', separator = ', ' ) => {
const description = []
if ( styleObject.fontFamily ) {
description.push( getFontFamilyLabel( styleObject.fontFamily ) )
}
- if ( styleObject.fontSize ) {
- description.push( `${ styleObject.fontSize }${ styleObject.fontSizeUnit || 'px' }` )
- }
-
- // Show the correct font size when in tablet or mobile previews.
- if ( device === 'tablet' && styleObject.tabletFontSize ) {
- if ( styleObject.fontSize ) {
- description.pop()
- }
- description.push( `${ styleObject.tabletFontSize }${ styleObject.tabletFontSizeUnit || 'px' }` )
- } else if ( device === 'mobile' && ( styleObject.tabletFontSize || styleObject.mobileFontSize ) ) {
- if ( styleObject.fontSize ) {
- description.pop()
- }
- if ( styleObject.mobileFontSize ) {
- description.push( `${ styleObject.mobileFontSize }${ styleObject.mobileFontSizeUnit || 'px' }` )
- } else {
- description.push( `${ styleObject.tabletFontSize }${ styleObject.tabletFontSizeUnit || 'px' }` )
- }
- }
+ const baseSize = styleObject.fontSize
+ const tabletSize = styleObject.tabletFontSize
+ const mobileSize = styleObject.mobileFontSize
+ let sizeForDevice = baseSize
+ if ( device === 'tablet' && tabletSize != null ) {
+ sizeForDevice = tabletSize
+ } else if ( device === 'mobile' && ( mobileSize != null || tabletSize != null ) ) {
+ sizeForDevice = mobileSize != null ? mobileSize : tabletSize
+ }
+ if ( sizeForDevice != null ) {
+ const unit =
+ device === 'tablet' && tabletSize != null
+ ? ( styleObject.tabletFontSizeUnit || 'px' )
+ : device === 'mobile' && mobileSize != null
+ ? ( styleObject.mobileFontSizeUnit || 'px' )
+ : ( styleObject.fontSizeUnit || 'px' )
+ description.push( `${ sizeForDevice }${ unit }` )
+ }
@@
- if ( styleObject.fontWeight ) {
+ if ( styleObject.fontWeight != null && styleObject.fontWeight !== '' ) {
description.push( styleObject.fontWeight )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const createTypographyDescription = ( styleObject, device = 'desktop', separator = ', ' ) => { | |
| const description = [] | |
| if ( styleObject.fontFamily ) { | |
| description.push( getFontFamilyLabel( styleObject.fontFamily ) ) | |
| } | |
| if ( styleObject.fontSize ) { | |
| description.push( `${ styleObject.fontSize }${ styleObject.fontSizeUnit || 'px' }` ) | |
| } | |
| // Show the correct font size when in tablet or mobile previews. | |
| if ( device === 'tablet' && styleObject.tabletFontSize ) { | |
| if ( styleObject.fontSize ) { | |
| description.pop() | |
| } | |
| description.push( `${ styleObject.tabletFontSize }${ styleObject.tabletFontSizeUnit || 'px' }` ) | |
| } else if ( device === 'mobile' && ( styleObject.tabletFontSize || styleObject.mobileFontSize ) ) { | |
| if ( styleObject.fontSize ) { | |
| description.pop() | |
| } | |
| if ( styleObject.mobileFontSize ) { | |
| description.push( `${ styleObject.mobileFontSize }${ styleObject.mobileFontSizeUnit || 'px' }` ) | |
| } else { | |
| description.push( `${ styleObject.tabletFontSize }${ styleObject.tabletFontSizeUnit || 'px' }` ) | |
| } | |
| } | |
| if ( styleObject.fontWeight ) { | |
| description.push( styleObject.fontWeight ) | |
| } | |
| if ( styleObject.textTransform ) { | |
| description.push( upperFirst( styleObject.textTransform ) ) | |
| } | |
| export const createTypographyDescription = ( styleObject = {}, device = 'desktop', separator = ', ' ) => { | |
| const description = [] | |
| if ( styleObject.fontFamily ) { | |
| description.push( getFontFamilyLabel( styleObject.fontFamily ) ) | |
| } | |
| const baseSize = styleObject.fontSize | |
| const tabletSize = styleObject.tabletFontSize | |
| const mobileSize = styleObject.mobileFontSize | |
| let sizeForDevice = baseSize | |
| if ( device === 'tablet' && tabletSize != null ) { | |
| sizeForDevice = tabletSize | |
| } else if ( device === 'mobile' && ( mobileSize != null || tabletSize != null ) ) { | |
| sizeForDevice = mobileSize != null ? mobileSize : tabletSize | |
| } | |
| if ( sizeForDevice != null ) { | |
| const unit = | |
| device === 'tablet' && tabletSize != null | |
| ? ( styleObject.tabletFontSizeUnit || 'px' ) | |
| : device === 'mobile' && mobileSize != null | |
| ? ( styleObject.mobileFontSizeUnit || 'px' ) | |
| : ( styleObject.fontSizeUnit || 'px' ) | |
| description.push( `${ sizeForDevice }${ unit }` ) | |
| } | |
| if ( styleObject.fontWeight != null && styleObject.fontWeight !== '' ) { | |
| description.push( styleObject.fontWeight ) | |
| } | |
| if ( styleObject.textTransform ) { | |
| description.push( upperFirst( styleObject.textTransform ) ) | |
| } | |
| return description.join( separator ) | |
| } |
🤖 Prompt for AI Agents
In src/util/typography/description.js around lines 13 to 44, the function
currently treats styleObject as required and uses truthy checks/pop to replace
desktop sizes which drops valid 0 values and is fragile; change the function
signature to default styleObject = {}, determine the effective size once per
device by checking for !== undefined (or != null) for each device-specific
property (tabletFontSize, mobileFontSize) instead of popping, compute the
correct unit for that chosen size (falling back to 'px' per corresponding unit
field), and push a single formatted size string; ensure all other checks use
explicit undefined/null checks so 0 is preserved.
| // If a css custom property, get just the name names | ||
| return description.map( value => { | ||
| if ( value.includes( 'var(' ) ) { | ||
| const propName = value.match( /var\(([^\),]*)/ )?.[ 1 ] | ||
| return startCase( last( propName.split( '--' ) ) ) | ||
| } | ||
| return value | ||
| } ).join( separator ) |
There was a problem hiding this comment.
Runtime error: value.includes may be called on numbers.
fontWeight can be numeric; calling includes on a number throws. Type-guard string values before parsing CSS vars.
- return description.map( value => {
- if ( value.includes( 'var(' ) ) {
- const propName = value.match( /var\(([^\),]*)/ )?.[ 1 ]
- return startCase( last( propName.split( '--' ) ) )
- }
- return value
- } ).join( separator )
+ return description
+ .map( value => {
+ if ( typeof value === 'string' && value.includes( 'var(' ) ) {
+ const propName = value.match( /var\(([^\),]*)/ )?.[ 1 ]
+ return propName ? startCase( last( propName.split( '--' ) ) ) : value
+ }
+ return value
+ } )
+ .join( separator )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If a css custom property, get just the name names | |
| return description.map( value => { | |
| if ( value.includes( 'var(' ) ) { | |
| const propName = value.match( /var\(([^\),]*)/ )?.[ 1 ] | |
| return startCase( last( propName.split( '--' ) ) ) | |
| } | |
| return value | |
| } ).join( separator ) | |
| // If a css custom property, get just the name names | |
| return description | |
| .map( value => { | |
| if ( typeof value === 'string' && value.includes( 'var(' ) ) { | |
| const propName = value.match( /var\(([^\),]*)/ )?.[ 1 ] | |
| return propName ? startCase( last( propName.split( '--' ) ) ) : value | |
| } | |
| return value | |
| } ) | |
| .join( separator ) |
🤖 Prompt for AI Agents
In src/util/typography/description.js around lines 46 to 53, the code calls
value.includes which can throw if value is a number (e.g., numeric fontWeight);
guard by checking typeof value === 'string' before using includes/match, only
parse CSS vars for string values, and for non-strings return value (or
String(value) if the surrounding logic expects a string) so numbers are not
processed with string methods.
| @@ -3,7 +3,7 @@ | |||
| */ | |||
|
|
|||
| :root { | |||
| --stk-welcome-primary: #dd2590; | |||
| --stk-welcome-primary: #f00069; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify contrast of new brand color and use the CSS var in gradients
Check WCAG contrast of #f00069 vs white (14px text used in buttons). If ratio < 4.5:1, consider darkening or increasing font size/weight.
Run this to compute contrast against #fff and #101828:
Also, for consistency, replace hardcoded #f00069 in gradients with the variable:
- background: linear-gradient(135deg, #b300be, #f00069);
+ background: linear-gradient(135deg, #b300be, var(--stk-welcome-primary));Apply similarly to other occurrences (e.g., .s-save-changes).
🏁 Script executed:
#!/bin/bash
python - <<'PY'
def srgb(c):
c=c/255.0
return c/12.92 if c<=0.03928 else ((c+0.055)/1.055)**2.4
def lum(rgb):
r,g,b=[srgb(x) for x in rgb]
return 0.2126*r+0.7152*g+0.0722*b
def ratio(c1,c2):
L1,L2=sorted([lum(c1),lum(c2)],reverse=True)
return (L1+0.05)/(L2+0.05)
hex2=lambda h:(int(h[1:3],16),int(h[3:5],16),int(h[5:],16))
pink=hex2("#f00069")
print("Contrast vs #FFFFFF:", ratio(pink,hex2("#ffffff")))
print("Contrast vs #101828:", ratio(pink,hex2("#101828")))
PYLength of output: 77
Darken primary color or adjust text to meet WCAG AA contrast: Verified that --stk-welcome-primary (#f00069) yields only 4.28:1 vs #fff and 4.15:1 vs #101828 – both below the 4.5:1 threshold for 14px text. Darken the variable or increase text size/weight to achieve ≥4.5:1. Replace all hardcoded #f00069 (eg. in background gradients) with var(--stk-welcome-primary) for consistency.
🤖 Prompt for AI Agents
In src/welcome/admin.scss around line 6, the primary CSS variable
--stk-welcome-primary is set to #f00069 which fails WCAG AA contrast (4.28:1 vs
#fff and 4.15:1 vs #101828); update the variable to a darker hex (or use a
slightly larger/heavier text style) so the contrast ratio is >= 4.5:1 for 14px
text, then replace every hardcoded #f00069 occurrence (including background
gradients) with var(--stk-welcome-primary) to ensure consistency across the
stylesheet.
| 'show_in_rest' => array( | ||
| 'schema' => array( | ||
| 'items' => array( | ||
| 'type' => 'string', | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
REST schema missing “type”.
show_in_rest schema should declare type: 'array'; current schema only defines items.
- 'show_in_rest' => array(
- 'schema' => array(
- 'items' => array(
- 'type' => 'string',
- ),
- ),
- ),
+ 'show_in_rest' => array(
+ 'schema' => array(
+ 'type' => 'array',
+ 'items' => array(
+ 'type' => 'string',
+ ),
+ ),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'show_in_rest' => array( | |
| 'schema' => array( | |
| 'items' => array( | |
| 'type' => 'string', | |
| ), | |
| ), | |
| ), | |
| 'show_in_rest' => array( | |
| 'schema' => array( | |
| 'type' => 'array', | |
| 'items' => array( | |
| 'type' => 'string', | |
| ), | |
| ), | |
| ), |
🤖 Prompt for AI Agents
In src/welcome/getting-started.php around lines 35 to 41, the REST schema for
show_in_rest currently only defines 'items' but is missing the top-level 'type'
declaration; update the schema to include "type" => "array" alongside the
existing "items" entry so the REST API properly recognizes this field as an
array.
| public function sanitize_array_setting( $input ) { | ||
| return array_map( 'sanitize_text_field', $input ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sanitize callback should handle non-array input.
Avoid warnings if a scalar is posted.
- public function sanitize_array_setting( $input ) {
- return array_map( 'sanitize_text_field', $input );
- }
+ public function sanitize_array_setting( $input ) {
+ if ( ! is_array( $input ) ) {
+ return array();
+ }
+ return array_map( 'sanitize_text_field', $input );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function sanitize_array_setting( $input ) { | |
| return array_map( 'sanitize_text_field', $input ); | |
| } | |
| public function sanitize_array_setting( $input ) { | |
| if ( ! is_array( $input ) ) { | |
| return array(); | |
| } | |
| return array_map( 'sanitize_text_field', $input ); | |
| } |
🤖 Prompt for AI Agents
In src/welcome/getting-started.php around lines 47 to 49, the
sanitize_array_setting function currently assumes $input is an array which
triggers warnings for scalar or null values; modify it to first check if $input
is an array and if so return array_map('sanitize_text_field', $input), otherwise
if $input is null return an empty array, and if $input is a scalar return an
array containing sanitize_text_field($input) (or return the sanitized scalar if
the API expects a scalar) so no warnings occur when non-array input is posted.
# Conflicts: # src/components/design-library-list/design-library-list-item.js # src/components/design-library-list/util.js # src/components/modal-design-library/modal.js
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/design-library-list/use-preview-renderer.js (1)
173-178: Review cleanSerializedBlock behavior and adjust if needed
- cleanSerializedBlock uses
/<!--[\s\S]*?-->/g, which removes all HTML comments (including customstk-start/endmarkers) rather than only Gutenberg block comments.- If you must preserve template markers, update the regex to target only Gutenberg comments (e.g.
<!--\s*wp:[^>]+-->), then strip those.- cleanSerializedBlock is idempotent (calling it again on its own output has no further effect).
🧹 Nitpick comments (3)
src/components/design-library-list/util.js (3)
10-10: Meta separator import looks right; add safe fallback for unknown keys.Use a default when attrs.metaSeparator is unrecognized.
Apply:
-const separator = META_SEPARATORS[ attrs.metaSeparator ?? 'dot' ] +const metaKey = attrs.metaSeparator ?? 'dot' +const separator = META_SEPARATORS[ metaKey ] ?? META_SEPARATORS.dot
289-302: Avoid invalid img width attribute and sanitize src.
- width attribute doesn’t accept “auto” or “px” units; rely on style width instead, or emit numeric-only width.
- Minor hardening: don’t inject img unescaped if a custom img is ever user-provided.
Apply:
-const renderedPosts = currentPosts.map( ( post, index ) => { - const imgSrc = img ?? `${ IMAGE_STORAGE }stk-design-library-image-${ index + 1 }.jpeg` - return template +const renderedPosts = currentPosts.map( ( post, index ) => { + const imgSrc = img ?? `${ IMAGE_STORAGE }stk-design-library-image-${ index + 1 }.jpeg` + const widthAttr = Number.isFinite( attrs.imageWidth ) ? ` width="${ attrs.imageWidth }"` : '' + return template .replace( /!#title!#/g, post.title_placeholder ) .replace( /!#excerpt!#/g, post.text_placeholder ) .replace( /!#authorName!#/g, 'John Doe' ) .replaceAll( /!#metaSeparator!#/g, separator ) .replace( /!#commentsNum!#/g, '3 comments' ) .replace( /!#date!#/g, 'March 1, 2025' ) .replace( /!#readmoreText!#/g, defaultValues[ 'post-btn_placeholder' ] ) .replace( /!#category!#/g, defaultValues.tag_placeholder ) - .replace( /img class="stk-img"/g, `img class="stk-img" src="${ imgSrc }" width="${ width }" style="width: ${ width } !important;"` ) + .replace( + /img class="stk-img"/g, + `img class="stk-img" src="${ imgSrc }"${ widthAttr } style="width: ${ width } !important;"` + ) } ).join( '\n' )Optional: validate/sanitize imgSrc if external sources may pass img.
289-299: Use translatable placeholders for author, comments, and date.Hard-coded English text and a fixed date can look stale in previews and aren’t i18n-friendly. Consider __() and a relative date (e.g., new Date()) for freshness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
plugin.php(1 hunks)src/components/design-library-list/use-preview-renderer.js(2 hunks)src/components/design-library-list/util.js(4 hunks)src/components/modal-design-library/modal.js(8 hunks)src/components/style-guide/utils.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/style-guide/utils.js
- plugin.php
- src/components/modal-design-library/modal.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/design-library-list/util.js (3)
src/components/design-library-list/use-preview-renderer.js (1)
DEFAULT_CONTENT(35-35)src/components/style-guide/utils.js (1)
defaultValues(274-274)src/block/posts/util.js (1)
separator(356-356)
🔇 Additional comments (4)
src/components/design-library-list/use-preview-renderer.js (1)
33-33: Good move: centralize preview cleanup via shared util.Switching to cleanSerializedBlock reduces duplication and keeps behavior consistent across call sites.
src/components/design-library-list/util.js (3)
274-288: Non-mutating post selection is cleaner and predictable.Using slice/pad avoids hidden state and works for numItems > placeholders.
241-241: addPlaceholderForPostsBlock callers safe—no changes neededBoth usages pass either the new
imgargument or omit it (falling back to the defaultnull), so the signature extension is backward-compatible.
12-12: DEFAULT_CONTENT named export usage confirmed No default-import patterns detected; all imports use the named export correctly.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
src/components/modal-design-library/modal.js (4)
264-267: Null-safe access and BC fallback for scheme preview (can crash today)Indexing colorSchemesCollection[...] and assuming .normal exists can throw if the key is absent or if older data still uses desktopColors. Guard with optional chaining and a desktopColors fallback.
Apply:
-? <ColorSchemePreview isCollapsed={ true } colors={ colorSchemesCollection[ selectedBackgroundScheme || backgroundModeColorScheme ].normal } /> +? <ColorSchemePreview + isCollapsed={ true } + colors={ + colorSchemesCollection?.[ selectedBackgroundScheme || backgroundModeColorScheme ]?.normal + || colorSchemesCollection?.[ selectedBackgroundScheme || backgroundModeColorScheme ]?.desktopColors + || {} + } + />
305-307: Guard scheme.normal and keep desktopColors fallback in the list itemsAvoid runtime errors when schemes are incomplete or legacy-shaped.
Apply:
-<ColorSchemePreview colors={ scheme.normal } isCollapsed={ true } /> +<ColorSchemePreview + colors={ scheme?.normal || scheme?.desktopColors || {} } + isCollapsed={ true } +/>
331-333: Null-safe access and BC fallback for Container Scheme toggle previewSame crash risk as Background Scheme toggle.
Apply:
-? <ColorSchemePreview isCollapsed={ true } colors={ colorSchemesCollection[ selectedContainerScheme || containerModeColorScheme ].normal } /> +? <ColorSchemePreview + isCollapsed={ true } + colors={ + colorSchemesCollection?.[ selectedContainerScheme || containerModeColorScheme ]?.normal + || colorSchemesCollection?.[ selectedContainerScheme || containerModeColorScheme ]?.desktopColors + || {} + } + />
364-366: Guard scheme.normal in Container Scheme list itemsMaintain compatibility with legacy desktopColors and prevent undefined access.
Apply:
-<ColorSchemePreview colors={ scheme.normal } isCollapsed={ true } /> +<ColorSchemePreview + colors={ scheme?.normal || scheme?.desktopColors || {} } + isCollapsed={ true } +/>src/components/style-guide/utils.js (2)
207-221: Guard against missing or incomplete block style generatorsUnknown blocks (or missing exports) will throw when calling methods on an undefined generator.
- const blockStyleGenerator = blockStyleGenerators[ block.name ] - const attrNamesWithValues = blockStyleGenerator.getAttributesWithValues( block.attributes ) - const blockStyleDefs = blockStyleGenerator.getBlockStyles( attrNamesWithValues ) + const blockStyleGenerator = blockStyleGenerators[ block.name ] + if ( + ! blockStyleGenerator || + typeof blockStyleGenerator.getAttributesWithValues !== 'function' || + typeof blockStyleGenerator.getBlockStyles !== 'function' || + typeof blockStyleGenerator.generateBlockStylesForSave !== 'function' + ) { + if ( generateForInnerBlocks && Array.isArray( block.innerBlocks ) ) { + block.innerBlocks = getGeneratedCss( block.innerBlocks, generateForInnerBlocks ) + } + return block + } + const attrNamesWithValues = blockStyleGenerator.getAttributesWithValues( block.attributes ) + const blockStyleDefs = blockStyleGenerator.getBlockStyles( attrNamesWithValues )
292-297: Eliminate XSS by rendering label outside RawHTMLInterpolating name as HTML enables markup injection; render it as a normal React node.
- return ( - <RawHTML> - { cleanSerializedBlock( serialized, SERIALIZE_CALLBACKS[ blockName ], attributes ) } - { `<p>${ name }</p>` } - </RawHTML> - ) + return ( + <> + <RawHTML> + { cleanSerializedBlock( serialized, SERIALIZE_CALLBACKS[ blockName ], attributes ) } + </RawHTML> + <p>{ name }</p> + </> + )
🧹 Nitpick comments (11)
src/components/design-library-list/use-preview-renderer.js (1)
33-33: cleanSerializedBlock export and alias resolution confirmed
Thesrc/util/index.jsbarrel re-exports everything from./blocks(includingcleanSerializedBlock), soimport { cleanSerializedBlock } from '~stackable/util'will resolve correctly.Optional: for smaller bundles, deep-import directly from
~stackable/util/blocks.src/components/modal-design-library/modal.js (1)
276-281: Mirror the close-button classname for Container Scheme for selector parityBackground Scheme popover adds ugb-modal-design-library__color-scheme-close-button; Container Scheme popover should match to keep styling/QA hooks consistent.
Apply near Line 340:
-<Button +<Button icon="no" + className="ugb-modal-design-library__color-scheme-close-button" onClick={ () => { onClose() } } />plugin.php (1)
314-314: Gate the include to non-frontend contexts to avoid loading admin UI on public requests.Unconditional require loads on every frontend hit. Include only for admin/REST to preserve endpoints while keeping the public site lean.
- require_once( plugin_dir_path( __FILE__ ) . 'src/welcome/getting-started.php' ); + // Load Getting Started only for admin screens and REST/AJAX, skip public frontend. + if ( ! is_frontend() ) { + require_once( plugin_dir_path( __FILE__ ) . 'src/welcome/getting-started.php' ); + }src/components/design-library-list/util.js (5)
12-12: Avoid duplicate DEFAULT_CONTENT definitions across filesThis now exports DEFAULT_CONTENT here, but src/components/design-library-list/use-preview-renderer.js also defines its own constant. Prefer a single source of truth by importing from this module to prevent drift.
Example (outside this diff):
// in use-preview-renderer.js import { DEFAULT_CONTENT } from '../design-library-list/util'
267-270: Template markers regex is brittle (leading slash required)The patterns require a leading "/" (e.g., ). If actual markers sometimes omit it, no replacement occurs. Make the "/" optional in both match and replace to be resilient.
Apply:
-const templateMatch = innerHtml.match( /<!--\s*\/stk-start:posts\/template\s*-->([\s\S]*?)<!--\s*\/stk-end:post\/template\s*-->/ ) +const templateMatch = innerHtml.match( /<!--\s*\/?stk-start:posts\/template\s*-->([\s\S]*?)<!--\s*\/?stk-end:post\/template\s*-->/ ) ... -const updatedInnerHtml = innerHtml.replace( - /<!--\s*\/stk-start:posts\/template\s*-->([\s\S]*?)<!--\s*\/stk-end:post\/template\s*-->/, +const updatedInnerHtml = innerHtml.replace( + /<!--\s*\/?stk-start:posts\/template\s*-->([\s\S]*?)<!--\s*\/?stk-end:post\/template\s*-->/, renderedPosts )Also applies to: 304-307
264-264: Guard unknown metaSeparator keysIf attrs.metaSeparator is unknown, separator becomes undefined. Add a fallback to a safe default.
-const separator = META_SEPARATORS[ attrs.metaSeparator ?? 'dot' ] +const sepKey = attrs.metaSeparator ?? 'dot' +const separator = META_SEPARATORS[ sepKey ] ?? META_SEPARATORS.dot ?? '•'
241-243: Selection doesn’t advance across multiple posts blockscurrentPosts uses slice without consuming/advancing a cursor. Multiple posts blocks in the same content will all render the same first N placeholders. Track a cursor per call and advance it per block.
Apply:
-export const addPlaceholderForPostsBlock = ( content, postsPlaceholder, defaultValues, img = null ) => { - const remainingPosts = [ ...postsPlaceholder ] +export const addPlaceholderForPostsBlock = ( content, postsPlaceholder, defaultValues, img = null ) => { + const placeholders = [ ...postsPlaceholder ] + let postCursor = 0 @@ - let currentPosts - if ( numItems <= remainingPosts.length ) { - currentPosts = remainingPosts.slice( 0, numItems ) - } else { - const needed = numItems - const postsToUse = [ - ...remainingPosts, - ...Array.from( - { length: needed - remainingPosts.length }, - ( _, i ) => postsPlaceholder[ i % postsPlaceholder.length ] // reuse placeholders if numberOfItems > 6 - ), - ] - currentPosts = postsToUse.slice( 0, numItems ) - } + const currentPosts = Array.from( + { length: numItems }, + ( _, i ) => placeholders[ ( postCursor + i ) % placeholders.length ] + ) + postCursor += numItemsAlso applies to: 274-287
241-241: Document the new img override parameter and update call sitesAdd a brief JSDoc describing img (expected type: string URL) and whether it accepts absolute or relative paths. Verify all invocations pass the new 4th arg intentionally.
Example JSDoc:
/** * @param {string} content * @param {Array} postsPlaceholder * @param {Record<string,string>} defaultValues * @param {string|null} [img=null] Optional image URL to override per-post images. */src/components/style-guide/utils.js (3)
304-304: Defensive default for getBlockVariationsWordPress can return undefined; default to [].
- const variations = getBlockVariations( blockName ) + const variations = getBlockVariations( blockName ) || []
171-181: Improve a11y for preview anchor (use button semantics)Make it keyboard-accessible without relying on href.
- <a className="stk-link stk-button stk--hover-effect-darken" href="" onClick={ e => e.preventDefault() }> + <a + className="stk-link stk-button stk--hover-effect-darken" + href="" + role="button" + tabIndex={ 0 } + aria-label={ text } + onClick={ e => e.preventDefault() } + onKeyDown={ e => ( e.key === 'Enter' || e.key === ' ' ) && e.preventDefault() } + >
185-199: Mirror a11y improvements for outline preview anchorApply the same button semantics here.
- <a className="stk-link stk-button stk--hover-effect-darken" href="" onClick={ e => e.preventDefault() }> + <a + className="stk-link stk-button stk--hover-effect-darken" + href="" + role="button" + tabIndex={ 0 } + aria-label={ text } + onClick={ e => e.preventDefault() } + onKeyDown={ e => ( e.key === 'Enter' || e.key === ' ' ) && e.preventDefault() } + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
plugin.php(1 hunks)src/components/design-library-list/use-preview-renderer.js(2 hunks)src/components/design-library-list/util.js(4 hunks)src/components/modal-design-library/modal.js(8 hunks)src/components/style-guide/components.js(1 hunks)src/components/style-guide/editor.scss(1 hunks)src/components/style-guide/utils.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/style-guide/components.js
- src/components/style-guide/editor.scss
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/modal-design-library/modal.js (2)
src/components/guided-modal-tour/index.js (1)
GuidedModalTour(34-95)src/components/color-scheme-preview/index.js (1)
ColorSchemePreview(32-83)
src/components/design-library-list/util.js (3)
src/components/design-library-list/use-preview-renderer.js (1)
DEFAULT_CONTENT(35-35)src/components/style-guide/utils.js (1)
defaultValues(274-274)src/block/posts/util.js (1)
separator(356-356)
src/components/style-guide/utils.js (5)
src/components/style-guide/index.js (2)
DUMMY_COLOR_SCHEMES(25-34)props(23-23)src/components/design-library-list/util.js (6)
blocks(235-235)defaultValues(184-184)DEFAULT_CONTENT(12-12)DEFAULT_CONTENT(12-12)addPlaceholderForPostsBlock(241-311)addPlaceholderForPostsBlock(241-311)src/util/blocks.js (4)
block(324-324)block(467-467)cleanSerializedBlock(613-621)cleanSerializedBlock(613-621)src/util/styles/block-style-generators.js (3)
blockStyleGenerators(2-2)blockStyleGenerators(18-18)blockStyleGenerators(18-18)src/util/block-templates.js (2)
PLACEHOLDER_INNER_BLOCKS(149-161)PLACEHOLDER_INNER_BLOCKS(149-161)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
🔇 Additional comments (6)
src/components/design-library-list/use-preview-renderer.js (1)
173-178: LGTM: centralized comment stripping via cleanSerializedBlockReplacing the inline regex with
cleanSerializedBlock(preview)keeps behavior while consolidating logic into a shared utility. Looks good.src/components/modal-design-library/modal.js (3)
216-218: Guided tour injection: LGTMPlacement inside the modal wrapper is appropriate and non-intrusive.
253-253: Background Scheme label styling hook: LGTMConsistent BEM-style classname addition; no functional impact.
9-9: GuidedModalTour export verified Default export is re-exported as a named export (export { default as GuidedModalTour }) in src/components/index.js, soimport { GuidedModalTour } from '~stackable/components'will succeed.src/components/design-library-list/util.js (1)
10-10: ```shell
#!/bin/bash
echo "Searching for '~stackable' alias usages"
rg -n "~stackable" -C2 || trueecho "Listing possible config files"
find . -maxdepth 2 -type f ( -name 'tsconfig*.json' -o -name 'jsconfig.json' -o -name '.babelrc' -o -name 'babel.config.js' -o -name 'next.config.js' )for f in tsconfig.json jsconfig.json babel.config.js .babelrc next.config.js; do
if [ -f "$f" ]; then
echo "==== $f ===="
sed -n '1,200p' "$f"
fi
done</blockquote></details> <details> <summary>src/components/style-guide/utils.js (1)</summary><blockquote> `24-57`: **LGTM: sensible defaults for DUMMY_COLOR_SCHEMES** Good, self-contained tokens for previews. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @@ -311,6 +311,7 @@ function is_frontend() { | |||
| /** | |||
| * Welcome screen. | |||
| */ | |||
| require_once( plugin_dir_path( __FILE__ ) . 'src/welcome/getting-started.php' ); | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm REST permissions and data scope for guided tour state.
If the included file registers REST routes or options, ensure:
- permission_callback restricts access appropriately.
- Tour “state” is per-user (user_meta) or intentionally site-wide; if site-wide, confirm capability (e.g., manage_options) and sanitization.
Run to verify routes, permissions, and storage:
🏁 Script executed:
#!/bin/bash
# Inspect Getting Started server bits
rg -n -C2 -H --type=php 'register_rest_route|rest_api_init|permission_callback|register_setting|stackable_guided_tour_states|add_localize_script' src/welcome/getting-started.php
# Check if tour state is used elsewhere and whether user_meta is involved
rg -n -C2 'stackable_guided_tour_states|update_user_meta|get_user_meta' srcLength of output: 3446
Expose and sanitize guided tour state via REST or migrate to per-user meta
- In src/welcome/getting-started.php, update register_setting('stackable_guided_tour_states', …) to include
'show_in_rest' => trueand asanitize_callbackfor the array, so the built-in /wp/v2/settings endpoint recognizes and safely processes it. - Tour state is currently site-wide (via get_option('stackable_guided_tour_states')). Confirm intended scope. If it should be per-user, migrate storage to user_meta (using update_user_meta/get_user_meta) and expose it via a custom REST field or route with an appropriate permission_callback.
🤖 Prompt for AI Agents
In plugin.php around line 314 (src/welcome/getting-started.php inclusion) update
the registration and storage of the guided tour state: if the guided tour state
is intended to remain site-wide, modify the register_setting call for
'stackable_guided_tour_states' to add 'show_in_rest' => true and provide a
sanitize_callback that validates the value is an array, ensures expected keys
and value types (e.g., booleans/strings), and strips unexpected entries before
returning safe data; if the state is intended per-user, migrate from
get_option/update_option to update_user_meta/get_user_meta for the current user,
remove the option registration, and expose the per-user state via a custom REST
field or route (use register_rest_field or register_rest_route) with a
permission_callback that checks current_user_can('edit_user', $user_id) or
similar and sanitize the input/output with the same array sanitizer.
| @@ -260,6 +261,7 @@ export const addPlaceholderForPostsBlock = ( content, postsPlaceholder, defaultV | |||
|
|
|||
| const numItems = attrs.numberOfItems ?? 6 | |||
| const width = attrs.imageWidth ? attrs.imageWidth + ( attrs.imageWidthUnit ?? 'px' ) : 'auto' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid width="auto", keep attribute numeric-only, and sanitize imgSrc injection
- width attribute should be numeric (pixels) or omitted; "auto" is invalid. Keep width in CSS and include the HTML width attribute only for px units.
- img (override) is interpolated unsanitized into an HTML attribute. Escape quotes at minimum to avoid attribute breaking.
Apply:
-const width = attrs.imageWidth ? attrs.imageWidth + ( attrs.imageWidthUnit ?? 'px' ) : 'auto'
+const widthVal = Number.isFinite( Number( attrs.imageWidth ) )
+ ? `${ attrs.imageWidth }${ attrs.imageWidthUnit ?? 'px' }`
+ : null
@@
-const renderedPosts = currentPosts.map( ( post, index ) => {
- const imgSrc = img ?? `${ IMAGE_STORAGE }stk-design-library-image-${ index + 1 }.jpeg`
+const renderedPosts = currentPosts.map( ( post, index ) => {
+ const baseSrc = img ?? `${ IMAGE_STORAGE }stk-design-library-image-${ index + 1 }.jpeg`
+ const safeImgSrc = String( baseSrc ).replace( /"/g, '"' )
+ const widthAttr = ( Number.isFinite( Number( attrs.imageWidth ) ) && ( ! attrs.imageWidthUnit || attrs.imageWidthUnit === 'px' ) )
+ ? ` width="${ parseInt( attrs.imageWidth, 10 ) }"`
+ : ''
+ const styleAttr = widthVal ? ` style="width: ${ widthVal } !important;"` : ''
return template
@@
- .replace( /img class="stk-img"/g, `img class="stk-img" src="${ imgSrc }" width="${ width }" style="width: ${ width } !important;"` )
+ .replace( /img class="stk-img"/g, `img class="stk-img" src="${ safeImgSrc }"${ widthAttr }${ styleAttr }` )Optional hardening: restrict overrides to the CDN origin if that’s the only intended source:
- Reject baseSrc unless it starts with IMAGE_STORAGE.
Also applies to: 289-302
| const INNER_BLOCK_CALLBACKS = { | ||
| 'stackable/team-member': innerBlocks => { | ||
| innerBlocks[ 0 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }` | ||
|
|
||
| return innerBlocks | ||
| }, | ||
| 'stackable/testimonial': innerBlocks => { | ||
| innerBlocks[ 1 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }` | ||
|
|
||
| return innerBlocks | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden INNER_BLOCK_CALLBACKS against missing children
Indexing blindly can throw when variations/templates change.
'stackable/team-member': innerBlocks => {
- innerBlocks[ 0 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }`
+ if ( innerBlocks?.[ 0 ]?.attributes ) {
+ innerBlocks[ 0 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }`
+ }
return innerBlocks
},
'stackable/testimonial': innerBlocks => {
- innerBlocks[ 1 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }`
+ if ( innerBlocks?.[ 1 ]?.attributes ) {
+ innerBlocks[ 1 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }`
+ }
return innerBlocks
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const INNER_BLOCK_CALLBACKS = { | |
| 'stackable/team-member': innerBlocks => { | |
| innerBlocks[ 0 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }` | |
| return innerBlocks | |
| }, | |
| 'stackable/testimonial': innerBlocks => { | |
| innerBlocks[ 1 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }` | |
| return innerBlocks | |
| }, | |
| const INNER_BLOCK_CALLBACKS = { | |
| 'stackable/team-member': innerBlocks => { | |
| if ( innerBlocks?.[ 0 ]?.attributes ) { | |
| innerBlocks[ 0 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }` | |
| } | |
| return innerBlocks | |
| }, | |
| 'stackable/testimonial': innerBlocks => { | |
| if ( innerBlocks?.[ 1 ]?.attributes ) { | |
| innerBlocks[ 1 ].attributes.imageExternalUrl = `${ srcUrl }/${ profile }` | |
| } | |
| return innerBlocks | |
| }, | |
| // …other callbacks… | |
| } |
🤖 Prompt for AI Agents
In src/components/style-guide/utils.js around lines 158 to 168, the
INNER_BLOCK_CALLBACKS handlers assume innerBlocks and specific indexed children
always exist which can throw if templates change; update each callback to first
verify innerBlocks is an array and has the target index, create or normalize
innerBlocks[index].attributes if missing, then set imageExternalUrl safely and
return innerBlocks; if the target child is missing, simply return innerBlocks
unchanged to avoid exceptions.
| const getGeneratedCss = ( blocks, generateForInnerBlocks = false ) => { | ||
| return blocks.map( block => { | ||
| if ( ! block.attributes.uniqueId ) { | ||
| block.attributes.uniqueId = createUniqueClass( block.clientId ) | ||
| } |
There was a problem hiding this comment.
Prevent crash when blocks is undefined in getGeneratedCss
Harden against undefined/null input.
-const getGeneratedCss = ( blocks, generateForInnerBlocks = false ) => {
- return blocks.map( block => {
+const getGeneratedCss = ( blocks, generateForInnerBlocks = false ) => {
+ const list = Array.isArray( blocks ) ? blocks : []
+ return list.map( block => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getGeneratedCss = ( blocks, generateForInnerBlocks = false ) => { | |
| return blocks.map( block => { | |
| if ( ! block.attributes.uniqueId ) { | |
| block.attributes.uniqueId = createUniqueClass( block.clientId ) | |
| } | |
| const getGeneratedCss = ( blocks, generateForInnerBlocks = false ) => { | |
| const list = Array.isArray( blocks ) ? blocks : [] | |
| return list.map( block => { | |
| if ( ! block.attributes.uniqueId ) { | |
| block.attributes.uniqueId = createUniqueClass( block.clientId ) | |
| } | |
| // …rest of the mapping logic… | |
| } ) | |
| } |
🤖 Prompt for AI Agents
In src/components/style-guide/utils.js around lines 201 to 205, getGeneratedCss
assumes blocks is an array and will throw if blocks is undefined/null; guard the
start of the function with a check like if (!blocks || !Array.isArray(blocks))
return '' (or the appropriate empty value the function normally returns) so the
rest of the logic can safely call map; keep generateForInnerBlocks behavior
unchanged.
| if ( generateForInnerBlocks ) { | ||
| block.innerBlocks = getGeneratedCss( block.innerBlocks, generateForInnerBlocks ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Recursion should handle missing innerBlocks
Avoid passing undefined into getGeneratedCss.
- if ( generateForInnerBlocks ) {
- block.innerBlocks = getGeneratedCss( block.innerBlocks, generateForInnerBlocks )
- }
+ if ( generateForInnerBlocks && Array.isArray( block.innerBlocks ) ) {
+ block.innerBlocks = getGeneratedCss( block.innerBlocks, generateForInnerBlocks )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( generateForInnerBlocks ) { | |
| block.innerBlocks = getGeneratedCss( block.innerBlocks, generateForInnerBlocks ) | |
| } | |
| if ( generateForInnerBlocks && Array.isArray( block.innerBlocks ) ) { | |
| block.innerBlocks = getGeneratedCss( block.innerBlocks, generateForInnerBlocks ) | |
| } |
🤖 Prompt for AI Agents
In src/components/style-guide/utils.js around lines 223 to 225, the code calls
getGeneratedCss(block.innerBlocks, generateForInnerBlocks) without guaranteeing
innerBlocks exists; update the call to guard against undefined by only calling
getGeneratedCss when innerBlocks is an array or by passing an empty array
fallback (e.g., block.innerBlocks ? getGeneratedCss(block.innerBlocks,
generateForInnerBlocks) : []) so that undefined is never passed into
getGeneratedCss and innerBlocks remains predictable.
…gambitph/Stackable into feat/getting-started-onboarding
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/components/guided-modal-tour/tour-steps.js (2)
193-201: Fix typo in user-facing copy and use numeric offset.Typo: “hundreds ofpre-built” → “hundreds of pre-built”. Set offsetX as a number for consistency with docs/consumer coercion.
- description: __( 'These are hundreds ofpre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ), + description: __( 'These are hundreds of pre-built designs that are style-matched to your block theme. You can insert one or more patterns to quickly build your page.', i18n ), @@ - offsetX: '-400px', + offsetX: -400,
212-218: Null-check before accessing .checked to avoid NPE.Guard el before reading .checked and clicking.
- postStep: () => { - const el = document.querySelector( '.ugb-modal-design-library__enable-background input' ) - // If the input is not checked, click the button. - if ( ! el.checked ) { - el.click() - } - }, + postStep: () => { + const el = document.querySelector( '.ugb-modal-design-library__enable-background input' ) + // If the input is not checked, click the button. + if ( el && ! el.checked ) { + el.click() + } + },src/components/guided-modal-tour/index.js (3)
288-301: Create/attach glow element in ownerDocument, not window.document.Prevents issues when rendered inside iframes.
useEffect( () => { - // Create the element. - const element = document.createElement( 'div' ) + const doc = modalRef.current?.ownerDocument || document + // Create the element. + const element = doc.createElement( 'div' ) element.className = `ugb-tour-modal__glow ugb-tour-modal__glow--hidden` - document.body.appendChild( element ) + doc.body.appendChild( element )
303-363: Compute geometry using ownerDocument/defaultView; avoid ref in deps.Fixes mispositioning in iframes and removes unstable ref from deps. Also query the modal node directly (no need for .querySelector('.ugb-tour-modal')).
const [ modalOffsetX, modalOffsetY ] = useMemo( () => { if ( ! modalRef.current ) { return [ '', '' ] // This is for the entire screen. } - const modalRect = modalRef.current.querySelector( '.ugb-tour-modal' ).getBoundingClientRect() - const defaultOffset = [ `${ ( window.innerWidth / 2 ) - ( modalRect.width / 2 ) }px`, `${ ( window.innerHeight / 2 ) - ( modalRect.height / 2 ) }px` ] + const doc = modalRef.current?.ownerDocument || document + const win = doc.defaultView || window + const modalRect = modalRef.current.getBoundingClientRect() + const defaultOffset = [ `${ ( win.innerWidth / 2 ) - ( modalRect.width / 2 ) }px`, `${ ( win.innerHeight / 2 ) - ( modalRect.height / 2 ) }px` ] if ( ! anchor ) { return defaultOffset // This is for the entire screen. } // Based on the anchor and position, calculate the X and Y offsets of the modal relative to the anchor. // We have the modalRef.current which we can use to get the modal's bounding client rect. - const anchorRect = document.querySelector( anchor )?.getBoundingClientRect() + const anchorRect = doc.querySelector( anchor )?.getBoundingClientRect() @@ - }, [ anchor, position, modalRef.current, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] ) + }, [ anchor, position, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] )
366-389: Use ownerDocument for glow target; fix medium-size threshold.Use the same ownerDocument as the modal and correct the duplicated “> 300” check so “medium” triggers sooner.
useEffect( () => { if ( glowTarget && isVisibleDelayed ) { // Get the top, left, width, and height of the target. - const target = document.querySelector( glowTarget ) + const doc = modalRef.current?.ownerDocument || document + const target = doc.querySelector( glowTarget ) if ( target ) { const targetRect = target.getBoundingClientRect() // Estimate the size of the glow target based on the size of the target. - const glowTargetSize = targetRect.width > 300 || targetRect.height > 200 ? 'large' - : targetRect.width > 300 || targetRect.height > 100 ? 'medium' + const glowTargetSize = targetRect.width > 300 || targetRect.height > 200 ? 'large' + : targetRect.width > 200 || targetRect.height > 100 ? 'medium' : 'small'
🧹 Nitpick comments (5)
src/components/guided-modal-tour/tour-steps.js (1)
19-24: Align docs with implementation: offset types and allowed positions.Docs say offsetX/offsetY are numbers, but index.js treats them as CSS lengths; also positions used include left-top/left-bottom/etc. Update the contract to accept numbers or CSS length strings and enumerate all supported positions.
- * - position (string): The position of the modal relative to the anchor. Can be 'left', 'right', 'top', 'bottom', or 'center'. Default is 'center'. - * - offsetX (number): X-axis offset in pixels for fine-tuning the modal's position relative to the anchor. - * - offsetY (number): Y-axis offset in pixels for fine-tuning the modal's position relative to the anchor. + * - position (string): The position of the modal relative to the anchor. Supported: 'left', 'right', 'top', 'bottom', 'center', 'left-top', 'left-bottom', 'right-top', 'right-bottom', 'top-left', 'top-right', 'bottom-left', 'bottom-right', 'center-top', 'center-bottom'. Default is 'center'. + * - offsetX (number|string): X-axis offset for fine-tuning. If a number is provided, it's treated as px; CSS length strings (e.g., '1rem', '12px') are also accepted. + * - offsetY (number|string): Y-axis offset for fine-tuning. Same rules as offsetX.src/components/guided-modal-tour/index.js (4)
423-430: Support numeric offsets by coercing to px in the injected CSS.Keeps backward-compat with CSS strings while aligning with the documented number type.
- { `.ugb-tour-modal { - --offset-x: ${ offsetX }; - --offset-y: ${ offsetY }; + { `.ugb-tour-modal { + --offset-x: ${ typeof offsetX === 'number' ? `${ offsetX }px` : offsetX }; + --offset-y: ${ typeof offsetY === 'number' ? `${ offsetY }px` : offsetY }; --left: ${ modalOffsetX }; --top: ${ modalOffsetY }; }` }
246-286: Use ownerDocument for non-click event bindings too.The non-click branch still queries global document; breaks in iframes.
useEffect( () => { let clickListener = null if ( nextEventTarget ) { + const doc = modalRef.current?.ownerDocument || document if ( nextEvent === 'click' ) { clickListener = event => { @@ - // Use ownerDocument instead of document directly - const doc = modalRef.current?.ownerDocument || document doc.addEventListener( 'click', clickListener ) } else { - const elements = document.querySelectorAll( nextEventTarget ) + const elements = doc.querySelectorAll( nextEventTarget ) for ( let i = 0; i < elements.length; i++ ) { elements[ i ].addEventListener( nextEvent, handleNextEvent ) } } } return () => { if ( nextEventTarget ) { if ( nextEvent === 'click' && clickListener ) { - // Use ownerDocument instead of document directly - const doc = modalRef.current?.ownerDocument || document doc.removeEventListener( 'click', clickListener ) } else { - const elements = document.querySelectorAll( nextEventTarget ) + const elements = doc.querySelectorAll( nextEventTarget ) for ( let i = 0; i < elements.length; i++ ) { elements[ i ].removeEventListener( nextEvent, handleNextEvent ) } } } } }, [ currentStep, nextEventTarget, nextEvent, handleNextEvent ] )
135-145: Replace polling with observers for positioning.Polling every 500ms is wasteful. Prefer ResizeObserver on the modal/anchor and a MutationObserver on layout containers to trigger setForceRefresh on actual changes.
I can wire a lightweight observer-based hook for this; want me to push a patch?
487-496: Confetti origin.y is negative; verify expected range is [0,1].canvas-confetti expects origin coords in 0..1. A negative y may be clamped or ignored.
- origin: { y: -0.3 }, + origin: { y: 0.3 },If negative is intentional and supported, ignore this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/guided-modal-tour/index.js(1 hunks)src/components/guided-modal-tour/tour-steps.js(1 hunks)src/components/style-guide/components.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/style-guide/components.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/guided-modal-tour/tour-steps.js (1)
src/components/guided-modal-tour/index.js (1)
TOUR_STEPS(45-49)
src/components/guided-modal-tour/index.js (1)
src/components/guided-modal-tour/tour-steps.js (2)
TOUR_STEPS(49-279)TOUR_STEPS(49-279)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: build
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
| const { | ||
| steps = [], | ||
| condition = null, | ||
| hasConfetti = true, | ||
| } = TOUR_STEPS[ tourId ] | ||
|
|
There was a problem hiding this comment.
Guard against unknown tourId to avoid a crash when destructuring.
TOUR_STEPS[tourId] can be undefined; destructuring it throws.
- const {
- steps = [],
- condition = null,
- hasConfetti = true,
- } = TOUR_STEPS[ tourId ]
+ const tour = TOUR_STEPS?.[ tourId ] || {}
+ const {
+ steps = [],
+ condition = null,
+ hasConfetti = true,
+ } = tour📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { | |
| steps = [], | |
| condition = null, | |
| hasConfetti = true, | |
| } = TOUR_STEPS[ tourId ] | |
| const tour = TOUR_STEPS?.[ tourId ] || {} | |
| const { | |
| steps = [], | |
| condition = null, | |
| hasConfetti = true, | |
| } = tour |
🤖 Prompt for AI Agents
In src/components/guided-modal-tour/index.js around lines 45-50,
TOUR_STEPS[tourId] may be undefined and destructuring it crashes; guard by first
resolving a safe object (e.g., const tour = TOUR_STEPS[tourId] || {}) or check
for existence and return/handle the missing tour before destructuring, then
destructure from that safe object to supply defaults for steps, condition, and
hasConfetti; optionally log a warning or return null when tourId is invalid.
Summary by CodeRabbit
New Features
Enhancements
Chores