Fix: Disable auto-scroll during fade transition to prevent conflicts#165
Open
milindmore22 wants to merge 1 commit into
Open
Fix: Disable auto-scroll during fade transition to prevent conflicts#165milindmore22 wants to merge 1 commit into
milindmore22 wants to merge 1 commit into
Conversation
…prevent conflicts
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR prevents Carousel Auto Scroll from being enabled/initialized when the transition is set to fade, avoiding plugin conflicts during fade transitions.
Changes:
- Update editor logic to hide Auto Scroll controls for
fadeand forcibly disableautoScrollwhen switching tofade. - Update frontend/view initialization to skip the Auto Scroll plugin when
transition === 'fade', and adjust saved output accordingly. - Add/adjust unit tests and introduce a Jest mock for
embla-carousel-auto-scroll.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/js/setup.ts |
Adds a Jest mock for embla-carousel-auto-scroll to support new view tests. |
src/blocks/carousel/view.ts |
Prevents Auto Scroll plugin initialization when transition is fade. |
src/blocks/carousel/save.tsx |
Stops emitting Auto Scroll settings when transition is fade. |
src/blocks/carousel/edit.tsx |
Hides Auto Scroll panel for fade and disables autoScroll when switching to fade. |
src/blocks/carousel/__tests__/view.test.ts |
Adds a test asserting Auto Scroll isn’t included for fade transitions. |
src/blocks/carousel/__tests__/edit.test.tsx |
Updates editor tests to reflect UI/attribute changes around Auto Scroll and fade. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+67
to
+78
| jest.mock( 'embla-carousel-auto-scroll', () => ( { | ||
| __esModule: true, | ||
| default: jest.fn( () => ( { | ||
| name: 'autoScroll', | ||
| options: {}, | ||
| init: jest.fn(), | ||
| destroy: jest.fn(), | ||
| play: jest.fn(), | ||
| stop: jest.fn(), | ||
| isPlaying: jest.fn( () => false ), | ||
| } ) ), | ||
| } ) ); |
Comment on lines
+957
to
+962
| const lastCall = ( EmblaCarousel as unknown as jest.Mock ).mock.calls.at( -1 ); | ||
| expect( lastCall?.[ 2 ] ).toContainEqual( ( Fade as unknown as jest.Mock ).mock.results.at( -1 )?.value ); | ||
| const autoScrollMockResult = ( AutoScroll as unknown as jest.Mock ).mock.results.at( -1 )?.value; | ||
| if ( autoScrollMockResult ) { | ||
| expect( lastCall?.[ 2 ] ).not.toContainEqual( autoScrollMockResult ); | ||
| } |
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
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
This pull request updates the Carousel block to ensure that the Auto Scroll feature is only available and active when the transition is not set to "fade". It adds safeguards in both the editor and view logic, updates tests to reflect the new behavior, and introduces a mock for the auto-scroll plugin in tests.
Feature restriction and UI updates:
'fade'. If the user selects'fade',autoScrollis automatically set tofalse. (src/blocks/carousel/edit.tsx) [1] [2] [3]autoScrollsettings if the transition is not'fade'. (src/blocks/carousel/save.tsx)Carousel initialization logic:
'fade'. (src/blocks/carousel/view.ts)Testing improvements:
src/blocks/carousel/__tests__/edit.test.tsx) [1] [2] [3]'fade', and added a mock for the auto-scroll module. (src/blocks/carousel/__tests__/view.test.ts,tests/js/setup.ts) [1] [2] [3]Type of change
Related issue(s)
Closes #162
What changed
Breaking changes
Does this introduce a breaking change? If yes, describe the impact and migration path below.
Testing
Describe how this was tested.
Test details:
Screenshots / recordings
If applicable, add screenshots or short recordings.
Checklist