Skip to content

refactor: 重构collapse为details实现 #362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

refactor: 重构collapse为details实现 #362

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3cfdc2e
feat: 重构collapse为details实现
IVLIU Mar 17, 2025
5ffc99a
fix: 用例失败
IVLIU Mar 18, 2025
df7f09e
fix: 补全缺失的包
IVLIU Mar 19, 2025
ce3ba4c
feat: 阻止onItemClick事件冒泡
IVLIU Mar 19, 2025
ff26366
feat: 完善动画支持
IVLIU May 10, 2025
925207f
feat: 将useEffect中状态设置改为派生状态
IVLIU May 10, 2025
5026c13
fix: 移除rc-motion & 修复用例错误
IVLIU Jun 13, 2025
9f5873f
Merge branch 'master' into refactor/native-details-element
IVLIU Jun 13, 2025
a834f41
feat: 支持view transition过渡
IVLIU Jun 13, 2025
3bd0abf
Merge branch 'refactor/native-details-element' of github.com:IVLIU/co…
IVLIU Jun 13, 2025
60430c7
feat: 阻止默认行为
IVLIU Jun 13, 2025
4ddcf52
feat: 移除startTransition
IVLIU Jun 14, 2025
dba7f64
feat: 恢复CSSMotion调用
IVLIU Jun 16, 2025
5ed0807
feat: 恢复PanelContent style使用
IVLIU Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ coverage
es
yarn.lock
package-lock.json
pnpm-lock.yaml
.storybook
.doc

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
},
"devDependencies": {
"@rc-component/father-plugin": "^2.0.1",
"@rc-component/np": "^1.0.3",
"@testing-library/jest-dom": "^6.1.4",
"@testing-library/react": "^14.1.2",
"@testing-library/user-event": "^14.5.2",
Expand Down
23 changes: 13 additions & 10 deletions src/Collapse.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,21 @@ const Collapse = React.forwardRef<HTMLDivElement, CollapseProps>((props, ref) =>
});

const onItemClick = (key: React.Key) =>
setActiveKey(() => {
if (accordion) {
return activeKey[0] === key ? [] : [key];
}
// ? 为了解决https://github.com/facebook/react/issues/15486
React.startTransition(() => {
setActiveKey(() => {
if (accordion) {
return activeKey[0] === key ? [] : [key];
}

const index = activeKey.indexOf(key);
const isActive = index > -1;
if (isActive) {
return activeKey.filter((item) => item !== key);
}
const index = activeKey.indexOf(key);
const isActive = index > -1;
if (isActive) {
return activeKey.filter((item) => item !== key);
}

return [...activeKey, key];
return [...activeKey, key];
});
});

// ======================== Children ========================
Expand Down
15 changes: 8 additions & 7 deletions src/Panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React from 'react';
import type { CollapsePanelProps } from './interface';
import PanelContent from './PanelContent';

const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((props, ref) => {
const CollapsePanel = React.forwardRef<HTMLDetailsElement, CollapsePanelProps>((props, ref) => {
const {
showArrow = true,
headerClass,
Expand Down Expand Up @@ -33,8 +33,9 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop
const ifExtraExist = extra !== null && extra !== undefined && typeof extra !== 'boolean';

const collapsibleProps = {
onClick: () => {
onClick: (e: React.MouseEvent) => {
onItemClick?.(panelKey);
e.stopPropagation();
},
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

阻止 <summary> 默认行为，避免双重状态切换

点击位于 <summary> 内部的元素时，浏览器会先执行 <details> 的默认展开 / 折叠，再执行 React 更新逻辑，导致「闪动」及状态竞争；尤其在受控模式下容易出现不一致。
应同时 preventDefault 与 stopPropagation，并在之后触发 onItemClick。

-    onClick: (e: React.MouseEvent) => {
-      onItemClick?.(panelKey);
-      e.stopPropagation();
-    },
+    onClick: (e: React.MouseEvent) => {
+      e.preventDefault();     // 阻止原生 toggle
+      e.stopPropagation();    // 阻止冒泡
+      onItemClick?.(panelKey);
+    },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onClick: (e: React.MouseEvent) => {
onItemClick?.(panelKey);
e.stopPropagation();
},
onClick: (e: React.MouseEvent) => {
e.preventDefault(); // 阻止原生 toggle
e.stopPropagation(); // 阻止冒泡
onItemClick?.(panelKey);
},
🤖 Prompt for AI Agents 
In src/Panel.tsx around lines 35 to 38, the onClick handler inside the <summary>
element only calls e.stopPropagation(), which does not prevent the default
toggle behavior of <details>. To fix the flickering and state conflicts, add
e.preventDefault() before e.stopPropagation(), and then call onItemClick. This
will stop the default expand/collapse action and avoid double state changes.

Copy link
Member

Choose a reason for hiding this comment

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

看看这个？

Copy link
Contributor

Choose a reason for hiding this comment

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

@afc163 CodeRabbit 提出的建议是正确的！这个问题很重要，因为在使用原生 <details> 元素的重构版本中：

问题分析：

  • 当前代码只调用了 e.stopPropagation() 但缺少 e.preventDefault()
  • <summary> 的默认行为是切换父级 <details> 的 open 属性
  • 在受控组件中，React 通过 props 管理 open 状态
  • 没有 preventDefault() 会导致原生切换和 React 状态更新同时发生，造成状态竞争和闪烁

修复建议：

onClick: (e: React.MouseEvent) => {
  e.preventDefault();     // 阻止原生 toggle 行为
  e.stopPropagation();    // 阻止事件冒泡
  onItemClick?.(panelKey); // React 控制状态更新
},

这个修复确保了：

  1. 阻止浏览器的默认展开/折叠行为
  2. 完全由 React 控制状态变化
  3. 避免双重切换导致的闪烁和不一致

这个修改对于从 div 实现迁移到原生 <details> 实现的稳定性至关重要。

Copy link
Author

Choose a reason for hiding this comment

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

看看这个？

已处理

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

onKeyDown: (e: React.KeyboardEvent) => {
if (e.key === 'Enter' || e.keyCode === KeyCode.ENTER || e.which === KeyCode.ENTER) {
Expand Down Expand Up @@ -79,16 +80,16 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop
);

// ======================== HeaderProps ========================
const headerProps: React.HTMLAttributes<HTMLDivElement> = {
const headerProps: React.HTMLAttributes<HTMLElement> = {
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Since headerProps are spread onto a <summary> element, using React.HTMLAttributes<HTMLSummaryElement> would give you more precise TypeScript support.

Suggested change
const headerProps: React.HTMLAttributes<HTMLElement> = {
const headerProps: React.HTMLAttributes<HTMLSummaryElement> = {

Copilot uses AI. Check for mistakes.

className: headerClassName,
style: styles?.header,
...(['header', 'icon'].includes(collapsible) ? {} : collapsibleProps),
};

// ======================== Render ========================
return (
<div {...resetProps} ref={ref} className={collapsePanelClassNames}>
<div {...headerProps}>
<details {...resetProps} ref={ref} className={collapsePanelClassNames} open={isActive}>
<summary {...headerProps}>
{showArrow && iconNode}
<span
className={classNames(`${prefixCls}-title`, customizeClassNames?.title)}
Expand All @@ -98,7 +99,7 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop
{header}
</span>
{ifExtraExist && <div className={`${prefixCls}-extra`}>{extra}</div>}
</div>
</summary>
<CSSMotion
visible={isActive}
leavedClassName={`${prefixCls}-panel-hidden`}
Expand All @@ -124,7 +125,7 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop
);
}}
</CSSMotion>
</div>
</details>
);
});

Expand Down
6 changes: 3 additions & 3 deletions src/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export interface ItemType
> {
key?: CollapsePanelProps['panelKey'];
label?: CollapsePanelProps['header'];
ref?: React.RefObject<HTMLDivElement>;
ref?: React.RefObject<HTMLDetailsElement>;
}

export interface CollapseProps {
Expand All @@ -42,15 +42,15 @@ export interface CollapseProps {
}

export type SemanticName = 'header' | 'title' | 'body' | 'icon';
export interface CollapsePanelProps extends React.DOMAttributes<HTMLDivElement> {
export interface CollapsePanelProps extends React.DOMAttributes<HTMLDetailsElement> {
id?: string;
header?: string | React.ReactNode;
prefixCls?: string;
headerClass?: string;
showArrow?: boolean;
className?: string;
classNames?: Partial<Record<SemanticName, string>>;
style?: object;
style?: React.CSSProperties;
styles?: Partial<Record<SemanticName, React.CSSProperties>>;
isActive?: boolean;
openMotion?: CSSMotionProps;
Expand Down
32 changes: 16 additions & 16 deletions tests/__snapshots__/index.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ exports[`collapse props items should work with nested 1`] = `
<div
class="rc-collapse"
>
<div
<details
class="rc-collapse-item rc-collapse-item-disabled"
>
<div
<summary
aria-disabled="true"
aria-expanded="false"
class="rc-collapse-header rc-collapse-collapsible-disabled"
Expand All @@ -26,12 +26,12 @@ exports[`collapse props items should work with nested 1`] = `
>
collapse 1
</span>
</div>
</div>
<div
</summary>
</details>
<details
class="rc-collapse-item"
>
<div
<summary
aria-disabled="false"
aria-expanded="false"
class="rc-collapse-header"
Expand All @@ -57,12 +57,12 @@ exports[`collapse props items should work with nested 1`] = `
ExtraSpan
</span>
</div>
</div>
</div>
<div
</summary>
</details>
<details
class="rc-collapse-item important"
>
<div
<summary
aria-disabled="false"
aria-expanded="false"
class="rc-collapse-header"
Expand All @@ -81,12 +81,12 @@ exports[`collapse props items should work with nested 1`] = `
>
collapse 3
</span>
</div>
</div>
<div
</summary>
</details>
<details
class="rc-collapse-item"
>
<div
<summary
aria-disabled="false"
aria-expanded="false"
class="rc-collapse-header"
Expand All @@ -105,7 +105,7 @@ exports[`collapse props items should work with nested 1`] = `
>
title 3
</span>
</div>
</div>
</summary>
</details>
</div>
`;