Skip to content

feat: x - #1

Open
Cuilz wants to merge 2 commits into
mainfrom
feat/x
Open

feat: x#1
Cuilz wants to merge 2 commits into
mainfrom
feat/x

Conversation

@Cuilz

@Cuilz Cuilz commented Feb 21, 2024

Copy link
Copy Markdown
Owner

No description provided.


exports[`renders ./components/auto-complete/demo/basic.md correctly 1`] = `
Array [
<div

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

components/auto-complete/tests/snapshots/demo.test.ts.snap

});

it('Test noop', async () => {
const { noop } = await import('../warning');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

是一个首屏加载的文件

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

comment this file

const getRealHeight: MotionEventHandler = node => {
const { scrollHeight } = node;
return { height: scrollHeight, opacity: 1 };
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

components/_util/motion.tsx

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Summary

This PR adds a comprehensive set of core UI components to the codebase, including buttons, cards, badges, breadcrumbs, calendars, and more. Each component includes TypeScript definitions, tests, demos, and documentation.

  • Added components/button/button.tsx with support for various button types (primary, ghost, dashed, link, text), loading states, and RTL
  • Added components/badge/index.tsx implementing badges with animations, status indicators, and ribbon variants
  • Added components/calendar/generateCalendar.tsx with customizable calendar generation supporting different date types and localization
  • Added components/breadcrumb/Breadcrumb.tsx with route-based navigation and dropdown menu support
  • Added components/card/Card.tsx featuring tabs, loading states, grid layouts and meta information

268 file(s) reviewed, 228 comment(s)
Edit PR Review Bot Settings | Greptile


const ActionButton: React.FC<ActionButtonProps> = (props) => {
const clickedRef = React.useRef<boolean>(false);
const ref = React.useRef<HTMLInputElement>(null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: Button ref type should be HTMLButtonElement instead of HTMLInputElement since this wraps a Button component

Suggested change
const ref = React.useRef<HTMLInputElement>(null);
const ref = React.useRef<HTMLButtonElement>(null);

setLoading(true);
returnValueOfOnOk!.then(
(...args: any[]) => {
setLoading(false, true);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: setLoading() is called with two arguments but useState's setState only accepts one argument

Suggested change
setLoading(false, true);
setLoading(false);

},
(e: Error) => {
// See: https://github.com/ant-design/ant-design/issues/6183
setLoading(false, true);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: setLoading() is called with two arguments but useState's setState only accepts one argument

Suggested change
setLoading(false, true);
setLoading(false);

Comment on lines +32 to +44
React.useEffect(() => {
let timeoutId: ReturnType<typeof setTimeout> | null = null;
if (props.autoFocus) {
timeoutId = setTimeout(() => {
ref.current?.focus();
});
}
return () => {
if (timeoutId) {
clearTimeout(timeoutId);
}
};
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: Missing dependency array for autoFocus prop in useEffect, could cause stale closure issues

Suggested change
React.useEffect(() => {
let timeoutId: ReturnType<typeof setTimeout> | null = null;
if (props.autoFocus) {
timeoutId = setTimeout(() => {
ref.current?.focus();
});
}
return () => {
if (timeoutId) {
clearTimeout(timeoutId);
}
};
}, []);
React.useEffect(() => {
let timeoutId: ReturnType<typeof setTimeout> | null = null;
if (props.autoFocus) {
timeoutId = setTimeout(() => {
ref.current?.focus();
});
}
return () => {
if (timeoutId) {
clearTimeout(timeoutId);
}
};
}, [props.autoFocus]);

const files = glob.sync(`./components/*/__tests__/demo.test.@(j|t)s?(x)`);

files.forEach(componentTestFile => {
const componentName = componentTestFile.match(/components\/([^/]*)\//)![1];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: Non-null assertion operator used without validation. Consider adding a check that the match exists first to avoid runtime errors

Suggested change
const componentName = componentTestFile.match(/components\/([^/]*)\//)![1];
const match = componentTestFile.match(/components\/([^/]*)\//);
if (!match) throw new Error(`Could not extract component name from ${componentTestFile}`);
const componentName = match[1];

| tabList | List of TabPane's head | Array&lt;{key: string, tab: ReactNode}> | - | |
| tabProps | [Tabs](/components/tabs/#Tabs) | - | - | |
| title | Card title | ReactNode | - | |
| type | Card style type, can be set to `inner` or not set | string | - | |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: type prop should specify literal type 'inner' instead of string since it only accepts that specific value

Comment on lines +227 to +229
&-type-inner &-extra {
padding: @card-inner-head-padding + 1.5px 0;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Adding 1.5px to padding may cause inconsistent rendering across browsers. Consider using a whole number

| bodyStyle | 内容区域自定义样式 | CSSProperties | - | |
| bordered | 是否有边框 | boolean | true | |
| cover | 卡片封面 | ReactNode | - | |
| defaultActiveTabKey | 初始化选中页签的 key,如果没有设置 activeTabKey | string | `第一个页签` | |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: defaultActiveTabKey default value should be consistent with English docs - consider using 'first tab' instead of '第一个页签'

| --- | --- | --- | --- | --- |
| className | 网格容器类名 | string | - | |
| hoverable | 鼠标移过时可浮起 | boolean | true | |
| style | 定义网格容器类名的样式 | CSSProperties | - | |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

syntax: style prop description mentions 'className' which is incorrect - should be '定义网格容器的样式'

Suggested change
| style | 定义网格容器类名的样式 | CSSProperties | - | |
| style | 定义网格容器的样式 | CSSProperties | - | |

Comment on lines +11 to +14
> .@{card-prefix-cls}-extra {
padding: @card-head-padding-sm 0;
font-size: @card-head-font-size-sm;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Consider moving font-size declaration to line 5 to avoid repeating the same value for both head and extra content

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants