feat(think, thought-chain): support destroyOnHidden - #1985
Conversation
Add prop to Think and ThoughtChain components, passing it to CSSMotion's to control whether content DOM nodes are removed when collapsed. - Think: new prop (default: true) - ThoughtChain: new on ThoughtChainItemType (default: true) - Add demo for both components - Add unit tests for both components - Update API docs (zh-CN & en-US) Closes ant-design#1954
📝 WalkthroughWalkthrough为 Think 和 ThoughtChain 增加 ChangesThink 与 ThoughtChain 隐藏销毁控制
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a new destroyOnHidden prop (defaulting to true) to both the Think and ThoughtChain components, allowing users to control whether the content node is destroyed from the DOM when collapsed. It also includes corresponding documentation, demos, and unit tests. A review comment identifies a potential false positive in the ThoughtChain unit tests, where fake timers need to be advanced using jest.runAllTimers() to ensure the collapse transition completes before asserting on the DOM state.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| it('should keep content in DOM when destroyOnHidden is false and collapsed', () => { | ||
| const { container, rerender } = render( | ||
| <ThoughtChain | ||
| items={[ | ||
| { | ||
| key: 'keep-test', | ||
| title: 'Collapsible', | ||
| content: 'Kept content', | ||
| collapsible: true, | ||
| destroyOnHidden: false, | ||
| }, | ||
| ]} | ||
| expandedKeys={['keep-test']} | ||
| />, | ||
| ); | ||
|
|
||
| // Expanded, content should exist | ||
| expect(container.querySelector('.ant-thought-chain-node-content')).toBeTruthy(); | ||
|
|
||
| // Collapse | ||
| rerender( | ||
| <ThoughtChain | ||
| items={[ | ||
| { | ||
| key: 'keep-test', | ||
| title: 'Collapsible', | ||
| content: 'Kept content', | ||
| collapsible: true, | ||
| destroyOnHidden: false, | ||
| }, | ||
| ]} | ||
| expandedKeys={[]} | ||
| />, | ||
| ); | ||
|
|
||
| // Collapsed with destroyOnHidden=false, content node should still exist | ||
| expect(container.querySelector('.ant-thought-chain-node-content')).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
Without advancing the fake timers (e.g., using jest.runAllTimers()), the collapse transition does not complete. As a result, the DOM node remains in the DOM immediately after rerender regardless of whether destroyOnHidden is true or false. This makes the test a false positive because it would pass even if destroyOnHidden was broken or ignored. Advancing the timers ensures we are asserting on the final state after the transition completes.
it('should keep content in DOM when destroyOnHidden is false and collapsed', () => {
const { container, rerender } = render(
<ThoughtChain
items={[
{
key: 'keep-test',
title: 'Collapsible',
content: 'Kept content',
collapsible: true,
destroyOnHidden: false,
},
]}
expandedKeys={['keep-test']}
/>,
);
// Expanded, content should exist
expect(container.querySelector('.ant-thought-chain-node-content')).toBeTruthy();
// Collapse
rerender(
<ThoughtChain
items={[
{
key: 'keep-test',
title: 'Collapsible',
content: 'Kept content',
collapsible: true,
destroyOnHidden: false,
},
]}
expandedKeys={[]}
/>,
);
// Advance timers to let the transition finish
jest.runAllTimers();
// Collapsed with destroyOnHidden=false, content node should still exist
expect(container.querySelector('.ant-thought-chain-node-content')).toBeTruthy();
});
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/x/components/think/__tests__/index.test.tsx`:
- Around line 95-107: Extend the destroyOnHidden test at
packages/x/components/think/__tests__/index.test.tsx:95-107 by collapsing Think
after the expansion assertion, waiting with waitFakeTimer, and asserting
.ant-think-content is removed. Update
packages/x/components/thought-chain/__tests__/index.test.tsx:239-257 to render
expanded via expandedKeys, assert content exists, then rerender without that
key, wait for any exit animation, and assert the content is removed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9fc26a7b-b578-4a41-88ff-f1fe15714f8a
📒 Files selected for processing (13)
packages/x/components/think/Think.tsxpackages/x/components/think/__tests__/index.test.tsxpackages/x/components/think/demo/destroyOnHidden.mdpackages/x/components/think/demo/destroyOnHidden.tsxpackages/x/components/think/index.en-US.mdpackages/x/components/think/index.zh-CN.mdpackages/x/components/thought-chain/Node.tsxpackages/x/components/thought-chain/__tests__/index.test.tsxpackages/x/components/thought-chain/demo/destroyOnHidden.mdpackages/x/components/thought-chain/demo/destroyOnHidden.tsxpackages/x/components/thought-chain/index.en-US.mdpackages/x/components/thought-chain/index.zh-CN.mdpackages/x/components/thought-chain/interface.ts
| it('Think should remove content from DOM when destroyOnHidden is true and collapsed', async () => { | ||
| const { container } = render( | ||
| <Think title="test" destroyOnHidden defaultExpanded={false}> | ||
| content | ||
| </Think>, | ||
| ); | ||
| // collapsed by default, content should not be in DOM | ||
| expect(container.querySelector('.ant-think-content')).toBeNull(); | ||
| // expand | ||
| fireEvent.click(container.querySelector('.ant-think-status-wrapper')!); | ||
| await waitFakeTimer(); | ||
| expect(container.querySelector('.ant-think-content')).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
完善对折叠卸载过程的测试用例覆盖。
这两个针对 destroyOnHidden={true} 的测试用例虽然旨在验证“折叠时从 DOM 移除”,但实际上仅校验了初始渲染且默认处于折叠状态时节点不存在的情况。为了确保卸载功能完全符合预期,建议先触发展开使内容挂载到 DOM 中,然后再触发折叠,最后断言节点已被成功卸载移除。
packages/x/components/think/__tests__/index.test.tsx#L95-L107: 建议在现有断言后,补充触发折叠的代码(例如点击事件及await waitFakeTimer();),并断言内容节点重新变为null。packages/x/components/thought-chain/__tests__/index.test.tsx#L239-L257: 建议初始渲染时指定expandedKeys使其处于展开状态,断言节点存在后,通过rerender移除该 key 触发折叠(注意如涉及离场动画需作相应等待),最后再断言内容节点不再存在。
📍 Affects 2 files
packages/x/components/think/__tests__/index.test.tsx#L95-L107(this comment)packages/x/components/thought-chain/__tests__/index.test.tsx#L239-L257
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/x/components/think/__tests__/index.test.tsx` around lines 95 - 107,
Extend the destroyOnHidden test at
packages/x/components/think/__tests__/index.test.tsx:95-107 by collapsing Think
after the expansion assertion, waiting with waitFakeTimer, and asserting
.ant-think-content is removed. Update
packages/x/components/thought-chain/__tests__/index.test.tsx:239-257 to render
expanded via expandedKeys, assert content exists, then rerender without that
key, wait for any exit animation, and assert the content is removed.
Description
Closes #1954
Add
destroyOnHiddenprop to Think and ThoughtChain components, allowing users to control whether content DOM nodes are removed from the DOM when the component is collapsed.Background
Current state
After investigating the source code, we found that the original code does not explicitly set
removeOnLeaveonCSSMotion. SinceCSSMotion'sremoveOnLeavedefaults totrue, content DOM nodes are already being removed when collapsed. This means the "DOM not destroyed" behavior described in the issue may have already been resolved in the current version.This PR is a minor enhancement suggestion: expose
CSSMotion'sremoveOnLeaveas a user-facingdestroyOnHiddenprop so that users can explicitly opt into either behavior:destroyOnHidden=true(default): content node is removed from DOM after collapse animation — current behavior, no breaking changedestroyOnHidden=false: content node is kept in DOM withleavedClassNamefor CSS-only hiding — useful for streaming rendering scenarios (supportdestroyOnHiddenwithThinkandThoughtChain#1677) where preserving DOM state is preferredChanges
Think (
packages/x/components/think/)ThinkProps: addeddestroyOnHidden?: booleanprop (default:true)Think.tsx: passesdestroyOnHiddento CSSMotion'sremoveOnLeaveThoughtChain (
packages/x/components/thought-chain/)ThoughtChainItemType: addeddestroyOnHidden?: booleanprop (default:true)Node.tsx: passesdestroyOnHiddento CSSMotion'sremoveOnLeaveDemo
think/demo/destroyOnHidden.tsx— demo with toggle switchthought-chain/demo/destroyOnHidden.tsx— demo with toggle switchUnit Tests
destroyOnHidden=true, and kept whendestroyOnHidden=falseDocs
index.zh-CN.mdandindex.en-US.mdfor both componentsChecklist