Skip to content

Refactor carousel block deprecations and ariaLabelPattern - #188

Open
milindmore22 wants to merge 4 commits into
developfrom
fix/210-deprecations
Open

Refactor carousel block deprecations and ariaLabelPattern#188
milindmore22 wants to merge 4 commits into
developfrom
fix/210-deprecations

Conversation

@milindmore22

Copy link
Copy Markdown
Contributor

Summary

This pull request refactors the deprecated versions of the Carousel block to improve maintainability and test coverage. It introduces a shared attributes and supports schema for all deprecated entries, adds a new SaveV210 function for the v2.1.0 save implementation, and provides comprehensive tests for the deprecations. Additionally, it removes the translation function from the ariaLabelPattern field in all save implementations.

Refactoring and Code Maintenance

  • Introduced a sharedAttributes and sharedSupports object to unify the schema for all deprecated Carousel block versions, reducing duplication and making future updates easier. [1] [2]
  • Added a new SaveV210 function to represent the v2.1.0 save logic, and updated the deprecated array to include this version as the latest deprecation entry. [1] [2]
  • Exported SaveV200, SaveV203, and SaveV210 for use in tests and other modules.

Testing Improvements

  • Added a new test file deprecated.test.tsx to verify the structure and behavior of the deprecated Carousel block versions, including attribute schemas and context output.

Internationalization

  • Removed the translation function (__) from the ariaLabelPattern field in all save implementations for both current and deprecated Carousel block versions, standardizing it as a plain string. [1] [2] [3]

Type of change

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

Related issue(s)

N/A

What changed

  • Added a new SaveV210 function to represent the v2.1.0 save logic, and updated the deprecated array to include this version as the latest deprecation entry. [1] [2]
  • Exported SaveV200, SaveV203, and SaveV210 for use in tests and other modules.

Breaking changes

Does this introduce a breaking change? If yes, describe the impact and migration path below.

  • Yes — migration path:
  • No

Testing

Describe how this was tested.

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

Test details:

Screenshots / recordings

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

Copilot AI balanced review requested due to automatic review settings August 7, 2026 09:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Refactors Carousel block deprecations by centralizing shared schema/support definitions, adding a v2.1.0 deprecated save implementation, and standardizing ariaLabelPattern as a plain string for saved output.

Changes:

  • Added SaveV210 and updated the deprecated array ordering to include it as the latest deprecated save.
  • Introduced sharedAttributes / sharedSupports to reduce duplication across deprecated entries.
  • Added unit tests for deprecated entries and exported deprecated save functions for testability.

Reviewed changes

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

File Description
src/blocks/carousel/save.tsx Removes translation call from ariaLabelPattern in the current save output.
src/blocks/carousel/deprecated.tsx Adds SaveV210, centralizes deprecated schema/supports, and removes translation call from deprecated ariaLabelPattern.
src/blocks/carousel/tests/deprecated.test.tsx Adds tests validating the deprecated array structure and SaveV210 context output.

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

Comment thread src/blocks/carousel/deprecated.tsx
Comment thread src/blocks/carousel/deprecated.tsx Outdated
Comment thread src/blocks/carousel/save.tsx Outdated
Comment thread src/blocks/carousel/deprecated.tsx Outdated
Comment thread src/blocks/carousel/deprecated.tsx Outdated
…t translation of ariaLabelPattern for consistent markup
Copilot AI review requested due to automatic review settings August 7, 2026 10:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 3 out of 3 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

src/blocks/carousel/save.tsx:66

  • This makes ariaLabelPattern permanently English in saved output. Because this value is intended for ARIA label text, hard-coding it prevents localization for non-English sites (accessibility regression). A better approach is to keep saved markup stable while still localizing at runtime (e.g., store a stable message key/identifier in saved context and translate in the interactive script), rather than persisting an English string.
		// Un-translated pattern to keep saved block markup consistent across translations
		ariaLabelPattern: 'Go to slide %d',

src/blocks/carousel/tests/deprecated.test.tsx:59

  • This test is brittle because it hard-codes the total number of deprecated entries. Adding a future deprecation (which is expected over time) will fail the test even if ordering and behavior remain correct. Consider asserting the relative ordering of these known entries (e.g., first entry is SaveV210, and SaveV203 appears before SaveV200) without asserting the exact array length.
	it( 'should export a deprecated array with three deprecation entries', () => {
		expect( Array.isArray( deprecated ) ).toBe( true );
		expect( deprecated ).toHaveLength( 3 );
		expect( deprecated[ 0 ].save ).toBe( SaveV210 );
		expect( deprecated[ 1 ].save ).toBe( SaveV203 );
		expect( deprecated[ 2 ].save ).toBe( SaveV200 );
	} );

Comment thread src/blocks/carousel/deprecated.tsx
Comment thread src/blocks/carousel/deprecated.tsx
Comment thread src/blocks/carousel/deprecated.tsx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/blocks/carousel/types.ts:31

  • carouselId is optional in CarouselAttributes, but block.json defines it as a string attribute with a default (\"\"), and the save implementations treat it as a string. Consider making this carouselId: string; to keep TS types aligned with the block schema and reduce the need for runtime fallback logic.
	autoScrollStopOnInteraction: boolean;
	autoScrollStopOnMouseEnter: boolean;
	useTabs: boolean;
	carouselId?: string;
};

src/blocks/carousel/save.tsx:90

  • This fallback is redundant given carouselId is already defaulted during destructuring (carouselId = ''). If you still want a defensive fallback, prefer nullish coalescing (carouselId ?? '') to avoid treating other falsy values as empty (and otherwise just pass carouselId).
		carouselId: carouselId || '',

src/blocks/carousel/deprecated.tsx:276

  • The comment on ariaLabelPattern indicates the goal is locale-independent saved markup, but countLabelPattern and announcementPattern are still translated during save, which keeps data-wp-context locale-dependent. To fully avoid validation drift across locales, consider also making these un-translated in save (and translating at runtime in JS), or revising the comment/rationale so it’s not misleading.
		// Un-translated pattern to keep saved block markup consistent across translations
		ariaLabelPattern: 'Go to slide %d',
		/* translators: {{currentSlide}}: current slide number, {{totalSlides}}: total slide count. */
		countLabelPattern: __(
			'Slide {{currentSlide}} of {{totalSlides}}',
			'rt-carousel',
		),
		announcement: '',
		shouldAnnounce: false,
		/* translators: {{currentSlide}}: current slide number, {{totalSlides}}: total slide count. */
		announcementPattern: __(
			'Slide {{currentSlide}} of {{totalSlides}}',
			'rt-carousel',
		),

src/blocks/carousel/block.json:131

  • This PR introduces a new persisted attribute (carouselId) in the block schema, and it’s now serialized into data-wp-context in save.tsx. The PR description focuses on deprecated refactors + ariaLabelPattern; consider explicitly calling out this schema/API addition (even if non-breaking) so downstream consumers understand the new attribute.
		"useTabs": {
			"type": "boolean",
			"default": false
		},
		"carouselId": {
			"type": "string",
			"default": ""
		}

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.

2 participants