fix: restore SharedElement styles when a layout effect is cleaned up by Hashim1999164 · Pull Request #10653 · adobe/react-spectrum

Hashim1999164 · 2026-09-25T23:25:56Z

SharedElement writes temporary translate, width, and height values so a selection indicator can animate from a previous instance. A requestAnimationFrame then restores the real styles. React StrictMode cleanup cancelled that frame and did not restore, so the next snapshot stored the temporary values. After SSR hydration the indicator stayed offset.

This change restores those styles when the pending frame is cancelled. A browser hydration matrix covers StrictMode on and off, plus first and last selected tabs. A Storybook story wraps the same layout in StrictMode.

Test plan:
Open the AnimatedSelectionIndicator story and confirm the indicator sits on Settings after load.
Run the Tabs browser hydration tests.
Change tabs with mouse and keyboard. The indicator should follow the selected tab.

I used an AI assistant while drafting this. I reviewed the SharedElement restore and the tests, and I pointed the assistant at AGENTS.md.

Checklist:
Linked issue 10570.
Added tests and a story.
Filled test instructions.
Docs were already accurate, so they are unchanged.
Keyboard and mouse tab behavior still works.
I understand every change.
I followed the AI contribution guidance.

Project: personal

StrictMode cleanup cancelled the restore frame and left temporary translate, width, and height overrides on the element. The next snapshot then stored those values, so the indicator stayed offset after hydration.

snowystinger

How do either of these display or confirm the issue with SSR?

Can you provide a before and after screenshot? The original issue didn't provide that or an easy reproduction, so it makes it hard to verify.

An additional AI review of this PR claims

In StrictMode, React mounts a SharedElement, immediately tears its effect down, then re-runs it. The teardown saves a position snapshot, so on the re-run the element sees that snapshot and thinks it's animating from a previous element — when really it's just itself. That wrong branch skips the step that clears the entering state, so data-entering stays on the element forever.

and gives this test which currently fails. Please verify if the concern is real.

Test code

```

interface EnteringTabsProps {
onEntering: () => void;
}

return (

<TabList aria-label="Entering tabs" style={{display: 'flex', gap: 12}}>
{interruptedKeys.map(key => (
<Tab key={key} id={key} style={{position: 'relative', padding: '12px 20px'}}>
<SelectionIndicator
style={{
position: 'absolute',
inset: 0,
transitionProperty: 'translate, width, height',
transitionDuration: '200ms'
}}
/>
{key}

))}

{interruptedKeys.map(key => (

{key}

))}

);
}

it('does not get stuck in the entering state when effects are double invoked', async () => {
let enteringCount = 0;
let onEntering = () => {
enteringCount++;
};
let {container} = await render(

);

// The entering state is applied on mount...
await expect.poll(() => enteringCount).toBeGreaterThan(0);
// ...and must be cleared once the entering frame has run.
await expect
.poll(() => getIndicator(getTab(container, 'two')).hasAttribute('data-entering'))
.toBe(false);
});

</details>

snowystinger · 2026-09-28T06:27:36Z


 export type TabsStory = StoryFn<typeof Tabs>;

+export const AnimatedSelectionIndicator: TabsStory = () => (


We can already put the storybook into strict mode with strict=true in the url or using the checkbox in the toolbar

good call, dropped the StrictMode wrap from the story. use the toolbar checkbox / ?strict=true instead

snowystinger · 2026-09-28T06:32:48Z


 export type TabsStory = StoryFn<typeof Tabs>;

+export const AnimatedSelectionIndicator: TabsStory = () => (


I ran this story, it doesn't appear to have changed before or after the changes. Is this what you were seeing? or how did you verify in the storybook?

TabsAnimation.mov

yeah the story alone was a weak demo for the ssr/hydration case. the real coverage is the browser hydration matrix in Tabs.browser.test.tsx (strict on/off, first/last selected). that asserts the indicator translate/width/height are cleared after hydrate.

pushed a follow up for the StrictMode double-invoke race your note called out too: a cancelled effect run could still fire its entering microtask and leave data-entering stuck. guarding those async bits now, plus the test you sketched.

Hashim1999164 · 2026-09-29T15:20:02Z

@snowystinger thanks for digging in.

short version:

story StrictMode wrap removed (toolbar / strict=true instead)
the visual offset bug is mainly an SSR hydration thing, so before/after is covered by the Tabs.browser hydration tests rather than the story alone
the AI StrictMode data-entering concern looked real (stale entering microtask after effect cleanup). pushed a guard for that + the double-invoke test you pasted

happy to tweak further if something still looks off

Cancel entering/exiting microtasks and frames from a cleaned-up effect run so StrictMode double-invoke cannot leave data-entering stuck. Drop the redundant StrictMode story wrap and cover the race with a browser test.

snowystinger · 2026-09-29T20:37:40Z

Thanks, looks like lint and browser tests are failing

Hashim1999164 · 2026-09-30T10:50:39Z

@snowystinger yeah those two were on me.

lint was oxlint yelling about useLayoutEffect coming from react, plus oxfmt on the browser test. both cleaned up.

the strict mode test was stuck at enteringCount 0. cleanup was snapshotting the same node, so the replay thought it was animating from a previous element and skipped entering entirely. i drop that self snapshot now, so the replay still enters and the next frame clears data-entering. pushed.

snowystinger · 2026-09-30T11:21:46Z

There's more failing

Hashim1999164 · 2026-09-30T17:09:15Z

lint and typecheck were still red. import order is fixed and the dropped snapshot is typed as optional now.

snowystinger

I was testing the storybook, this PR appears to have broken the forward animation. So arrow left is working, but arrow right is not.

I know it's convenient to rely on the AI, but I'm going to need you, the human, to test these things too. Otherwise this PR will stop helping us and will instead hamper us getting through all the other PRs and work we have. I really appreciate your understanding.

snowystinger · 2026-10-01T05:18:04Z

+  <Tabs defaultSelectedKey="settings">
+    <TabList aria-label="Sections" style={{display: 'flex', gap: 12}}>
+      {['overview', 'activity', 'settings'].map(key => (
+        <Tab key={key} id={key} style={{position: 'relative', padding: '12px 20px'}}>


Suggested change

<Tab key={key} id={key} style={{position: 'relative', padding: '12px 20px'}}>

<Tab

key={key}

id={key}

style={{position: 'relative', padding: '12px 20px', outlineOffset: '2px'}}>

snowystinger · 2026-10-01T05:19:17Z

+    let cancelled = false;
+    // StrictMode cleanup snapshots this same node. That is not a move between parents,
+    // so drop it and take the entering path on the replay.
+    if (prevSnapshot && element && prevSnapshot.element === element) {


I think it's missing isVisible

snowystinger · 2026-10-01T05:35:20Z

 export type TabsStory = StoryFn<typeof Tabs>;

+// Use Storybook toolbar / `?strict=true` for StrictMode. Hydration coverage lives in Tabs.browser.test.tsx.
+export const AnimatedSelectionIndicator: TabsStory = () => (


I need you to explain why you're adding this story. It does not do SSR, so what does it have to do with the bug report?

You description says:

Open the AnimatedSelectionIndicator story and confirm the indicator sits on Settings after load.

But this was the case prior to your changes as well, nothing has changed from what I see. Can you provide a screenshot of before and after, maybe I'm just missing something?

Should testing this story actually be about data-entering being stuck and the description needs some updating?

snowystinger · 2026-10-01T05:43:55Z

+        <TabList aria-label="Hydrated tabs" style={{display: 'flex', gap: 12}}>
+          {keys.map(key => (
+            <Tab key={key} id={key} style={{position: 'relative', padding: '12px 20px'}}>
+              <SelectionIndicator


please add styles to this so that I can see it when i watch or pause the browser test, they can just be what the storybook story had

snowystinger · 2026-10-01T05:52:37Z

+  // for it. The observer is attached during the commit that mounts the tabs, which is
+  // before the microtask that applies the entering state runs.
+  useLayoutEffect(() => {
+    let observer = new MutationObserver(records => {


instead of using a MutationObserver, you could probably use renderProps isEntering

fix: restore SharedElement styles when a layout effect is cleaned up

f3eb6c1

StrictMode cleanup cancelled the restore frame and left temporary translate, width, and height overrides on the element. The next snapshot then stored those values, so the indicator stayed offset after hydration.

github-actions Bot added the RAC label Sep 25, 2026

snowystinger reviewed Sep 28, 2026

View reviewed changes

fix: ignore stale SharedElement async work after StrictMode cleanup

668ff9b

Cancel entering/exiting microtasks and frames from a cleaned-up effect run so StrictMode double-invoke cannot leave data-entering stuck. Drop the redundant StrictMode story wrap and cover the race with a browser test.

Hashim1999164 force-pushed the fix/sharedElementRestoreOnCleanup branch from 06c3c1c to 668ff9b Compare September 29, 2026 15:20

fix: drop StrictMode self snapshots so entering still runs

85a0b54

fix: type the dropped snapshot and sort the test imports

7132f2b

snowystinger reviewed Oct 1, 2026

View reviewed changes


		export type TabsStory = StoryFn<typeof Tabs>;

		export const AnimatedSelectionIndicator: TabsStory = () => (

Conversation

Hashim1999164 commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hashim1999164 commented Sep 29, 2026

Uh oh!

snowystinger commented Sep 29, 2026

Uh oh!

Hashim1999164 commented Sep 30, 2026

Uh oh!

snowystinger commented Sep 30, 2026

Uh oh!

Hashim1999164 commented Sep 30, 2026

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Hashim1999164 commented Sep 25, 2026 •

edited

Loading