-
Notifications
You must be signed in to change notification settings - Fork 67
feat (design library): save patterns #3629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughAdds a "saved" tab and a design Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Modal as ModalDesignLibrary
participant List as DesignLibraryList
participant Filter as filterDesigns
participant Util as replacePlaceholders
participant Block as createBlockWithAttributes
Note over Modal,List: Tabs = patterns | pages | saved
User->>Modal: Open modal (selectedTab)
Modal->>Filter: filterDesigns(type=selectedTab)
Filter-->>Modal: designs[]
Modal->>List: Render list (selectedTab)
User->>List: Select design(s)
List-->>Modal: onSelect(design { ..., type: selectedTab })
Modal->>Util: replacePlaceholders(content, ..., type=selectedTab)
alt type == saved
Util-->>Modal: return content unchanged
else type == pages
Util-->>Modal: apply pages placeholder logic
else
Util-->>Modal: apply DEFAULT_CONTENT replacements
end
Modal->>Block: createBlockWithAttributes(..., type=selectedTab)
Block-->>Modal: blocks inserted
sequenceDiagram
autonumber
actor Admin
participant Settings as Admin Settings
participant Modal as ImportExportModal
participant Ctx as ImportExportSettingsContext
participant Filters as applyFilters(...)
participant Model as models.Settings
Admin->>Settings: Open Import/Export
Settings->>Modal: Open ImportExportModal
Modal->>Ctx: Provide shared state (file / settings)
alt Export
Modal->>Filters: export-settings handlers
Filters-->>Modal: exported settings
Modal->>Admin: trigger download (timestamped JSON)
else Import
Admin->>Modal: Upload JSON
Modal->>Filters: import-settings handlers
Filters-->>Modal: prepared settings
Modal->>Model: save settings
Model-->>Modal: success
Modal-->>Admin: show snackbar + close
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/modal-design-library/modal.js (1)
81-101: EnsuresavedPatternsdefaults to an array
applyFilters( 'stackable.design-library.patterns', selectedTab )uses the current tab string as the default value. When no filter provides saved patterns (the common case on non‑Pro setups),savedPatternsbecomes the string'saved', so the sidebar state setters receive a string instead of an array and the saved tab breaks. Pass an empty array as the default and forwardselectedTabas a context argument.- const savedPatterns = applyFilters( 'stackable.design-library.patterns', selectedTab ) + const savedPatterns = applyFilters( 'stackable.design-library.patterns', [], selectedTab )
🧹 Nitpick comments (1)
src/components/modal-design-library/editor.scss (1)
468-472: Consider a z-index management strategy.The conditional z-index elevation works, but the high value (100001) and the need for
:has()selector suggests potential z-index management complexity. Consider establishing a documented z-index scale or using CSS custom properties for consistent layering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/block/design-library/edit.js(4 hunks)src/components/design-library-list/design-library-list-item.js(6 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/editor.scss(2 hunks)src/components/design-library-list/index.js(3 hunks)src/components/design-library-list/use-preview-renderer.js(4 hunks)src/components/design-library-list/util.js(1 hunks)src/components/modal-design-library/editor.scss(1 hunks)src/components/modal-design-library/header-actions.js(2 hunks)src/components/modal-design-library/modal.js(9 hunks)src/components/pro-control/index.js(1 hunks)src/design-library/index.js(2 hunks)src/design-library/init.php(3 hunks)src/welcome/admin.js(4 hunks)src/welcome/admin.scss(1 hunks)src/welcome/import-export/context.js(1 hunks)src/welcome/import-export/design-library.js(1 hunks)src/welcome/import-export/import-export.scss(1 hunks)src/welcome/import-export/index.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/components/design-library-list/index.jssrc/components/design-library-list/use-preview-renderer.jssrc/components/design-library-list/design-preview.jssrc/components/modal-design-library/modal.jssrc/components/design-library-list/design-library-list-item.js
🧬 Code graph analysis (12)
src/components/design-library-list/util.js (1)
src/components/design-library-list/use-preview-renderer.js (1)
isDesignLibraryDevMode(71-71)
src/components/design-library-list/index.js (4)
src/components/design-library-list/use-preview-renderer.js (1)
props(41-53)src/components/modal-design-library/header-actions.js (1)
props(25-32)src/components/modal-design-library/modal.js (1)
selectedTab(68-68)src/components/pro-control/index.js (1)
ProControl(163-191)
src/design-library/init.php (3)
src/dynamic-breakpoints.php (1)
sanitize_array_setting(135-137)src/plugins/global-settings/color-schemes/index.php (1)
sanitize_array_setting(154-156)src/global-settings.php (1)
sanitize_array_setting(507-509)
src/components/design-library-list/use-preview-renderer.js (3)
src/components/design-library-list/index.js (1)
selectedTab(84-92)src/components/modal-design-library/modal.js (1)
selectedTab(68-68)src/components/design-library-list/util.js (4)
getCategorySlug(329-339)getCategorySlug(329-339)replacePlaceholders(183-197)replacePlaceholders(183-197)
src/components/design-library-list/design-preview.js (2)
src/components/design-library-list/index.js (1)
selectedTab(84-92)src/components/modal-design-library/modal.js (1)
selectedTab(68-68)
src/components/modal-design-library/header-actions.js (1)
src/components/advanced-toolbar-control/index.js (1)
AdvancedToolbarControl(197-291)
src/components/modal-design-library/modal.js (3)
src/components/color-palette-control/index.js (1)
applyFilters(119-119)src/components/design-library-list/index.js (1)
selectedTab(84-92)src/design-library/index.js (2)
filterDesigns(74-89)filterDesigns(74-89)
src/components/design-library-list/design-library-list-item.js (3)
src/components/design-library-list/use-auto-scroll.js (2)
onMouseOut(141-141)onMouseOver(140-140)src/components/design-library-list/index.js (2)
selectedTab(84-92)previewProps(101-124)src/components/color-palette-control/index.js (1)
applyFilters(119-119)
src/block/design-library/edit.js (3)
src/components/design-library-list/use-preview-renderer.js (1)
isDesignLibraryDevMode(71-71)src/design-library/index.js (2)
design(103-103)designs(7-7)src/components/design-library-list/util.js (1)
blocksForSubstitution(217-217)
src/welcome/import-export/design-library.js (3)
src/welcome/admin.js (9)
props(606-613)props(646-650)props(832-836)props(898-903)props(1171-1175)props(1359-1363)props(1464-1468)props(1605-1608)settings(514-514)src/welcome/import-export/index.js (1)
props(23-25)src/welcome/import-export/context.js (2)
useImportExportSettingsContext(5-7)useImportExportSettingsContext(5-7)
src/welcome/import-export/index.js (2)
src/welcome/admin.js (3)
modalState(1615-1615)importFile(1616-1616)settings(514-514)src/welcome/import-export/context.js (2)
ImportExportSettingsContext(3-3)ImportExportSettingsContext(3-3)
src/welcome/admin.js (1)
src/welcome/import-export/index.js (2)
ImportExportModal(22-51)ImportExportModal(22-51)
⏰ 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 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP latest
- 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 6.7.2
🔇 Additional comments (12)
src/components/pro-control/index.js (1)
153-160: LGTM!The new ProControl label for saved patterns follows the established pattern and provides clear feature descriptions.
src/components/design-library-list/editor.scss (3)
57-75: LGTM!The overlay edit button container follows a standard hover-reveal pattern with appropriate transitions and positioning.
83-86: LGTM!The hover transition delay provides a good user experience by avoiding premature display of edit controls during quick mouse movements.
285-290: LGTM!The cursor styling correctly indicates that saved items are non-interactive content while the footer remains actionable.
src/block/design-library/edit.js (2)
119-119: LGTM!The type parameter is correctly threaded through the design creation flow, enabling type-aware processing for saved patterns and other design types.
Also applies to: 259-260, 267-267, 341-341, 348-350
210-238: Verify spacing behavior for saved designs.The conditional
type !== 'saved'skips margin/padding adjustments for saved designs, preserving their original spacing. Confirm this is the intended behavior, as users might expect consistent spacing adjustments across all design types.Consider:
- Should saved designs retain their exact spacing, or should they adapt to the current site's spacing presets like other designs?
- Are there specific use cases where preserving original spacing is critical for saved patterns?
src/design-library/index.js (2)
32-38: LGTM!The defensive coding with optional chaining and fallbacks prevents runtime errors when API data is incomplete or missing, improving robustness.
78-82: LGTM!The filtering logic correctly skips plan-based filtering for saved patterns, as these are user-created designs that don't have premium/plan restrictions.
src/welcome/admin.scss (1)
918-918: LGTM!The import statement correctly integrates the new import-export UI styles into the admin stylesheet.
src/welcome/import-export/context.js (1)
1-8: LGTM!The context and hook implementation follows React best practices for sharing state across the import/export UI components.
src/components/design-library-list/design-preview.js (2)
58-76: Verify the condition change for event listeners.The condition changed from
selectedTab !== 'patterns'toselectedTab !== 'pages', which is a logic inversion. This now restricts mouse drag listeners to only the 'pages' tab, whereas they previously applied to all non-patterns tabs. Confirm this is intentional and that patterns, saved, and other tabs still have appropriate interaction handlers.Based on learnings
124-124: LGTM!The addition of the
is-layout-constrainedclass provides appropriate layout constraints for the preview wrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/design-library-list/design-library-list-item.js(6 hunks)src/components/design-library-list/design-preview.js(3 hunks)src/components/design-library-list/editor.scss(2 hunks)src/welcome/import-export/index.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/components/design-library-list/design-library-list-item.jssrc/components/design-library-list/design-preview.js
🧬 Code graph analysis (3)
src/components/design-library-list/design-library-list-item.js (2)
src/components/design-library-list/use-auto-scroll.js (2)
onMouseOut(141-141)onMouseOver(140-140)src/components/design-library-list/index.js (2)
selectedTab(84-92)previewProps(101-124)
src/welcome/import-export/index.js (2)
src/welcome/admin.js (3)
modalState(1615-1615)importFile(1616-1616)settings(514-514)src/welcome/import-export/context.js (2)
ImportExportSettingsContext(3-3)ImportExportSettingsContext(3-3)
src/components/design-library-list/design-preview.js (2)
src/components/design-library-list/index.js (1)
selectedTab(84-92)src/components/modal-design-library/modal.js (1)
selectedTab(68-68)
⏰ 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 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- 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 6.6.2
- GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (3)
src/components/design-library-list/design-preview.js (3)
56-76: Tab gating logic correctly updated for multi-tab support.The condition change from
selectedTab === 'patterns'toselectedTab !== 'pages'correctly limits the drag-scrolling functionality to the 'pages' tab only, which aligns with the PR's addition of a 'saved' patterns tab alongside existing 'pages' and 'patterns' tabs.
127-127: Verify layout across all preview types. Theis-layout-constrainedclass is applied unconditionally in design-preview.js (line 127). Manually test pages, patterns, and saved previews to confirm it delivers the intended layout for each.
93-94: Confirm nativeinertsupport or include polyfill.
- Native support: Chrome/Edge ≥102, Firefox ≥112, Safari ≥15.5.
- For older browsers, import the WICG/inert polyfill (wicg/inert) plus required shims (Map, Set, Element.prototype.matches, Node.prototype.contains).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/modal-design-library/modal.js (1)
81-81: Consider memoizingsavedPatternsto avoid unnecessary effect runs.
savedPatternsis computed on every render viaapplyFilters. If the filter returns a new array reference each time (rather than a memoized value), the effect on line 95 will re-run on every render whenselectedTab === 'saved', causingsetSidebarDesignsto be called repeatedly with the same data.Consider wrapping the computation in
useMemo:-const savedPatterns = applyFilters( 'stackable.design-library.patterns', [], selectedTab ) +const savedPatterns = useMemo( + () => applyFilters( 'stackable.design-library.patterns', [], selectedTab ), + [ selectedTab ] +)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/modal-design-library/modal.js(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/components/modal-design-library/modal.js
🧬 Code graph analysis (1)
src/components/modal-design-library/modal.js (2)
src/components/design-library-list/index.js (1)
selectedTab(84-92)src/design-library/index.js (2)
filterDesigns(74-89)filterDesigns(74-89)
⏰ 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 8.2 and WP 6.6.2
- 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 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (4)
src/components/modal-design-library/modal.js (4)
96-102: LGTM! Early return logic for saved tab is correct.The saved tab path appropriately:
- Populates sidebar from filtered patterns instead of fetching from server
- Resets category selection and flags
- Updates loading state
125-125: Type propagation is consistent and aligns with filter logic.Adding
type: selectedTabto:
- The
filterDesignscall (line 125)- Selected designs for 'pages' tab (line 152)
- Multi-select design data for other tabs (line 182)
This correctly threads the design type through selection and filtering, enabling downstream code to differentiate between pattern sources (patterns vs. saved vs. pages).
Also applies to: 152-152, 182-182
261-269: Visibility logic correctly expanded to include saved patterns.Changing conditions from
selectedTab === 'patterns'toselectedTab !== 'pages'appropriately allows both 'patterns' and 'saved' tabs to:
- Display the Section Background toggle (line 261)
- Show the multi-select footer with "Add Designs" (line 409)
This aligns with the intended behavior where saved patterns support the same styling options as regular patterns.
Also applies to: 409-420
406-406: PassingselectedTabprop enables tab-aware rendering.DesignLibraryList now receives the active tab context, which per the relevant code snippets is consumed via
useDesignLibraryContextand used to adapt preview rendering behavior based on the tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/design-library-list/editor.scss(2 hunks)src/components/modal-design-library/modal.js(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/components/modal-design-library/modal.js
🧬 Code graph analysis (1)
src/components/modal-design-library/modal.js (3)
src/hooks/use-block-color-schemes.js (1)
useSelect(19-217)src/components/design-library-list/index.js (1)
selectedTab(84-92)src/design-library/index.js (2)
filterDesigns(74-89)filterDesigns(74-89)
⏰ 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 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/welcome/import-export/index.js (3)
125-125: Remove unnecessary arrow function wrapper.The arrow function wrapper
() => handleImport()is unnecessary sincehandleImporttakes no arguments.Apply this diff:
- onClick={ () => handleImport() } + onClick={ handleImport }
101-115: Consider extracting TabPanel configuration.The TabPanel structure is duplicated between Import and Export components. Extract the tab configuration to a shared constant to reduce duplication.
Add before the components:
const SETTINGS_TABS = [ { name: 'design-library', title: __( 'Design Library', i18n ), }, { name: 'design-system', title: __( 'Design System', i18n ), disabled: true, }, ]Then use
tabs={ SETTINGS_TABS }in both TabPanel instances.Also applies to: 167-181
97-98: Note: Context value structure differs between Import and Export.Import provides a 3-item array
[importFile, importedSettings, setImportedSettings]while Export provides a 2-item array[exportedSettings, setExportedSettings]. Consumers must know which mode they're in to destructure correctly. Consider documenting this or using a consistent object shape instead.Also applies to: 163-164
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/design-library-list/design-library-list-item.js(6 hunks)src/welcome/import-export/index.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.564Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.564Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/components/design-library-list/design-library-list-item.js
🧬 Code graph analysis (2)
src/components/design-library-list/design-library-list-item.js (2)
src/components/design-library-list/use-auto-scroll.js (2)
onMouseOut(141-141)onMouseOver(140-140)src/components/design-library-list/index.js (2)
selectedTab(84-92)previewProps(101-124)
src/welcome/import-export/index.js (2)
src/welcome/admin.js (3)
modalState(1615-1615)importFile(1616-1616)settings(514-514)src/welcome/import-export/context.js (2)
ImportExportSettingsContext(3-3)ImportExportSettingsContext(3-3)
⏰ 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 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.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (10)
src/components/design-library-list/design-library-list-item.js (6)
24-24: LGTM: Clean imports for the new filtering mechanism.The
FragmentandapplyFiltersimports support the new pattern-actions filter hook added at line 124.Also applies to: 28-28
94-104: LGTM: Keyboard activation properly restored.The
buttonAttributesobject correctly implements Enter/Space key handling withpreventDefault()to avoid page scrolling, addressing the previous review feedback.
124-124: LGTM: Safe default for the filter hook.Using
Fragmentas the default prevents the render crash (plain object) while providing a safe, renderable fallback when no Pro filters are attached. This is a valid alternative to passingnull.
113-113: Verify the intentional inconsistency in interaction areas across tabs.The tile's clickable region varies by tab:
- Patterns tab: entire tile is clickable (line 113)
- Saved tab: only the footer is clickable (line 152)
- Pages tab: only the Insert button is clickable (lines 169-182)
This inconsistent interaction model may confuse users as they switch tabs. Please confirm this is intentional and aligns with the UX design.
Also applies to: 152-152, 169-182
107-108: LGTM: eslint-disable comments are justified for the conditional interaction model.The suppressed accessibility warnings are reasonable because:
- Mouse handlers (
onMouseOut/onMouseOver) implement auto-scroll preview, not core user interaction- Core interaction (design selection) has proper keyboard support via
buttonAttributeswhen applied- The conditional spread pattern makes it difficult for the linter to verify keyboard support
Monitor these suppressions if the interaction logic changes to ensure accessibility remains intact.
Also applies to: 148-148
168-183: LGTM: Selection indicator now works correctly across all non-pages tabs.Inverting the conditional from
selectedTab === 'pages'toselectedTab !== 'pages'ensures the selection number displays for both Patterns and Saved tabs while preserving the Insert button for the Pages tab.src/welcome/import-export/index.js (4)
22-51: LGTM!The modal structure is clean with proper i18n, conditional rendering based on
modalState, and appropriate state management for notices.
65-93: LGTM! Previous review concerns fully addressed.The async error handling now correctly:
- Awaits
model.save()with try/catch- Returns early on validation errors (lines 71-76) without closing the modal
- Returns early when no settings to save (lines 78-81)
- Only calls
onClose()after successful save (line 87)- Surfaces failures to the user via notices and console errors
130-141: LGTM!The timestamp formatting is correct, including the month adjustment (+1). Using local time for admin export filenames is acceptable.
2-2: Remove incorrect side-effect import placement suggestion
The side-effect import of./design-libraryis safe:design-library.jsimports its needed WordPress dependencies internally, so there’s no hook availability issue.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/design-library-list/editor.scss(2 hunks)src/welcome/import-export/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/design-library-list/editor.scss
🧰 Additional context used
🧬 Code graph analysis (1)
src/welcome/import-export/index.js (2)
src/welcome/admin.js (3)
modalState(1615-1615)importFile(1616-1616)settings(514-514)src/welcome/import-export/context.js (2)
ImportExportSettingsContext(3-3)ImportExportSettingsContext(3-3)
⏰ 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.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (5)
src/welcome/import-export/index.js (5)
22-51: LGTM! Clean modal wrapper implementation.The component correctly handles modal state, notice display, and conditional rendering of import/export flows.
66-68: Verify: Should users be notified when import file is invalid?When
importedSettingslacks asettingskey, the modal closes silently without showing any error notice. This may confuse users who don't understand why their import failed.Consider whether this should show an error notice before closing:
if ( ! ( 'settings' in importedSettings ) ) { + setNotice( __( 'Invalid import file format.', i18n ) ) + // eslint-disable-next-line no-console + console.error( __( 'Stackable: Import error - Missing "settings" key', i18n ) ) onClose() return }
65-93: Excellent error handling in import flow.The async error handling properly addresses previous review feedback:
- Awaits
model.save()and catches failures- Returns early on validation errors without closing the modal
- Only closes on successful save
- Surfaces errors to users via notices and console logs
130-141: LGTM! Timestamped filename generation.The utility correctly formats the current date/time into a consistent filename pattern.
143-202: LGTM! Robust export implementation.The export flow correctly:
- Wraps
JSON.stringifyin try-catch to handle serialization errors (addresses previous review)- Creates and downloads the blob safely
- Only closes the modal on successful export
- Surfaces errors via notices and console logs
- Disables the button until there are settings to export
Summary by CodeRabbit
New Features
UI / Behaviour
Bug Fix / Preservation
Styling