Skip to content

feat: 增加extra #483

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 5 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
18 changes: 13 additions & 5 deletions assets/index/Dialog.less
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
font-weight: bold;
}

&-extra {
padding-right: 36px;
}

&-section {
position: relative;
background-color: #ffffff;
Expand All @@ -43,15 +47,15 @@
color: #000;
text-shadow: 0 1px 0 #fff;
filter: alpha(opacity=20);
opacity: .2;
opacity: 0.2;
text-decoration: none;

&:disabled {
pointer-events: none;
}

&-x:after {
content: '×'
content: '×';
}

&:hover {
Expand All @@ -67,6 +71,9 @@
background: #fff;
color: #666;
border-bottom: 1px solid #e9e9e9;
display: flex;
align-items: center;
justify-content: space-between;
}

&-body {
Expand All @@ -85,7 +92,8 @@
animation-fill-mode: both;
}

&-zoom-enter, &-zoom-appear {
&-zoom-enter,
&-zoom-appear {
opacity: 0;
.effect();
animation-timing-function: cubic-bezier(0.08, 0.82, 0.17, 1);
Expand All @@ -98,7 +106,8 @@
animation-play-state: paused;
}

&-zoom-enter&-zoom-enter-active, &-zoom-appear&-zoom-appear-active {
&-zoom-enter&-zoom-enter-active,
&-zoom-appear&-zoom-appear-active {
animation-name: rcDialogZoomIn;
animation-play-state: running;
}
Expand All @@ -120,7 +129,6 @@
}
@keyframes rcDialogZoomOut {
0% {

transform: scale(1, 1);
}
100% {
Expand Down
1 change: 1 addition & 0 deletions docs/examples/ant-design.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const MyControl: React.FC = () => {
onClose={onClose}
style={style}
title="dialog1"
extra={<a onClick={onClick}>click me</a>}
mousePosition={mousePosition}
destroyOnHidden={destroyOnHidden}
closeIcon={useIcon ? getSvg(clearPath, {}, true) : undefined}
Expand Down
20 changes: 19 additions & 1 deletion src/Dialog/Content/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const Panel = React.forwardRef<PanelRef, PanelProps>((props, ref) => {
height,
classNames: modalClassNames,
styles: modalStyles,
extra,
} = props;

// ================================= Refs =================================
Expand Down Expand Up @@ -97,6 +98,15 @@ const Panel = React.forwardRef<PanelRef, PanelProps>((props, ref) => {
</div>
) : null;

const extraNode = extra ? (
<div
style={{ ...modalStyles?.extra }}
className={classNames(`${prefixCls}-extra`, modalClassNames?.extra)}
>
{extra}
</div>
) : null;

const headerNode = title ? (
<div
className={classNames(`${prefixCls}-header`, modalClassNames?.header)}
Expand All @@ -109,8 +119,16 @@ const Panel = React.forwardRef<PanelRef, PanelProps>((props, ref) => {
>
{title}
</div>
{extraNode}
</div>
) : null;
) : (
<div
className={classNames(`${prefixCls}-header`, modalClassNames?.header)}
style={{ ...modalStyles?.header }}
>
{extraNode}
</div>
);

const closableObj = useMemo(() => {
if (typeof closable === 'object' && closable !== null) {
Expand Down
6 changes: 6 additions & 0 deletions src/Dialog/Content/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,17 @@ const Content = React.forwardRef<ContentRef, ContentProps>((props, ref) => {
ariaId,
onVisibleChanged,
mousePosition,
extra,
} = props;

const dialogRef = useRef<{ nativeElement: HTMLElement } & CSSMotionStateRef>(null);

const panelRef = useRef<PanelRef>(null);

const hasExtra = !!extra && !(typeof extra === 'string' && extra.trim() === '');

const curExtra = hasExtra ? extra : null;

// ============================== Refs ==============================
React.useImperativeHandle(ref, () => ({
...panelRef.current,
Expand Down Expand Up @@ -77,6 +82,7 @@ const Content = React.forwardRef<ContentRef, ContentProps>((props, ref) => {
<Panel
{...props}
ref={panelRef}
extra={curExtra}
title={title}
ariaId={ariaId}
prefixCls={prefixCls}
Expand Down
12 changes: 11 additions & 1 deletion src/IDialogPropTypes.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import type { GetContainer } from '@rc-component/util/lib/PortalWrapper';
import type { CSSProperties, ReactNode, SyntheticEvent } from 'react';

export type SemanticName = 'header' | 'body' | 'footer' | 'section' | 'title' | 'wrapper' | 'mask';
export type SemanticName =
| 'header'
| 'body'
| 'footer'
| 'section'
| 'title'
| 'wrapper'
| 'mask'
| 'extra';

export type ModalClassNames = Partial<Record<SemanticName, string>>;

Expand Down Expand Up @@ -55,4 +63,6 @@ export type IDialogPropTypes = {

// Refs
panelRef?: React.Ref<HTMLDivElement>;

extra?: ReactNode;
};
3 changes: 3 additions & 0 deletions tests/__snapshots__/index.spec.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ exports[`dialog add rootClassName and rootStyle should render correct 1`] = `
class="rc-dialog-close-x"
/>
</button>
<div
class="rc-dialog-header"
/>
<div
class="rc-dialog-body"
/>
Expand Down
38 changes: 37 additions & 1 deletion tests/index.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable react/no-render-return-value, max-classes-per-file, func-names, no-console */
import { fireEvent, render, act } from '@testing-library/react';
import { fireEvent, render, act, screen } from '@testing-library/react';
import { Provider } from '@rc-component/motion';
import KeyCode from '@rc-component/util/lib/KeyCode';
import React, { cloneElement, useEffect } from 'react';
Expand Down Expand Up @@ -733,4 +733,40 @@ describe('dialog', () => {
expect(document.querySelector('.rc-dialog')).toBeTruthy();
expect(document.querySelector('.rc-dialog-close')).toBeFalsy();
});

it('should render extra when extra is a React node', () => {
render(<Dialog visible extra={<span data-testid="extra-node">Node</span>} />);

expect(screen.getByTestId('extra-node')).toBeInTheDocument();
});

it('does not render extra when extra is empty string', () => {
render(<Dialog visible extra="" />);
expect(screen.queryByTestId('.rc-dialog-extra')).toBeNull();
});
Comment on lines +743 to +746
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

修复测试选择器错误

测试逻辑正确,但选择器使用有误。screen.queryByTestId 应该传入测试 ID,而不是 CSS 选择器。

-    expect(screen.queryByTestId('.rc-dialog-extra')).toBeNull();
+    expect(document.querySelector('.rc-dialog-extra')).toBeNull();
🤖 Prompt for AI Agents
In tests/index.spec.tsx around lines 743 to 746, the test uses
screen.queryByTestId with a CSS selector string '.rc-dialog-extra' instead of
the actual test ID. Fix this by passing the correct test ID string without any
CSS selector syntax to screen.queryByTestId to properly query the element by its
test ID.


it('does not render extra when extra is string with only spaces', () => {
render(<Dialog visible extra=" " />);
expect(screen.queryByText(' ')).toBeNull();
});
Comment on lines +748 to +751
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

测试逻辑需要改进

测试空格字符串的场景很有用,但当前的断言方法不够准确。应该检查 DOM 中是否存在 .rc-dialog-extra 元素。

-    expect(screen.queryByText('   ')).toBeNull();
+    expect(document.querySelector('.rc-dialog-extra')).toBeNull();
📝 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
it('does not render extra when extra is string with only spaces', () => {
render(<Dialog visible extra=" " />);
expect(screen.queryByText(' ')).toBeNull();
});
it('does not render extra when extra is string with only spaces', () => {
render(<Dialog visible extra=" " />);
- expect(screen.queryByText(' ')).toBeNull();
+ expect(document.querySelector('.rc-dialog-extra')).toBeNull();
});
🤖 Prompt for AI Agents
In tests/index.spec.tsx around lines 748 to 751, the test currently checks for
absence of the string with spaces but does not accurately verify if the extra
content is rendered. Update the test to check that the DOM does not contain any
element with the class `.rc-dialog-extra` when extra is a string of only spaces,
ensuring the extra content is truly not rendered.


it('renders extra when extra is non-empty string', () => {
render(<Dialog visible extra="hello" />);
expect(screen.getByText('hello')).toBeInTheDocument();
const extraDiv = document.querySelector('.rc-dialog-extra');
expect(extraDiv).toHaveTextContent('hello');
});

it('does not render extra when extra is null or undefined', () => {
const { container } = render(<Dialog visible extra={null} />);
expect(container.querySelector('.rc-dialog-extra')).toBeNull();

const { container: container2 } = render(<Dialog visible />);
expect(container2.querySelector('.rc-dialog-extra')).toBeNull();
});

it('renders extra when extra is a non-empty string', () => {
render(<Dialog visible extra="Extra Text" />);
expect(screen.getByText('Extra Text')).toBeInTheDocument();
});
Comment on lines +768 to +771
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

删除重复的测试用例

这个测试用例与第753-758行的测试用例功能重复,都是测试非空字符串的渲染。建议删除以避免冗余。

-  it('renders extra when extra is a non-empty string', () => {
-    render(<Dialog visible extra="Extra Text" />);
-    expect(screen.getByText('Extra Text')).toBeInTheDocument();
-  });
📝 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
it('renders extra when extra is a non-empty string', () => {
render(<Dialog visible extra="Extra Text" />);
expect(screen.getByText('Extra Text')).toBeInTheDocument();
});
🤖 Prompt for AI Agents
In tests/index.spec.tsx around lines 768 to 771, there is a test case that
duplicates the functionality of the test case between lines 753 to 758, both
checking rendering of a non-empty string in the extra prop. Remove the test case
at lines 768 to 771 to eliminate redundancy and keep the test suite concise.

});