fix(components): Fix Lightbox transitions [JOB-152407]#2918
fix(components): Fix Lightbox transitions [JOB-152407]#2918Aiden-Brine merged 4 commits intomasterfrom
Conversation
|
|
||
| await POM.goNextThenPreviousWithRealTimers(250); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 50)); |
There was a problem hiding this comment.
I really tried to get this to work with fake timers but due to some of the internals of Framer I couldn't get it to work. That means this test will take ~550ms to run which is not ideal but is also not terrible. Open to suggestions but at this time I'm quite confident we can't have this test coverage with fake timers
|
Great fix — the root cause analysis is spot on and using a ref instead of state is the right approach for this AnimatePresence issue. One suggestion on the new test: the
The actual logic that was broken (and fixed) is in the // exported from LightBox.tsx (or tested via a test-only export)
it("enter slides from the right when direction is forward", () => {
expect(variants.enter({ current: 1 })).toEqual({ x: "150%" });
});
it("exit slides to the right when direction is backward", () => {
expect(variants.exit({ current: -1 })).toEqual({ x: "150%" });
});This approach:
The existing navigation tests already cover that clicking next/previous changes the displayed image. Combined with variant unit tests, the full fix is covered: handlers set the ref → variants read the ref correctly → Framer Motion applies the result (library's responsibility). The POM additions and other test cleanups look good 👍 |
Deploying atlantis with
|
| Latest commit: |
e0e9076
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0953ab82.atlantis.pages.dev |
| Branch Preview URL: | https://job-152407-fix-lightbox.atlantis.pages.dev |
32919bc to
e0e9076
Compare
edison-cy-yang
left a comment
There was a problem hiding this comment.
Tested this in Storybook, LGTM!
Motivations
There is some unexpected behaviour with Lightbox transitions when switching direction:
However, things get funky when you change directions:
Changes
This component uses Framer Motion’s AnimatePresence. When the index changes by clicking next, React doesn’t just swap one image for another. It:
So for a short time there are two motion.img elements:
Both need to know the direction (next or previous) to pick the right animation. That direction was passed in via the custom prop (
custom={direction}) where direction was a React state value. When you click “Previous”, the handler runs and updates state:The entering image gets the new direction from the latest render. The exiting image never gets the new props, so it keeps the old direction. This leads to both slides moving the same way for one transition. The fix for this is that instead of using a state value that the exiting element never sees updated, I switched it to a ref that will always hold the current direction.
Fixed
Testing
Play around with things in Storybook. Clicking next should always have the current image leave to the left and the new image enter from the right. And then clicking previous should always be the opposite (current image leaves to the right and the new image enters from the left).
Changes can be
tested via Pre-release
In Atlantis we use Github's built in pull request reviews.