Skip to content

feat: Add slide template picker and templates for carousel block#83

Open
deepaklalwani97 wants to merge 4 commits intodevelopfrom
feature/79-slide-templates
Open

feat: Add slide template picker and templates for carousel block#83
deepaklalwani97 wants to merge 4 commits intodevelopfrom
feature/79-slide-templates

Conversation

@deepaklalwani97
Copy link
Copy Markdown
Member

Summary

Adds a two-step setup flow to the Carousel block: users first pick a slide count, then choose a slide template (Blank, Image, Hero, Image + Caption, or Query Loop). Templates are extensible via the rtcamp.carouselKit.slideTemplates WordPress filter hook.

Type of change

  • Bug fix
  • New feature
  • Enhancement/refactor
  • Documentation update
  • Test update
  • Build/CI/tooling

Related issue(s)

Closes #79

What changed

  • Add slide template picker and templates for carousel block

Testing

Describe how this was tested.

  • Unit tests
  • Manual testing
  • Cross-browser testing (if UI changes)

Test details:

  1. Add a new Carousel block in the editor.
  2. Verify the placeholder shows "How many slides would you like to start with?" with buttons for 1–4 slides and a Skip link.
  3. Click 2 Slides — verify the prompt changes to "Choose a slide template:" with five template cards and a Back button.
  4. Click Back — verify it returns to the slide count step.
  5. Click 2 Slides again, then select Image + Caption.
  6. Verify the carousel initializes with 2 slides, each containing a core/image and a core/paragraph block.
  7. Click Skip on a fresh Carousel block — verify it initializes with an empty viewport and navigation controls.

Screenshots / recordings

Screen.Recording.2026-03-12.at.2.58.29.PM.mov

If applicable, add screenshots or short recordings.

Checklist

  • I have self-reviewed this PR
  • I have added/updated tests where needed
  • I have updated docs where needed
  • I have checked for breaking changes

@deepaklalwani97 deepaklalwani97 marked this pull request as ready for review March 12, 2026 09:32
Copilot AI review requested due to automatic review settings March 12, 2026 09:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a two-step “first-run” setup experience for the Carousel block (slide count → template selection) and introduces a template registry that can be extended via a WordPress filter hook.

Changes:

  • Added a slide template registry (getSlideTemplates) with default templates (Blank, Image, Hero, Image + Caption, Query Loop) and a filter hook for extensibility.
  • Added a Template Picker UI component and editor styling for the picker.
  • Updated Carousel block setup flow in edit.tsx to support the two-step onboarding and initialize inner blocks based on the selected template.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/blocks/carousel/templates.ts Introduces default slide templates and exposes getSlideTemplates() via a filter hook.
src/blocks/carousel/edit.tsx Implements the two-step setup flow and applies selected templates when initializing inner blocks.
src/blocks/carousel/components/TemplatePicker.tsx Adds the template selection UI used during initial block setup.
src/blocks/carousel/editor.scss Adds editor-only styling for the template picker grid and items.
src/blocks/carousel/__tests__/templates.test.ts Adds unit coverage for template registry behavior and default template shapes.
package.json Adds @wordpress/hooks as a direct dependency for the new filter-based extension point.
package-lock.json Locks the updated @wordpress/hooks dependency version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown
Collaborator

@theMasudRana theMasudRana left a comment

Choose a reason for hiding this comment

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

The implementation looks good and functions as expected.
One accessibility concern — in the template selection panel, the options do not appear to be reachable via the Tab key. Could you please verify and fix this?

Image

Copilot AI review requested due to automatic review settings March 27, 2026 06:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +5 to +20
:root {
// Border colors
--carousel-kit-border-default: #ddd;
--carousel-kit-border-dashed: #ccc;

// Background colors
--carousel-kit-bg-white: #fff;
--carousel-kit-bg-icon: #f0f0f0;

// Text colors
--carousel-kit-text-primary: #1e1e1e;
--carousel-kit-text-muted: #757575;

// Accent / interactive (falls back to WP admin theme color)
--carousel-kit-accent: var(--wp-admin-theme-color, #3858e9);
}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

Defining these CSS custom properties on :root makes them global in the editor and can unintentionally affect other blocks/plugins (even if prefixed). Consider scoping them to the carousel wrapper (e.g., .carousel-kit / .editor-styles-wrapper .carousel-kit) so they only apply when the block is present.

Copilot uses AI. Check for mistakes.
Comment on lines +134 to +142
export function getSlideTemplates(): SlideTemplate[] {
const templates = applyFilters(
'rtcamp.carouselKit.slideTemplates',
DEFAULT_TEMPLATES,
);

if ( Array.isArray( templates ) ) {
return templates as SlideTemplate[];
}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

applyFilters receives DEFAULT_TEMPLATES by reference. Filter callbacks can accidentally mutate this array (e.g., templates.push(...)), which would permanently alter defaults for subsequent calls. Pass a shallow copy (and optionally validate/sanitize returned entries) before returning to keep defaults immutable and prevent malformed templates from breaking the picker.

Copilot uses AI. Check for mistakes.
Comment on lines 59 to 64
const [ emblaApi, setEmblaApi ] = useState<EmblaCarouselType | undefined>();
const [ canScrollPrev, setCanScrollPrev ] = useState( false );
const [ canScrollNext, setCanScrollNext ] = useState( false );
const [ setupStep, setSetupStep ] = useState<SetupStep>( 'slide-count' );
const [ pendingSlideCount, setPendingSlideCount ] = useState<number>( 0 );

Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

setupStep/pendingSlideCount are not reset after setup completes. If the user later deletes all inner blocks (making showSetup true again), the placeholder can reopen on the template step with a stale slide count and no direct Skip link. Consider resetting setup state when hasInnerBlocks transitions to false (e.g., in an effect keyed on showSetup).

Copilot uses AI. Check for mistakes.
Comment on lines +453 to +458
{ setupStep === 'template' && (
<TemplatePicker
templates={ getSlideTemplates() }
onSelect={ handleTemplateSelected }
onBack={ () => setSetupStep( 'slide-count' ) }
/>
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

getSlideTemplates() runs during render, which will re-run applyFilters on every re-render while the setup UI is shown. Consider memoizing the templates list (e.g., useMemo) or only computing it when entering the template step to avoid unnecessary work.

Copilot uses AI. Check for mistakes.
expect( template.name.length ).toBeGreaterThan( 0 );
expect( typeof template.label ).toBe( 'string' );
expect( typeof template.description ).toBe( 'string' );
expect( typeof template.icon ).toBe( 'object' );
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

This assertion is brittle: IconType can be a string, function, or element depending on how icons are provided (especially for filtered/custom templates). Consider asserting the icon is defined (or is one of the allowed primitive types) rather than strictly typeof === 'object'.

Suggested change
expect( typeof template.icon ).toBe( 'object' );
expect( template.icon ).toBeDefined();
expect( template.icon ).not.toBeNull();
expect( [ 'string', 'function', 'object' ] ).toContain(
typeof template.icon,
);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Template Selection When Creating Carousel Slides

3 participants