feat (design library): save patterns#3629
Open
kaeizen wants to merge 9 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds saved-pattern storage and design-library support, including saved-tab rendering, typed selection data, preview behavior, and access controls. It also adds an admin import-export settings view with JSON file handling and modal integration. ChangesSaved design library
Settings import and export
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AdminSettings
participant ImportExportSettings
participant ImportExportModal
participant SettingsModel
AdminSettings->>ImportExportSettings: open import-export tab
ImportExportSettings->>ImportExportSettings: parse selected JSON file
ImportExportSettings->>ImportExportModal: provide modal state and import data
ImportExportModal->>SettingsModel: save imported settings
SettingsModel-->>ImportExportModal: return save result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
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.
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.
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.
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.
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.
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
Port saved-patterns design library changes into lazy-components, combine develop error handling with saved-tab support, and fix saved-patterns REST setting defaults/sanitization. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Size Change: +2.58 kB (+0.1%) Total Size: 2.63 MB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lazy-components/design-library/header-actions.js (1)
43-46: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
showProNoticeis used as a value instead of called as a function — Saved tab always shown.
! showProNoticeevaluates tofalsewhenshowProNoticeis a function reference, so! isPro && ! showProNoticeis alwaysfalse. The ternary always appends the Saved tab regardless of pro-notice settings. This was flagged in a previous review and appears unresolved.#!/bin/bash # Verify that showProNotice is a function, not a boolean/value rg -n 'export.*showProNotice' --type=js -g '!node_modules' -g '!dist' | head -20🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lazy-components/design-library/header-actions.js` around lines 43 - 46, Update the controls condition to invoke showProNotice() rather than evaluating the function reference, so the Saved tab is appended only when the existing isPro and pro-notice settings require it. Preserve the current controls and Saved-tab construction unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lazy-components/design-library/design-library-list/editor.scss`:
- Around line 57-83: Update the `@media` (hover: none) override within
.stk-block-design__edit-btn-container to set visibility to visible alongside
opacity: 1, ensuring the container is displayed and interactive on touch devices
while preserving the hover transition behavior elsewhere.
In `@src/lazy-components/design-library/index.js`:
- Around line 112-115: The errors producer in ModalDesignLibrary passes a string
while DesignLibraryList expects an object. In
src/lazy-components/design-library/index.js:112-115, update setErrors in the
designs.error branch to wrap the message under an error key;
src/lazy-components/design-library/design-library-list/index.js:65-70 requires
no direct change because its existing Object.values(errors) rendering will then
display the message.
---
Duplicate comments:
In `@src/lazy-components/design-library/header-actions.js`:
- Around line 43-46: Update the controls condition to invoke showProNotice()
rather than evaluating the function reference, so the Saved tab is appended only
when the existing isPro and pro-notice settings require it. Preserve the current
controls and Saved-tab construction unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80bb8ed4-bfc6-4a7a-9c90-565fa4355200
📒 Files selected for processing (13)
src/design-library/index.jssrc/design-library/init.phpsrc/lazy-components/design-library/design-library-list/design-library-list-item.jssrc/lazy-components/design-library/design-library-list/design-preview.jssrc/lazy-components/design-library/design-library-list/editor.scsssrc/lazy-components/design-library/design-library-list/index.jssrc/lazy-components/design-library/design-library-list/use-preview-renderer.jssrc/lazy-components/design-library/editor.scsssrc/lazy-components/design-library/header-actions.jssrc/lazy-components/design-library/index.jssrc/lazy-components/design-library/util.jssrc/welcome/admin.jssrc/welcome/admin.scss
✅ Files skipped from review due to trivial changes (1)
- src/lazy-components/design-library/editor.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- src/design-library/init.php
- src/welcome/admin.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lazy-components/design-library/header-actions.js (1)
43-46: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
showProNoticeis used as a value instead of called as a function — Saved tab always shown.
! showProNoticeevaluates tofalsewhenshowProNoticeis a function reference, so! isPro && ! showProNoticeis alwaysfalse. The ternary always appends the Saved tab regardless of pro-notice settings. This was flagged in a previous review and appears unresolved.#!/bin/bash # Verify that showProNotice is a function, not a boolean/value rg -n 'export.*showProNotice' --type=js -g '!node_modules' -g '!dist' | head -20🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lazy-components/design-library/header-actions.js` around lines 43 - 46, Update the controls condition to invoke showProNotice() rather than evaluating the function reference, so the Saved tab is appended only when the existing isPro and pro-notice settings require it. Preserve the current controls and Saved-tab construction unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lazy-components/design-library/design-library-list/editor.scss`:
- Around line 57-83: Update the `@media` (hover: none) override within
.stk-block-design__edit-btn-container to set visibility to visible alongside
opacity: 1, ensuring the container is displayed and interactive on touch devices
while preserving the hover transition behavior elsewhere.
In `@src/lazy-components/design-library/index.js`:
- Around line 112-115: The errors producer in ModalDesignLibrary passes a string
while DesignLibraryList expects an object. In
src/lazy-components/design-library/index.js:112-115, update setErrors in the
designs.error branch to wrap the message under an error key;
src/lazy-components/design-library/design-library-list/index.js:65-70 requires
no direct change because its existing Object.values(errors) rendering will then
display the message.
---
Duplicate comments:
In `@src/lazy-components/design-library/header-actions.js`:
- Around line 43-46: Update the controls condition to invoke showProNotice()
rather than evaluating the function reference, so the Saved tab is appended only
when the existing isPro and pro-notice settings require it. Preserve the current
controls and Saved-tab construction unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80bb8ed4-bfc6-4a7a-9c90-565fa4355200
📒 Files selected for processing (13)
src/design-library/index.jssrc/design-library/init.phpsrc/lazy-components/design-library/design-library-list/design-library-list-item.jssrc/lazy-components/design-library/design-library-list/design-preview.jssrc/lazy-components/design-library/design-library-list/editor.scsssrc/lazy-components/design-library/design-library-list/index.jssrc/lazy-components/design-library/design-library-list/use-preview-renderer.jssrc/lazy-components/design-library/editor.scsssrc/lazy-components/design-library/header-actions.jssrc/lazy-components/design-library/index.jssrc/lazy-components/design-library/util.jssrc/welcome/admin.jssrc/welcome/admin.scss
✅ Files skipped from review due to trivial changes (1)
- src/lazy-components/design-library/editor.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- src/design-library/init.php
- src/welcome/admin.js
🛑 Comments failed to post (2)
src/lazy-components/design-library/design-library-list/editor.scss (1)
57-83: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Edit button container remains invisible on touch devices due to missing
visibilityoverride.The
@media (hover: none)block at lines 71-73 setsopacity: 1but does not overridevisibility: hidden(line 70). Sincevisibility: hiddenmakes an element both invisible and non-interactive regardless ofopacity, the edit button container won't appear on touch devices. Compare with the existing.ugb-button-componentat lines 47-56, which only usesopacity— its touch fallback works because there's novisibilityto fight. The new container introducedvisibilityfor the hover transition but forgot to override it in the no-hover media query.🔧 Proposed fix: add `visibility: visible` to the touch media query
visibility: hidden; `@media` (hover: none) { opacity: 1; + visibility: visible; }📝 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..stk-block-design__edit-btn-container { opacity: 0; z-index: 2; transition: opacity 0.4s cubic-bezier(0.2, 0.6, 0.4, 1), visibility 0.4s cubic-bezier(0.2, 0.6, 0.4, 1); position: absolute; padding: 60px 30px 30px; background: linear-gradient(180deg, transparent, `#fffe` 40px, `#fff`) !important; top: auto; bottom: -1px; left: 0; right: 0; display: flex; gap: 24px; visibility: hidden; `@media` (hover: none) { opacity: 1; visibility: visible; } .ugb-button-component { width: 100%; justify-content: center; pointer-events: none; `@media` (hover: none) { pointer-events: auto; } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lazy-components/design-library/design-library-list/editor.scss` around lines 57 - 83, Update the `@media` (hover: none) override within .stk-block-design__edit-btn-container to set visibility to visible alongside opacity: 1, ensuring the container is displayed and interactive on touch devices while preserving the hover transition behavior elsewhere.src/lazy-components/design-library/index.js (1)
112-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
errorsprop contract mismatch: producer passes a string, consumer expects an object.ModalDesignLibrarysetserrorstodesigns.error(a string), butDesignLibraryListguards ontypeof errors === 'object'— the error message is silently swallowed and never shown to users.
src/lazy-components/design-library/index.js#L112-L115: ChangesetErrors( designs.error )tosetErrors( { error: designs.error } )so the value is an object with anerrorkey.src/lazy-components/design-library/design-library-list/index.js#L65-L70: No change needed once the producer wraps the error —Object.values( errors ).join( '; ' )will correctly render the message.📍 Affects 2 files
src/lazy-components/design-library/index.js#L112-L115(this comment)src/lazy-components/design-library/design-library-list/index.js#L65-L70🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lazy-components/design-library/index.js` around lines 112 - 115, The errors producer in ModalDesignLibrary passes a string while DesignLibraryList expects an object. In src/lazy-components/design-library/index.js:112-115, update setErrors in the designs.error branch to wrap the message under an error key; src/lazy-components/design-library/design-library-list/index.js:65-70 requires no direct change because its existing Object.values(errors) rendering will then display the message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit