Feat: More Guided Tours #3639
Feat: More Guided Tours #3639
Conversation
WalkthroughThis PR extends the guided modal tour system by adding five new tour condition keys and corresponding tour modules (block-backgrounds, responsive-controls, hover-states, advanced-hover-states, global-color-schemes). It refactors getting-started to use data-driven configuration, enhances tour positioning with iframe support, and introduces utility functions for DOM synchronization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GettingStarted as Getting Started Component
participant GuidedModalTour as Guided Modal Tour
participant TourConditions as Tour Conditions
participant TourFilter as WordPress Filter
participant TourModule as Tour Module
User->>GettingStarted: Click quick-start button (e.g., "Block Backgrounds")
GettingStarted->>GuidedModalTour: Render with tourId from QUICK_BUTTONS
GuidedModalTour->>TourConditions: Check TOUR_CONDITIONS[tourId]
TourConditions->>TourConditions: Check URL for tour parameter
GuidedModalTour->>TourFilter: applyFilters('stackable.guided-tour.conditions', TOUR_CONDITIONS)
TourFilter->>GuidedModalTour: Return filtered conditions
GuidedModalTour->>TourModule: Load tour module (e.g., blockBackgrounds)
TourModule->>TourModule: initialize() - insert blocks, setup editor state
TourModule->>TourModule: steps[0] - preStep hook
TourModule->>User: Display first tour step with anchor, glow, modal
User->>GuidedModalTour: Click Next
GuidedModalTour->>TourModule: Execute postStep hook for current step
TourModule->>TourModule: Update editor state (select blocks, open panels)
GuidedModalTour->>TourModule: Execute preStep hook for next step
TourModule->>User: Display next step
sequenceDiagram
participant Modal as Modal Tour Component
participant Step as Step Config
participant IFrame as Editor Canvas iframe
participant MainDoc as Main Document
participant CalcPos as Position Calculator
Modal->>Step: Load step with anchorInIframe=true
Step->>Modal: Provide anchor selector & iframe flag
Modal->>IFrame: querySelector(anchorSelector) in iframe.contentDocument
IFrame->>CalcPos: Return element from iframe
CalcPos->>CalcPos: offsetCoord(element, isInIframe=true)
CalcPos->>CalcPos: Add iframe.getBoundingClientRect() offset
CalcPos->>MainDoc: Compute position in viewport coordinates
MainDoc->>Modal: Return adjusted modal position
Modal->>Modal: Render glow element with iframe-adjusted coordinates
Modal->>User: Display modal at correct position overlaying iframe content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/welcome/getting-started.js (1)
200-203: Consider making the "Quick Start" label more explicit.The condition
i === 0assumes the first item should show the label. If the array order changes or items are filtered in the future, this could break.Consider adding a
showLabelproperty to QUICK_BUTTONS entries:const QUICK_BUTTONS = [ { id: 'design-library', + showLabel: true, icon: addTemplateIcon,Then use:
-{ i === 0 && <div className="s-quick-buttons-arrow"> +{ item.showLabel && <div className="s-quick-buttons-arrow">src/lazy-components/modal-tour/tours/block-backgrounds.js (1)
83-97: Improve resilience of DOM manipulation in tour steps.The direct
querySelectorcalls and DOM clicks are fragile and may fail if:
- Selectors change due to WordPress/plugin updates
- Elements are not yet rendered
- Class names are dynamically generated differently
Consider:
- Adding error handling and logging when elements aren't found
- Using data attributes instead of complex class selectors for tour targets
- Wrapping DOM operations in try-catch blocks to prevent tour crashes
Example:
preStep: () => { try { dispatch('core/edit-post').openGeneralSidebar('edit-post/block') const allBlocks = select('core/block-editor').getBlocks() const columnsBlock = allBlocks.find(block => block.name === 'stackable/columns') if (columnsBlock) { dispatch('core/block-editor').selectBlock(columnsBlock.clientId) } else { console.warn('Tour: columns block not found') } } catch (error) { console.error('Tour step failed:', error) } },Also applies to: 110-125, 138-146, 175-188, 193-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/guided-modal-tour/tour-conditions.js(1 hunks)src/lazy-components/modal-tour/tours/block-backgrounds.js(1 hunks)src/plugins/guided-modal-tour/editor-block-tour.js(1 hunks)src/welcome/getting-started.js(1 hunks)src/welcome/getting-started.scss(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/plugins/guided-modal-tour/editor-block-tour.js (1)
src/components/guided-modal-tour/index.js (1)
GuidedModalTour(27-104)
⏰ 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 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: build
🔇 Additional comments (7)
src/components/guided-modal-tour/tour-conditions.js (1)
30-33: LGTM! Consistent tour condition implementation.The new
block-backgroundstour condition follows the established pattern and correctly checks for the URL parameter to activate the tour.src/welcome/getting-started.scss (2)
197-200: Good refinement of the SVG sizing rule.Excluding
.s-quick-buttons-arrowfrom the generic SVG sizing prevents unintended styling conflicts with the decorative arrow element.
239-268: Layout refactor supports the new data-driven cards.The grid gap adjustment, video positioning, and flex-direction changes align well with the QUICK_BUTTONS rendering strategy in
getting-started.js.src/welcome/getting-started.js (1)
140-156: Verify the purpose of hidden quick-start items.Two entries (
site-kitanddesign-system-picker) havestyle: { display: 'none' }. Are these feature-flagged for future release, or should they be removed from the configuration?Consider one of these approaches:
- If they're coming soon, add a comment explaining they're feature-flagged
- If they're experimental, move them to a separate
EXPERIMENTAL_BUTTONSarray- If they're not needed, remove them to reduce maintenance overhead
src/plugins/guided-modal-tour/editor-block-tour.js (1)
5-10: LGTM! Clean integration of the new tour.The dual-tour rendering is handled correctly. The
GuidedModalTourcomponent's internal logic ensures only one tour shows at a time based on URL parameters and conditions.src/lazy-components/modal-tour/tours/block-backgrounds.js (2)
7-71: Clean block structure initialization.The nested block creation logic is well-structured and creates appropriate default content for the tour demonstration.
72-210: Well-crafted tour flow and copy.The step-by-step progression is logical, and the i18n-ready help text provides clear guidance for users learning about backgrounds and containers.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/lazy-components/modal-tour/tours/responsive-controls.js (2)
65-67: Remove commented-out code.Dead code should be removed to improve maintainability.
- // Delete all blocks - // const allBlocks = select( 'core/block-editor' ).getBlocks() - // dispatch( 'core/block-editor' ).removeBlocks( allBlocks.map( block => block.clientId ) ) -
117-122: Code duplication in preStep and postStep.The
postStepat line 121 duplicates similar logic from thepreStepat line 118, and this pattern repeats in subsequent steps (e.g., line 134). This duplication makes the code harder to maintain.Consider extracting common DOM manipulation patterns into helper functions:
// At the top of the file const helpers = { openResponsiveToggle: () => { document.querySelector( '.ugb-sort-control .stk-control-responsive-toggle:not(.is-open) button' )?.click() }, selectDevice: ( device ) => { document.querySelector( `.ugb-sort-control .stk-control-responsive-toggle button:not(.is-active)[data-value="${device}"]` )?.click() }, } // Then in steps: preStep: () => { helpers.openResponsiveToggle() }, postStep: () => { helpers.selectDevice( 'mobile' ) },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/guided-modal-tour/tour-conditions.js(1 hunks)src/lazy-components/modal-tour/tours/responsive-controls.js(1 hunks)src/plugins/guided-modal-tour/editor-block-tour.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/guided-modal-tour/tour-conditions.js
- src/plugins/guided-modal-tour/editor-block-tour.js
⏰ 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 6.6.2
- GitHub Check: build
- GitHub Check: PHP 7.3 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 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (2)
src/lazy-components/modal-tour/tours/responsive-controls.js (2)
75-199: Well-structured tour flow with good UX guidance.The tour steps provide clear, progressive guidance through the responsive controls feature with helpful descriptions using interpolated elements for emphasis. The flow logically guides users through enabling, using, and understanding responsive controls.
4-4: No issues found with @wordpress/element dependency.The codebase widely uses
@wordpress/elementacross 100+ files with no issues. The import ofcreateInterpolateElementfrom@wordpress/elementin the responsive-controls.js file is consistent with this established pattern throughout the codebase, and the package is properly managed in the project's dependency lock file. The original concern is not applicable here.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/lazy-components/modal-tour/tours/responsive-controls.js (3)
148-155: Call the importedselect/dispatchhelpers directly.We already import
selectanddispatchat the top, but this block still reaches forwp.data.*. Sticking to the imports keeps the bundle self-contained and avoids depending on the globalwpnamespace (which may not exist in automated contexts).- const allBlocks = wp.data.select( 'core/block-editor' ).getBlocks() + const allBlocks = select( 'core/block-editor' ).getBlocks() @@ - wp.data.dispatch( 'core/block-editor' ).updateBlockAttributes( columnsBlock.clientId, { + dispatch( 'core/block-editor' ).updateBlockAttributes( columnsBlock.clientId, {
10-60: Generate blockuniqueIds dynamically.All six blocks are still created with fixed IDs (
'1dbe04e','f957abc', …'e063798'). If a user runs this tour more than once—or runs it after the hover-states tour that reuses'e063798'—Gutenberg encounters duplicateuniqueIdattributes and throws invalid-block warnings. This was raised earlier and remains unresolved; we need runtime-generated IDs to keep editor state consistent.One approach:
initialize: () => { - // Add some default content that we will select - - const blockObject = wp.blocks.createBlock( + const makeUniqueId = ( prefix = 'tour-' ) => `${ prefix }${ Math.random().toString(36).slice(2, 11) }` + const blockObject = wp.blocks.createBlock( 'stackable/columns', { - uniqueId: '1dbe04e', + uniqueId: makeUniqueId( 'columns-' ), @@ wp.blocks.createBlock( 'stackable/column', { - uniqueId: 'f957abc', + uniqueId: makeUniqueId( 'column-' ), @@ wp.blocks.createBlock( 'stackable/heading', { - uniqueId: 'a8ebea7', + uniqueId: makeUniqueId( 'heading-' ), @@ wp.blocks.createBlock( 'stackable/text', { - uniqueId: '57e76a1', + uniqueId: makeUniqueId( 'text-' ), @@ wp.blocks.createBlock( 'stackable/column', { - uniqueId: '3dcffca', + uniqueId: makeUniqueId( 'column-' ), @@ wp.blocks.createBlock( 'stackable/image', { - uniqueId: 'e063798', + uniqueId: makeUniqueId( 'image-' ),
86-178: Eliminate hard-coded timeouts from tour steps.The tour flow is stitched together with
setTimeout(100/200)(Lines 94-100, 118-121, 175-177). Those arbitrary sleeps are fragile—on slower editors the selectors aren’t ready yet (steps stall), while on faster environments they just add lag. Please replace them with deterministic waits (e.g., an asyncwaitForElementhelper or MutationObserver) so each click happens only after the UI element exists and is in the expected state.Here’s a sketch of how you can restructure the first occurrence; the same helper can be reused for the other steps:
- setTimeout( () => { - // Open Layout tab if it's not open - document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--layout:not(.is-active)' )?.click() - }, 100 ) - setTimeout( () => { - document.querySelector( '.ugb-panel--layout:not(.is-opened)' )?.click() - }, 200 ) + const waitForElement = selector => new Promise( resolve => { + const tick = () => { + const node = document.querySelector( selector ) + return node ? resolve( node ) : requestAnimationFrame( tick ) + } + tick() + } ) + + waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--layout' ) + .then( tab => { + if ( ! tab.classList.contains( 'is-active' ) ) { + tab.click() + } + return waitForElement( '.ugb-panel--layout' ) + } ) + .then( panel => { + if ( ! panel.classList.contains( 'is-opened' ) ) { + panel.click() + } + } )Apply the same waiting strategy for the mobile/desktop toggles later in the tour.
🧹 Nitpick comments (2)
src/components/base-control2/index.js (1)
112-112: LGTM! Data attribute added for guided tour targeting.The addition of the
data-attributealigns well with the PR's guided tour functionality, enabling easier targeting of specific controls.Consider using a more specific attribute name like
data-stk-attributeordata-control-attributeto improve clarity in DevTools and avoid potential naming conflicts with other libraries or user code.- <div className="stk-control-content" data-attribute={ props.attribute }> + <div className="stk-control-content" data-stk-attribute={ props.attribute }>src/lazy-components/modal-tour/tours/hover-states.js (1)
76-83: Use the imported data helpers instead ofwp.data.*.Inside this module we already depend on the bundled build. Leaning on the global
wp.datamixes patterns and can break when the globals are not exposed (e.g., automated tests). Please importselectalongsidedispatchand call those helpers directly for consistency.You can streamline it as follows:
-import { dispatch } from '@wordpress/data' +import { dispatch, select } from '@wordpress/data' @@ - const allBlocks = wp.data.select( 'core/block-editor' ).getBlocks() + const allBlocks = select( 'core/block-editor' ).getBlocks() @@ - wp.data.dispatch( 'core/block-editor' ).updateBlockAttributes( imageBlock.clientId, { + dispatch( 'core/block-editor' ).updateBlockAttributes( imageBlock.clientId, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/base-control2/index.js(1 hunks)src/components/guided-modal-tour/tour-conditions.js(1 hunks)src/lazy-components/modal-tour/tours/hover-states.js(1 hunks)src/lazy-components/modal-tour/tours/responsive-controls.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/guided-modal-tour/tour-conditions.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/lazy-components/modal-tour/tours/hover-states.js (1)
src/lazy-components/modal-tour/tours/responsive-controls.js (3)
blockObject(10-63)allBlocks(89-89)allBlocks(148-148)
src/lazy-components/modal-tour/tours/responsive-controls.js (1)
src/lazy-components/modal-tour/tours/hover-states.js (2)
blockObject(10-16)allBlocks(77-77)
⏰ 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: build
- GitHub Check: PHP 7.3 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.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/block-styles-control/index.js (1)
218-226: Inconsistent popover behavior for non-Pro users.For non-Pro users, selecting a block style (line 214) toggles the pro notice and keeps the popover open, but adding a block style (line 222) toggles the pro notice and closes the popover (line 225). Since both actions result in the same outcome for non-Pro users (showing a notice), consider maintaining consistent popover behavior.
Apply this diff to keep the popover open for non-Pro users when adding a block style:
const onAddBlockStyle = () => { if ( isPro ) { doAction( 'stackable.global-settings.global-block-styles.add-block-style', setOpenSaveModal ) + setOpenPopover( false ) } else { setOpenProNotice( value => ! value ) } - - setOpenPopover( false ) }
🧹 Nitpick comments (11)
src/components/block-styles-control/index.js (1)
238-240: Consider more specific class names for different states.The
is-openedclass is used here for the popover state and on line 321 for the modal state. Using distinct class names (e.g.,is-popover-openedandis-modal-opened) would improve clarity and reduce the risk of styling conflicts.src/welcome/getting-started.scss (4)
236-268: Grid/card layout works but&:nth-child(2)is brittleThe new grid layout for
.s-getting-started__quick-start-wrapperand the row/column placement of.s-getting-started-videolook consistent with the JSX, and using&:nth-child(2)to allow overflow for the first quick-start card matches the current DOM (video is child 1, first quick-start card is child 2).However, this hard-couples styling to the exact child order; any future change (e.g., inserting another element before the first quick-start card) will silently break this behavior. Consider a more robust selector, such as targeting the first non-video card:
.s-getting-started__quick-start-wrapper { > .s-card:not(.s-getting-started-video):first-of-type { overflow: visible; } }
277-297: nth-child(5n + …) patterns may unintentionally include the video cardThe color rules:
.s-card:nth-child(5n + 3) { … } .s-card:nth-child(5n + 4) { … } .s-card:nth-child(5n) { … } .s-card:nth-child(5n + 6) { … }are evaluated against all
.s-cardsiblings, including the.s-getting-started-videocard. Currently that card doesn’t contain.s-quick-button-icon, so there’s effectively no impact, but if you ever add an icon there it will pick up one of these “succeeding icon color” styles.If you intend these colors only for quick-start cards, consider scoping more tightly, e.g.:
.s-getting-started__quick-start-wrapper { > .s-card:not(.s-getting-started-video):nth-child(5n + 3) { … } // etc. }
298-305: Disabled quick-start buttons are visually disabled but still behave like links
.s-quick-button-button .s-button--disabledlowers opacity and shows a not-allowed cursor but doesn’t actually prevent clicks; the<a>still has anhref(often'') so users can trigger navigation/reload.To better align behavior with visuals:
.s-quick-button-button { @@ - .s-button--disabled, - .s-button--disabled:hover { - opacity: 0.5; - cursor: not-allowed; - } + .s-button--disabled, + .s-button--disabled:hover { + opacity: 0.5; + cursor: not-allowed; + pointer-events: none; + }(And see the JS comment about adding
aria-disabled/avoiding emptyhrefs.)
372-383: Custom icon wrapper integrates cleanly with card SVG sizing
.s-custom-icon-wrappercentralizes sizing for the hover-state SVGs (24×24 wrapper with a 16×16 SVG) and matches the surrounding quick-start icon dimensions. Once the general.s-card svgselector is corrected, the!importanton the SVG height likely becomes unnecessary, but the current setup is functionally sound.src/welcome/getting-started.js (1)
128-263: QUICK_BUTTONS config looks solid; consider URL-encoding dynamic query params and centralizing pro gatingThe QUICK_BUTTONS data structure is coherent (ids, icons, titles, descriptions, CTAs, premium flags), and the new guided-tour entries line up with the
tour=query parameter usage.Two small robustness improvements to consider:
- Encode dynamic
contentvalues for URLsLocalized strings inserted into the
content=parameter could, in theory, contain characters like&that break the query string. To be safe, wrap the dynamic part withencodeURIComponent(or a shared helper):- link: `/wp-admin/post-new.php?post_type=page&content=${ __( 'Welcome to Stackable', i18n ) }&tour=design-library`, + link: `/wp-admin/post-new.php?post_type=page&content=${ encodeURIComponent( __( 'Welcome to Stackable', i18n ) ) }&tour=design-library`,(and similarly for other entries building
content=URLs).
- Avoid repeating
isPro ? url : ''in every premium itemIt may be cleaner to store the target URL unconditionally and handle
isProonce at render time (e.g., passhref={ isPro || ! item.premium ? item.link : undefined }and mark disabled). That reduces duplication and makes premium gating easier to adjust later.src/lazy-components/modal-tour/tours/index.js (1)
1-5: Dynamic tour loading + filter hook looks solid; document single-export assumptionThe
require.contextloop plusapplyFilters( 'stackable.guided-tour.tours', tours )gives a clean, extensible way to register tours, and the default-vs-first-named export fallback is pragmatic.One thing to make explicit (in a comment or docs) is the assumption that each tour module exposes exactly one relevant tour; otherwise, “first named export” selection may surprise future contributors.
No change strictly required—just worth documenting to avoid accidental multiple exports in a single tour file.
Also applies to: 11-12, 26-35, 38-39
src/lazy-components/modal-tour/index.js (4)
77-80: ClarifymodalDelaysemantics and apply consistently for back navigationRight now:
modalDelayis destructured fromsteps[ currentStep ]and used when moving forward (Line 155).- The delay used actually comes from the step you’re leaving, not the one you’re entering.
handleBackEventstill uses a hard‑coded200ms delay (Line 188), ignoringmodalDelay.This means:
- Per‑step
modalDelaydoesn’t intuitively map to “how long before this step’s modal appears”; it controls the delay for the next step when moving forward.- Backward transitions always use 200 ms regardless of configuration.
If the intent is “each step can define how long its own modal should wait before appearing”, consider:
- Looking up the delay from the target step (next/previous) inside the
setCurrentStepcallback, and- Using the same logic in
handleBackEventinstead of the hard‑coded 200 ms.Since this changes timing behavior, please confirm the intended semantics before refactoring.
Also applies to: 148-156, 181-189
82-89: Effect now depends oncurrentStepandpreStepbut only listsinitializeThe initialization effect now also calls
preStepwhencurrentStep === 0(Lines 85‑87), but the dependency array is still[ initialize ].That’s safe for the current “run once on mount for step 0” behavior, but it will trip the React Hooks lint rule and can be surprising if
preStepor how the component is mounted ever changes.Suggested options:
- Either explicitly add
currentStepandpreStepto the dependency array and accept that this effect will rerun when those change, or- Split
preStepinto a separate effect, e.g.:useEffect( () => { const timer = setTimeout( () => { initialize() }, 50 ) return () => clearTimeout( timer ) }, [ initialize ] ) useEffect( () => { if ( currentStep === 0 && preStep ) { preStep( 0 ) } }, [ currentStep, preStep ] )This keeps responsibilities clear and satisfies hook dependency rules.
294-315: Consider addinganchorInIframeto modal offset dependenciesThe modal positioning logic correctly handles anchors inside
iframe[name="editor-canvas"]and offsets by the iframe’s rect (Lines 294‑315, 319‑347). Functionally this looks good and is null‑safe.However, the
useMemodependencies (Line 351) don’t includeanchorInIframe, even though it changes which query path and coordinate system are used:}, [ anchor, position, modalRef.current, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] )Adding
anchorInIframe(and optionallycurrentStep, since the step config drives these) would make the dependency list more accurate and avoid future surprises if you reuseModalTourin a way whereanchorInIframecan change without other dependencies changing.- }, [ anchor, position, modalRef.current, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] ) + }, [ anchor, anchorInIframe, position, modalRef.current, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] )Behavior today is unlikely to break because other state changes (visibility/refresh) already trigger recomputation, so this is mainly a maintainability/linting improvement.
Also applies to: 316-347, 351-351
355-363: Align glow effect dependencies with newglowTargetInIframebehaviorThe glow effect now correctly supports targets inside
iframe[name="editor-canvas"]and offsets the glow element by the iframe’s bounding rect (Lines 357‑362, 374‑383). The null checks look safe.Similar to the modal offsets, the effect’s dependency array (Line 393) doesn’t include
glowTargetInIframe, even though it controls whether we query in the main document or the iframe:}, [ glowTarget, currentStep, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] )For accurate hook semantics and to keep linting happy, consider:
- }, [ glowTarget, currentStep, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] ) + }, [ glowTarget, glowTargetInIframe, currentStep, isVisible, isVisibleDelayed, isTransitioning, forceRefresh ] )Functionally this is low‑risk since other dependencies already change between steps; it’s mainly about making the dependency contract explicit.
Also applies to: 374-385, 390-393
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/components/block-styles-control/index.js(5 hunks)src/components/color-palette-control/index.js(1 hunks)src/components/guided-modal-tour/editor.scss(1 hunks)src/components/guided-modal-tour/index.js(2 hunks)src/components/guided-modal-tour/tour-conditions.js(1 hunks)src/components/sortable-picker/index.js(1 hunks)src/lazy-components/modal-tour/index.js(5 hunks)src/lazy-components/modal-tour/tours/advanced-hover-states.js(1 hunks)src/lazy-components/modal-tour/tours/block-backgrounds.js(1 hunks)src/lazy-components/modal-tour/tours/global-color-schemes.js(1 hunks)src/lazy-components/modal-tour/tours/index.js(2 hunks)src/welcome/getting-started.js(3 hunks)src/welcome/getting-started.scss(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lazy-components/modal-tour/tours/block-backgrounds.js
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/guided-modal-tour/index.js (2)
src/components/color-palette-control/index.js (1)
applyFilters(119-119)src/components/guided-modal-tour/tour-conditions.js (2)
TOUR_CONDITIONS(4-50)TOUR_CONDITIONS(4-50)
src/lazy-components/modal-tour/tours/index.js (1)
src/components/color-palette-control/index.js (1)
applyFilters(119-119)
src/lazy-components/modal-tour/index.js (1)
src/plugins/global-settings/typography/get-theme-styles.js (1)
iframe(3-3)
src/lazy-components/modal-tour/tours/advanced-hover-states.js (2)
src/lazy-components/modal-tour/tours/block-backgrounds.js (7)
blockObject(10-65)allBlocks(88-88)allBlocks(115-115)allBlocks(179-179)columnsBlock(89-89)columnsBlock(116-116)columnsBlock(180-180)src/lazy-components/modal-tour/tours/global-color-schemes.js (4)
allBlocks(199-199)columnsBlock(20-70)columnsBlock(200-200)block(196-196)
src/lazy-components/modal-tour/tours/global-color-schemes.js (3)
src/lazy-components/modal-tour/tours/advanced-hover-states.js (10)
columnsBlock(130-130)columnsBlock(149-149)columnsBlock(219-219)columnsBlock(235-235)columnsBlock(277-277)allBlocks(129-129)allBlocks(148-148)allBlocks(218-218)allBlocks(234-234)allBlocks(276-276)src/lazy-components/modal-tour/tours/block-backgrounds.js (6)
columnsBlock(89-89)columnsBlock(116-116)columnsBlock(180-180)allBlocks(88-88)allBlocks(115-115)allBlocks(179-179)src/components/color-palette-control/index.js (1)
select(66-69)
🔇 Additional comments (9)
src/components/color-palette-control/index.js (1)
216-216: No issues found. Pattern is correct and intentional.The addition of
data-attribute={ props.attribute }follows the established pattern inbase-control2(line 112) and is consistent across the codebase. Empty string attributes render asdata-attribute=""in the DOM, which is valid HTML and presents no functional issues. The pattern is intentional for the guided tours feature.src/components/block-styles-control/index.js (3)
270-278: Verify popover dismissal behavior aligns with UX expectations.The popover can only be dismissed via the Escape key (line 273) or by clicking the toggle button again (line 244). There are no
onFocusOutsideoronClickOutsidehandlers, which means users cannot dismiss it by clicking elsewhere or tabbing away. This may be intentional for the guided tours feature, but please confirm this matches the desired user experience.
244-244: Good change to useonClickinstead ofonMouseDown.Using
onClickis more standard and provides better user experience as it fires after the click is complete, rather than on mouse press.
303-304: LGTM! Data attributes enhance testability and tour integration.The addition of
data-slug,data-name, anddata-actionattributes provides useful hooks for automated testing and the guided tours feature introduced in this PR.Also applies to: 356-357
src/welcome/getting-started.scss (1)
308-314: Inline-flex header for description is consistent with the new icon + text layoutSwitching the first child in
.s-quick-button-descriptiontoinline-flexwith centered alignment and an 8px gap is a good fit for the pulsating premium indicator + title combination and keeps line-height tidy. No issues here.src/welcome/getting-started.js (2)
12-13: New icon and stackable imports are consistent and all used
SVGStateHoverIcon/SVGStateParentHoverIcon, the additional@wordpress/iconsexports, and the newguidedTourStates/isProimports fromstackableare all referenced later inQUICK_BUTTONSand the quick-start button rendering; there are no obvious unused imports or naming mismatches.Also applies to: 26-35, 40-42
273-279: Video intro card integration looks goodThe new
.s-getting-started-videocard, with its iframe and supporting heading/paragraph, matches the SCSS expectations (grid placement and.s-cardstyling). The use ofallowFullScreenand the standard YouTube embed URL is consistent; nothing problematic here.src/components/guided-modal-tour/editor.scss (1)
130-137: Unifying left-/right-top pointer offset looks correctSharing the
top: 30pxoverride between.ugb-tour-modal--left-topand.ugb-tour-modal--right-topkeeps the tooltip pointer consistent without affecting other positions.src/components/guided-modal-tour/index.js (1)
25-25: Filterable tour conditions look good; consider a safe fallbackMaking
TOUR_CONDITIONSfilterable viaapplyFilters( 'stackable.guided-tour.conditions', TOUR_CONDITIONS )is a nice extension point and aligns with your other hooks usage.To harden against a misbehaving filter returning a non-object, you could fall back to the original map:
- const conditions = applyFilters( 'stackable.guided-tour.conditions', TOUR_CONDITIONS ) + const filtered = applyFilters( 'stackable.guided-tour.conditions', TOUR_CONDITIONS ) + const conditions = filtered && typeof filtered === 'object' ? filtered : TOUR_CONDITIONSNot required if you fully trust filter implementers, but it avoids a hard failure when indexing
conditions[ tourId ].Also applies to: 63-65
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/welcome/getting-started.js (1)
128-263: QUICK_BUTTONS data + rendering look solid; minor “Quick Start” arrow nitThe data-driven QUICK_BUTTONS config, stable
key={ item.id }, and premium gating (hrefundefined +aria-disabled+ disabled class) all look good and align with the guided tour entries.One small optional tweak: the “Quick Start” arrow is tied to
i === 0. If you ever add a leading item withdisplay: false, the arrow will no longer point at the first visible card. You could instead compute the first visible item (e.g., map overQUICK_BUTTONS.filter( item => item.display !== false )) or track a separate render index.Also applies to: 280-316
src/lazy-components/modal-tour/tours/responsive-controls.js (1)
11-64: Avoid hard-coded uniqueId values and external image dependencyThe demo block tree uses fixed
uniqueIdstrings and an external image URL. Both can introduce fragility:
- Reusing the same
uniqueIdvalues across sessions/pages risks collisions with existing Stackable blocks or other tours.https://picsum.photos/...is an external dependency that may be slow or unavailable, which could degrade the tour.Consider:
+// Simple helper for demo-only unique IDs; or use any existing Stackable ID utility. +const generateUniqueId = () => Math.random().toString( 36 ).slice( 2, 9 ) + const blockObject = wp.blocks.createBlock( 'stackable/columns', { - uniqueId: '1dbe04e', + uniqueId: generateUniqueId(), @@ wp.blocks.createBlock( 'stackable/column', { - uniqueId: 'f957abc', + uniqueId: generateUniqueId(), @@ wp.blocks.createBlock( 'stackable/heading', { - uniqueId: 'a8ebea7', + uniqueId: generateUniqueId(), @@ wp.blocks.createBlock( 'stackable/text', { - uniqueId: '57e76a1', + uniqueId: generateUniqueId(), @@ wp.blocks.createBlock( 'stackable/column', { - uniqueId: '3dcffca', + uniqueId: generateUniqueId(), @@ wp.blocks.createBlock( 'stackable/image', { - uniqueId: 'e063798', - imageExternalUrl: 'https://picsum.photos/id/177/500/700.jpg', + uniqueId: generateUniqueId(), + // Prefer a bundled placeholder asset instead of an external URL. + imageExternalUrl: STACKABLE_PLACEHOLDER_IMAGE_URL, }(Replace
STACKABLE_PLACEHOLDER_IMAGE_URLwith your actual local/bundled placeholder.)src/lazy-components/modal-tour/tours/advanced-hover-states.js (1)
167-178: Guard sidebar scrolling to avoid runtime errorsThese two
preStepcallbacks calleditorSidebar.scrollTo(...)without checking thateditorSidebarexists or thattargetElTopis a number. If the sidebar is missing or the control isn’t found, this will throw and break the tour. You already guard this pattern later in the file; mirror that here.preStep: () => { setTimeout( () => { - const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top - const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) - editorSidebar.scrollTo( { - top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, - behavior: 'smooth', - } ) + const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top + const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) + if ( editorSidebar && typeof targetElTop === 'number' ) { + editorSidebar.scrollTo( { + top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, + behavior: 'smooth', + } ) + } document.querySelector( '.stk-control:has([data-attribute="textColor1"]) .stk-label-unit-toggle__wrapper:not(.is-open) button' )?.click() }, 100 ) }, @@ preStep: () => { setTimeout( () => { - const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top - const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) - editorSidebar.scrollTo( { - top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, - behavior: 'smooth', - } ) + const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top + const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) + if ( editorSidebar && typeof targetElTop === 'number' ) { + editorSidebar.scrollTo( { + top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, + behavior: 'smooth', + } ) + } }, 100 ) },Also applies to: 195-203
src/lazy-components/modal-tour/tours/global-color-schemes.js (1)
197-214: Make waitForElement selectors robust and align comment with selectionUsing
waitForElementon.ugb-tab--style:not(.is-active)and.ugb-block-background-panel:not(.is-opened) buttonwill time out if the tab/panel is already active/open, even though that’s a valid state for the tour. Also, the comment mentions selecting the “Inner Column” while the code selects the parentstackable/columnsblock.Consider:
- dispatch( 'core/edit-post' ).openGeneralSidebar( 'edit-post/block' ) - // Make sure the Inner Column is selected. + dispatch( 'core/edit-post' ).openGeneralSidebar( 'edit-post/block' ) + // Make sure the Columns block is selected. @@ - dispatch( 'core/block-editor' ).selectBlock( columnsBlock.clientId ) - await waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' ) - document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() - - await waitForElement( '.ugb-block-background-panel:not(.is-opened) button' ) - document.querySelector( '.ugb-block-background-panel:not(.is-opened) button' )?.click() + dispatch( 'core/block-editor' ).selectBlock( columnsBlock.clientId ) + await waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style' ) + document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() + + await waitForElement( '.ugb-block-background-panel button' ) + document.querySelector( '.ugb-block-background-panel:not(.is-opened) button' )?.click()This keeps the behavior but avoids timeouts when the tab or panel is already open and clarifies the intent in the comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/guided-modal-tour/tour-conditions.js(1 hunks)src/lazy-components/modal-tour/tours/advanced-hover-states.js(1 hunks)src/lazy-components/modal-tour/tours/block-backgrounds.js(1 hunks)src/lazy-components/modal-tour/tours/global-color-schemes.js(1 hunks)src/lazy-components/modal-tour/tours/hover-states.js(1 hunks)src/lazy-components/modal-tour/tours/responsive-controls.js(1 hunks)src/lazy-components/modal-tour/utils.js(1 hunks)src/welcome/getting-started.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lazy-components/modal-tour/tours/block-backgrounds.js
- src/components/guided-modal-tour/tour-conditions.js
- src/lazy-components/modal-tour/tours/hover-states.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/lazy-components/modal-tour/tours/responsive-controls.js (1)
src/lazy-components/modal-tour/utils.js (2)
waitForElement(1-16)waitForElement(1-16)
src/lazy-components/modal-tour/tours/advanced-hover-states.js (3)
src/lazy-components/modal-tour/tours/block-backgrounds.js (7)
blockObject(11-66)allBlocks(89-89)allBlocks(116-116)allBlocks(180-180)columnsBlock(90-90)columnsBlock(117-117)columnsBlock(181-181)src/lazy-components/modal-tour/tours/responsive-controls.js (5)
blockObject(11-64)allBlocks(90-90)allBlocks(148-148)columnsBlock(91-91)columnsBlock(149-149)src/lazy-components/modal-tour/tours/global-color-schemes.js (4)
allBlocks(203-203)columnsBlock(21-71)columnsBlock(204-204)block(200-200)
src/lazy-components/modal-tour/tours/global-color-schemes.js (2)
src/lazy-components/modal-tour/tours/advanced-hover-states.js (10)
columnsBlock(130-130)columnsBlock(149-149)columnsBlock(219-219)columnsBlock(235-235)columnsBlock(281-281)allBlocks(129-129)allBlocks(148-148)allBlocks(218-218)allBlocks(234-234)allBlocks(280-280)src/lazy-components/modal-tour/utils.js (2)
waitForElement(1-16)waitForElement(1-16)
🔇 Additional comments (1)
src/lazy-components/modal-tour/utils.js (1)
1-16: waitForElement helper is clean and appropriate for tour needsThe polling implementation with
requestAnimationFrameand a timeout is concise and well-suited to synchronizing tour steps with DOM availability. No issues from my side.
# Conflicts: # src/welcome/getting-started.scss
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/lazy-components/modal-tour/tours/global-color-schemes.js (2)
78-81: Consider usingwaitForElementinstead of a fixed 200ms delay ininitialize
initializecurrently waits a hardcoded 200ms before opening the Global Color Schemes panel:setTimeout( () => { document.querySelector( '.ugb-global-color-schemes__panel:not(.is-opened) .components-panel__body-title button' )?.click() }, 200 )Since you already have
waitForElementavailable, you could make this more robust against slower machines or layout changes:- setTimeout( () => { - document.querySelector( '.ugb-global-color-schemes__panel:not(.is-opened) .components-panel__body-title button' )?.click() - }, 200 ) + waitForElement( '.ugb-global-color-schemes__panel .components-panel__body-title button' ) + .then( () => { + document.querySelector( '.ugb-global-color-schemes__panel:not(.is-opened) .components-panel__body-title button' )?.click() + } ) + .catch( () => {} )Not mandatory, but it would align this tour with the more resilient patterns used elsewhere.
186-214: Align “Inner Column” selection comment with code and hardenwaitForElementusageIn this postStep:
// Make sure the Inner Column is selected. const block = select( 'core/block-editor' ).getSelectedBlock() if ( block?.name !== 'stackable/column' ) { const allBlocks = select( 'core/block-editor' ).getBlocks() const columnsBlock = allBlocks.find( block => block.name === 'stackable/columns' ) if ( columnsBlock ) { dispatch( 'core/block-editor' ).selectBlock( columnsBlock.clientId ) await waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style' ) document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() await waitForElement( '.ugb-block-background-panel button' ) document.querySelector( '.ugb-block-background-panel:not(.is-opened) button' )?.click() } }Two points:
The comment says “Inner Column”, but the fallback selection uses
columnsBlock.clientId(the outerstackable/columnsblock). If the control you’re about to point at (backgroundColorScheme) actually lives on the innerstackable/column, you probably want to locate and selectcolumnsBlock.innerBlocks[0].clientIdinstead. Otherwise, update the comment to say “Columns block” so it matches the behavior.Both
await waitForElement(...)calls will throw if the selector never matches, which can abort the tour. A smalltry/catchkeeps it resilient:- if ( columnsBlock ) { - dispatch( 'core/block-editor' ).selectBlock( columnsBlock.clientId ) - await waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style' ) - document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() - - await waitForElement( '.ugb-block-background-panel button' ) - document.querySelector( '.ugb-block-background-panel:not(.is-opened) button' )?.click() - } + if ( columnsBlock ) { + // Select the appropriate block (outer Columns or inner Column) for the backgroundColorScheme control. + dispatch( 'core/block-editor' ).selectBlock( columnsBlock.clientId ) + try { + await waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style' ) + document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() + + await waitForElement( '.ugb-block-background-panel button' ) + document.querySelector( '.ugb-block-background-panel:not(.is-opened) button' )?.click() + } catch ( e ) { + // Fail quietly if the expected UI isn’t present. + } + }You may also want to adjust the selected block to the actual inner column if that’s what the tour step is describing.
src/lazy-components/modal-tour/tours/advanced-hover-states.js (2)
6-56: Avoid reusing hardcodeduniqueIdvalues across toursThe blocks created in
initializeuse fixeduniqueIdstrings ('1dbe04e','f957abc','a8ebea7','57e76a1'), which are also used in other tours. That increases the risk of ID collisions and unintended cross‑block styling when multiple tours have run in the same editor session.Consider either:
- Dropping the
uniqueIdattributes entirely and letting Stackable/Gutenberg generate them, or- Generating fresh, tour‑scoped IDs for each block instance (or using a shared ID helper if one exists in the codebase).
This will make the tours more robust and isolated.
190-207: GuardeditorSidebar.scrollToin “Selecting the Heading Color” preStepIn this preStep you call
editorSidebar.scrollTo(...)without checking that the sidebar element exists:setTimeout( () => { const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) editorSidebar.scrollTo( { top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, behavior: 'smooth', } ) }, 100 )If
.editor-sidebar.ugb--has-panel-tabsisn’t present (different layout, markup change, or editor variant), this will throw and break the tour. The other scroll blocks in this file already use a defensive guard; this one should match.Suggested fix:
- preStep: () => { - setTimeout( () => { - const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top - const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) - editorSidebar.scrollTo( { - top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, - behavior: 'smooth', - } ) - }, 100 ) - }, + preStep: () => { + setTimeout( () => { + const targetElTop = document.querySelector( '.stk-control:has([data-attribute="textColor1"])' )?.getBoundingClientRect()?.top + const editorSidebar = document.querySelector( '.editor-sidebar.ugb--has-panel-tabs' ) + if ( editorSidebar && typeof targetElTop === 'number' ) { + editorSidebar.scrollTo( { + top: targetElTop - editorSidebar.getBoundingClientRect().top - 100, + behavior: 'smooth', + } ) + } + }, 100 ) + },src/lazy-components/modal-tour/tours/block-backgrounds.js (2)
11-72: Stop using shared hardcodeduniqueIdvalues for tour blocks
blockObjectis created with fixeduniqueIdvalues ('1dbe04e','f957abc','a8ebea7','57e76a1','e063798','5d04ca8') that are also reused in other tour modules. This can cause collisions in style scoping or behavior if multiple tours are started in the same editor session.It would be safer to either:
- Omit
uniqueIdon these tour-only blocks, or- Generate fresh IDs per run (ideally via a shared utility).
That keeps the tours isolated from each other and from any real content blocks that might coincidentally share these IDs.
101-148: Fix:not(...)selector typos so background/container toggles actually fireSeveral selectors here use
:not(is-opened),:not(is-checked), and:not(is-open)without the leading dot on the class name. As written, they’re looking for<is-opened>/<is-checked>tags, so the clicks never run:document.querySelector( '.ugb-block-background-panel:not(is-opened) button' ) document.querySelector( '.ugb-block-background-panel .ugb-toggle-panel-form-toggle:not(is-checked) input' ) document.querySelector( '.ugb-block-background-panel .stk-color-palette-control .block-editor-panel-color-gradient-settings__dropdown:not(is-open)' ) document.querySelector( '.ugb-panel--container-size:not(is-opened)' ) document.querySelector( '.ugb-panel--container-size .ugb-toggle-panel-form-toggle:not(is-checked) input' )This means the preStep helpers won’t reliably open the panels or toggles, weakening the tour UX.
Align them with the fixed pattern you’re already using elsewhere (e.g., in the global-color-schemes tour):
- document.querySelector( '.ugb-block-background-panel:not(is-opened) button' )?.click() - document.querySelector( '.ugb-block-background-panel .ugb-toggle-panel-form-toggle:not(is-checked) input' )?.click() + document.querySelector( '.ugb-block-background-panel:not(.is-opened) button' )?.click() + document.querySelector( '.ugb-block-background-panel .ugb-toggle-panel-form-toggle:not(.is-checked) input' )?.click() @@ - document.querySelector( '.ugb-block-background-panel .stk-color-palette-control .block-editor-panel-color-gradient-settings__dropdown:not(is-open)' )?.click() + document.querySelector( '.ugb-block-background-panel .stk-color-palette-control .block-editor-panel-color-gradient-settings__dropdown:not(.is-open)' )?.click() @@ - document.querySelector( '.ugb-panel--container-size:not(is-opened)' )?.click() - document.querySelector( '.ugb-panel--container-size .ugb-toggle-panel-form-toggle:not(is-checked) input' )?.click() + document.querySelector( '.ugb-panel--container-size:not(.is-opened)' )?.click() + document.querySelector( '.ugb-panel--container-size .ugb-toggle-panel-form-toggle:not(.is-checked) input' )?.click()Also applies to: 191-199
🧹 Nitpick comments (2)
src/welcome/getting-started.scss (1)
318-329: Consider avoiding!importanton.s-custom-icon-wrapper svgheightLocking icon height with
!importantcan make future icon-specific overrides harder (e.g., if an icon truly needs a different aspect). If possible, drop!importantand rely on the 24×24 wrapper plus normal specificity; keep this only if you’ve already hit conflicts that require it.src/lazy-components/modal-tour/tours/block-backgrounds.js (1)
122-126: Optionally handlewaitForElementrejection to avoid unhandled promisesBoth uses of
waitForElementhere call.then(...)but don’t attach a.catch, so if the selector never matches within the timeout you’ll get an unhandled promise rejection in the console.You might want to add a no-op catch so failures degrade quietly:
- waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style' ).then( () => { - document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() - } ) + waitForElement( '.edit-post-sidebar__panel-tab.ugb-tab--style' ) + .then( () => { + document.querySelector( '.edit-post-sidebar__panel-tab.ugb-tab--style:not(.is-active)' )?.click() + } ) + .catch( () => {} ) @@ - waitForElement( '.ugb-panel--layout.is-opened .components-panel__body-title button' ).then( () => { - document.querySelector( '.ugb-panel--layout.is-opened .components-panel__body-title button' )?.click() - } ) + waitForElement( '.ugb-panel--layout.is-opened .components-panel__body-title button' ) + .then( () => { + document.querySelector( '.ugb-panel--layout.is-opened .components-panel__body-title button' )?.click() + } ) + .catch( () => {} )Not blocking, but it keeps the tour from spewing console noise if the UI changes.
Also applies to: 180-188
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/sortable-picker/index.js(1 hunks)src/lazy-components/modal-tour/tours/advanced-hover-states.js(1 hunks)src/lazy-components/modal-tour/tours/block-backgrounds.js(1 hunks)src/lazy-components/modal-tour/tours/global-color-schemes.js(1 hunks)src/lazy-components/modal-tour/tours/hover-states.js(1 hunks)src/lazy-components/modal-tour/tours/responsive-controls.js(1 hunks)src/welcome/getting-started.scss(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/lazy-components/modal-tour/tours/responsive-controls.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lazy-components/modal-tour/tours/hover-states.js
- src/components/sortable-picker/index.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/lazy-components/modal-tour/tours/global-color-schemes.js (1)
src/lazy-components/modal-tour/utils.js (2)
waitForElement(1-16)waitForElement(1-16)
src/lazy-components/modal-tour/tours/block-backgrounds.js (2)
src/lazy-components/modal-tour/tours/global-color-schemes.js (3)
columnsBlock(21-71)columnsBlock(204-204)block(200-200)src/lazy-components/modal-tour/utils.js (2)
waitForElement(1-16)waitForElement(1-16)
src/lazy-components/modal-tour/tours/advanced-hover-states.js (1)
src/lazy-components/modal-tour/tours/block-backgrounds.js (7)
blockObject(11-66)allBlocks(89-89)allBlocks(116-116)allBlocks(180-180)columnsBlock(90-90)columnsBlock(117-117)columnsBlock(181-181)
⏰ 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 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: build
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (1)
src/welcome/getting-started.scss (1)
181-316: Quick-start grid/flex layout and hover arrow styling look consistentThe grid-based wrapper, flex alignment on non-video cards, nth-child icon colors, disabled button state, and arrow SVG sizing all look internally consistent and align well with the data-driven QUICK_BUTTONS usage. Nothing blocking here.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.