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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions assets/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

@import './motion.less';

// ::view-transition-old(root), /* 旧视图*/
// ::view-transition-new(root) { /* 新视图*/
// animation-duration: 0.6s;
// }

Comment on lines +7 to +11
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] This commented-out view-transition snippet appears stale. Removing or relocating it behind a feature flag will keep the stylesheet clearer.

Suggested change
// ::view-transition-old(root), /* 旧视图*/
// ::view-transition-new(root) { /* 新视图*/
// animation-duration: 0.6s;
// }

Copilot uses AI. Check for mistakes.

#arrow {
.common() {
width: 0;
Expand Down Expand Up @@ -35,6 +40,20 @@

& > &-item {
border-top: @borderStyle;
list-style-position: outside;
interpolate-size: allow-keywords;
overflow: hidden;

&::details-content {
block-size: 0;
transition: block-size 0.6s, content-visibility 0.6s;
transition-behavior: allow-discrete;
}

&[open]::details-content {
Comment on lines +47 to +53
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.

The CSS pseudo-element ::details-content isn't part of the standard and won't target actual <details> content. Consider wrapping the panel body in a dedicated element (e.g., .rc-collapse-content) and applying transitions to that selector instead.

Suggested change
&::details-content {
block-size: 0;
transition: block-size 0.6s, content-visibility 0.6s;
transition-behavior: allow-discrete;
}
&[open]::details-content {
> .rc-collapse-content {
block-size: 0;
transition: block-size 0.6s, content-visibility 0.6s;
transition-behavior: allow-discrete;
}
&[open] > .rc-collapse-content {

Copilot uses AI. Check for mistakes.

block-size: auto;
}
Comment on lines +43 to +55
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

使用现代CSS特性实现动画过渡

代码引入了一些新的CSS特性来实现基于<details>元素的动画效果:

  • interpolate-size: allow-keywords; 允许在尺寸关键词(如auto)和具体值之间进行插值动画
  • ::details-content 伪元素选择器用于直接控制details内容区域的样式和动画
  • 使用transition-behavior: allow-discrete;支持离散过渡效果

这些CSS特性是比较新的,需要注意浏览器兼容性问题。与Panel.tsx中的特性检测(CSS.supports('selector(details::details-content)'))相配合,可以实现渐进增强的用户体验。

建议添加一些注释说明这些属性的浏览器兼容性要求,或考虑为不支持这些特性的浏览器提供兼容性样式。

🤖 Prompt for AI Agents (early access)
In assets/index.less around lines 38 to 50, the CSS uses modern features like
interpolate-size, ::details-content, and transition-behavior which have limited
browser support. Add comments explaining the browser compatibility requirements
for these properties and consider adding fallback styles or alternative
approaches for browsers that do not support these features to ensure graceful
degradation.

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
Author

Choose a reason for hiding this comment

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

兼容性如何?

这只是demo实现,ant实现上考虑下css兼容吧

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
Author

@IVLIU IVLIU Jun 16, 2025

Choose a reason for hiding this comment

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

有方案么,最好提前考虑起来。

  1. 兼容::detail-content的情况使用Panel的className编写动画,openMotion.motionName传入一个空实现
  2. 不兼容::detail-content的情况则使用openMotion.motionName编写动画

你看下是不是可行,::detail-content兼容性确实比较差

Copy link
Author

Choose a reason for hiding this comment

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

我尝试引入startTransition,编译报错,需要升级哪里吗?


&:first-child {
border-top: none;
}
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/_util/motionUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {

const getCollapsedHeight: MotionEventHandler = () => ({ height: 0, opacity: 0 });
const getRealHeight: MotionEventHandler = (node) => ({ height: node.scrollHeight, opacity: 1 });
const getCurrentHeight: MotionEventHandler = (node) => ({ height: node.offsetHeight });
const getCurrentHeight: MotionEventHandler = (node) => ({ height: node?.offsetHeight ?? 0 });
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

可选链可能触发 TS18048 编译错误

MotionEventHandler 的签名中,node 参数通常被声明为 HTMLElement,并未包含 null | undefined。在严格空检查开启的项目中,对一个非可空类型使用 ?. 会被 TypeScript 报错(TS18048: “The left-hand side of an optional chain operation does not include ‘null’ or ‘undefined’”)。

建议两种修复思路,任选其一:

  1. 保持函数签名不变,改用三元判断:
-const getCurrentHeight: MotionEventHandler = (node) => ({ height: node?.offsetHeight ?? 0 });
+const getCurrentHeight: MotionEventHandler = (node) => ({ height: node ? node.offsetHeight : 0 });
  1. 若确有传入 null/undefined 的场景,则在参数层面显式放宽类型:
-const getCurrentHeight: MotionEventHandler = (node) => ({ height: node?.offsetHeight ?? 0 });
+const getCurrentHeight: MotionEventHandler = (node?: HTMLElement | null) => ({
+  height: node?.offsetHeight ?? 0,
+});

请根据实际调用情况选择,以确保通过 CI 并避免隐藏的空值问题。

📝 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
const getCurrentHeight: MotionEventHandler = (node) => ({ height: node?.offsetHeight ?? 0 });
// Option 1: keep the signature and use a ternary check
const getCurrentHeight: MotionEventHandler = (node) => ({
height: node ? node.offsetHeight : 0,
});
Suggested change
const getCurrentHeight: MotionEventHandler = (node) => ({ height: node?.offsetHeight ?? 0 });
// Option 2: relax the parameter type to include null/undefined
const getCurrentHeight: MotionEventHandler = (node?: HTMLElement | null) => ({
height: node?.offsetHeight ?? 0,
});
🤖 Prompt for AI Agents
In docs/examples/_util/motionUtil.ts at line 9, the use of optional chaining on
the parameter 'node' causes a TS18048 error because 'node' is typed as
non-nullable HTMLElement. To fix this, either remove the optional chaining and
use a ternary check to handle potential null or undefined values while keeping
the parameter type unchanged, or update the function signature to explicitly
allow 'node' to be null or undefined if such cases occur. Choose the approach
based on actual usage to ensure type safety and pass CI.

const skipOpacityTransition: MotionEndEventHandler = (_, event) =>
(event as TransitionEvent).propertyName === 'height';

Expand Down
24 changes: 14 additions & 10 deletions src/Collapse.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,22 @@ const Collapse = React.forwardRef<HTMLDivElement, CollapseProps>((props, ref) =>
});

const onItemClick = (key: React.Key) =>
setActiveKey(() => {
if (accordion) {
return activeKey[0] === key ? [] : [key];
}
// ? 用于解决react状态与details[open]状态不一致的问题
// ? 具体参考issue 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
18 changes: 10 additions & 8 deletions src/Panel.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import classNames from 'classnames';
import CSSMotion from '@rc-component/motion';
import KeyCode from '@rc-component/util/lib/KeyCode';
import CSSMotion from '@rc-component/motion';
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,10 @@ 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.preventDefault();
e.stopPropagation();
},
onKeyDown: (e: React.KeyboardEvent) => {
if (e.key === 'Enter' || e.keyCode === KeyCode.ENTER || e.which === KeyCode.ENTER) {
Expand Down Expand Up @@ -79,16 +81,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 +100,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 +126,7 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop
);
}}
</CSSMotion>
</div>
</details>
);
});

Expand Down
8 changes: 1 addition & 7 deletions src/PanelContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,7 @@ const PanelContent = React.forwardRef<
styles,
} = props;

const [rendered, setRendered] = React.useState(isActive || forceRender);

React.useEffect(() => {
if (forceRender || isActive) {
setRendered(true);
}
}, [forceRender, isActive]);
const rendered = isActive || forceRender;

if (!rendered) {
return null;
Expand Down
7 changes: 3 additions & 4 deletions src/interface.ts
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,16 +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?: 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
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>
`;
8 changes: 1 addition & 7 deletions tests/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ describe('collapse', () => {
expect(collapse.container.querySelectorAll('.rc-collapse-panel-active')).toHaveLength(1);
fireEvent.click(header);
jest.runAllTimers();
expect(collapse.container.querySelector('.rc-collapse-panel-inactive')?.innerHTML).toBe(
'<div class="rc-collapse-body">second</div>',
);
expect(collapse.container.querySelector('.rc-collapse-panel-inactive')).toBe(null);
expect(collapse.container.querySelectorAll('.rc-collapse-panel-active').length).toBeFalsy();
});

Expand Down Expand Up @@ -433,9 +431,6 @@ describe('collapse', () => {
jest.runAllTimers();

expect(container.querySelectorAll('.rc-collapse-panel-active')).toHaveLength(0);
expect(container.querySelector('.rc-collapse-panel')!.className).not.toContain(
'rc-collapse-panel-active',
);
});

describe('wrapped in Fragment', () => {
Expand Down Expand Up @@ -510,7 +505,6 @@ describe('collapse', () => {
);
fireEvent.click(container.querySelector('.rc-collapse-header')!);
expect(container.querySelectorAll('.rc-collapse-panel-active')).toHaveLength(0);
expect(container.querySelector('.rc-collapse-panel')).toHaveClass('rc-collapse-panel-inactive');
});

describe('prop: collapsible', () => {
Expand Down