Skip to content

Conversation

@HaixingOoO
Copy link
Collaborator

🤔 这个 PR 的性质是?

  • 日常 bug 修复
  • 新特性提交
  • 文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • CI/CD 改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他

🔗 相关 Issue

💡 需求背景和解决方案

Cascader support virtual scroll api

📝 更新日志

  • feat(Cascader): 支持虚拟滚动 scroll 配置

  • 本条 PR 不需要纳入 Changelog

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • Changelog 已提供或无须提供

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 19, 2025

tdesign-react-demo

npm i https://pkg.pr.new/tdesign-react@3901

commit: cf5c09f

@github-actions
Copy link
Contributor

完成

@RylanBot RylanBot requested a review from Copilot October 20, 2025 09:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds virtual scroll support to Cascader via a new scroll prop, refactors panel rendering to a List component with virtualization, and updates docs and examples.

  • Introduces scroll?: TScroll to Cascader props and passes it through Cascader/CascaderPanel to Panel/List.
  • Adds PanelContext and a new List component using useListVirtualScroll; adjusts markup structure accordingly.
  • Updates docs (CN/EN) and adds a virtual-scroll example; updates CSR/SSR snapshots.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
packages/components/cascader/type.ts Adds scroll?: TScroll prop to Cascader API.
packages/components/cascader/context.ts New PanelContext to share cascaderContext, trigger, option, scroll.
packages/components/cascader/components/Panel.tsx Refactors to use List and provide PanelContext.
packages/components/cascader/components/List.tsx New virtualized list implementation using useListVirtualScroll.
packages/components/cascader/Cascader.tsx Passes scroll prop, adjusts updateScrollTop behavior when virtual scrolling.
packages/components/cascader/CascaderPanel.tsx Forwards scroll to Panel.
packages/components/cascader/_example/virtual-scroll.tsx New demo showing virtual scroll usage.
packages/components/cascader/cascader.md Documents new scroll prop (CN).
packages/components/cascader/cascader.en-US.md Documents new scroll prop (EN).
test/snap/snapshots/* Updates CSR/SSR snapshots to reflect structural changes and new example.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +3 to +9
import { CascaderPanelProps } from './components/Panel';

export const PanelContext = createContext<{
cascaderContext: CascaderContextType;
trigger: CascaderPanelProps['trigger'];
option: CascaderPanelProps['option'];
scroll: CascaderPanelProps['scroll'];
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

context.ts imports CascaderPanelProps from Panel.tsx, and Panel.tsx imports PanelContext from context.ts, creating a circular dependency risk at runtime. Avoid value imports for types here and decouple from Panel by switching to type-only import from the prop source (e.g., TdCascaderProps) or asserting non-value type import. Suggested fix: remove the CascaderPanelProps import and use import type { TdCascaderProps } from './type', then reference TdCascaderProps['trigger' | 'option' | 'scroll'] in the context type.

Suggested change
import { CascaderPanelProps } from './components/Panel';
export const PanelContext = createContext<{
cascaderContext: CascaderContextType;
trigger: CascaderPanelProps['trigger'];
option: CascaderPanelProps['option'];
scroll: CascaderPanelProps['scroll'];
import type { TdCascaderProps } from './type';
export const PanelContext = createContext<{
cascaderContext: CascaderContextType;
trigger: TdCascaderProps['trigger'];
option: TdCascaderProps['option'];
scroll: TdCascaderProps['scroll'];

Copilot uses AI. Check for mistakes.
Comment on lines +84 to +86
const handleScroll = (event: React.WheelEvent<HTMLDivElement>): void => {
if (isVirtualScroll) onInnerVirtualScroll(event as unknown as globalThis.WheelEvent);
};
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

onScroll handlers receive UIEvent, not WheelEvent; typing this as React.WheelEvent and casting to globalThis.WheelEvent is incorrect and may break virtualization logic that relies on wheel delta. Use an onWheel handler and pass event.nativeEvent directly to onInnerVirtualScroll.

Copilot uses AI. Check for mistakes.
return (
<div
ref={panelWrapperRef}
onScroll={handleScroll}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

This binds the wheel-based virtualization handler to the scroll event. Bind the corrected wheel handler instead, e.g., onWheel={handleWheel}, to ensure delta-based virtualization works as designed.

Suggested change
onScroll={handleScroll}
onWheel={handleScroll}

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +41
const onScrollIntoView = useEventCallback(() => {
const checkedNodes = ctx.cascaderContext.treeStore.getCheckedNodes();
let lastCheckedNodes = checkedNodes[checkedNodes.length - 1];
let index = -1;
if (lastCheckedNodes?.level === level) {
index = treeNodes.findLastIndex((item) => item.value === lastCheckedNodes.value);
} else {
while (lastCheckedNodes) {
if (lastCheckedNodes?.level === level) {
// eslint-disable-next-line no-loop-func
index = treeNodes.findIndex((item) => item.value === lastCheckedNodes.value);
break;
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Array.prototype.findLastIndex is not supported in all target environments and can break in older browsers; replace with a reverse loop to compute the last index match for better compatibility.

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +28
const { treeNodes, isFilter = false, segment = true, listKey: key, level = 0 } = props;
const ctx = usePanelContext();
const panelWrapperRef = useRef<HTMLDivElement>(null);
const { classPrefix } = useConfig();
const COMPONENT_NAME = `${classPrefix}-cascader`;

const { virtualConfig, cursorStyle, listStyle, isVirtualScroll, onInnerVirtualScroll, scrollToElement } =
useListVirtualScroll(ctx.scroll, panelWrapperRef, treeNodes);
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

usePanelContext() may return null per its context initializer; dereferencing ctx.scroll without a non-null assertion or guard can cause type/runtime issues under strictNullChecks. Use a non-null assertion (const ctx = usePanelContext()!) or add an early return/assertion to guarantee non-null.

Copilot uses AI. Check for mistakes.

const updateScrollTop = (content: HTMLDivElement) => {
// virtual scroll not trigger event
if (props.scroll) return;
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

This disables updateScrollTop for any scroll config, but the intent appears to be only when using virtual scrolling; if scroll also configures lazy loading, this change will unintentionally suppress the alignment behavior. Check the scroll type instead, e.g., if (props.scroll?.type === 'virtual') return;.

Suggested change
if (props.scroll) return;
if (props.scroll?.type === 'virtual') return;

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +10
for (let i = 1; i < 100; i++) {
const children = [];
for (let j = 1; j < 100; j++) {
const child = [];
for (let k = 1; k < 100; k++) {
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

This creates ~970k nodes (99×99×99), which is very heavy for demos/tests and can cause excessive memory/CPU usage even if virtualized rendering is enabled. Reduce the data size (e.g., 30×30×30 or less) or switch to a lazy-loading example to illustrate virtualization without constructing such a large tree eagerly.

Suggested change
for (let i = 1; i < 100; i++) {
const children = [];
for (let j = 1; j < 100; j++) {
const child = [];
for (let k = 1; k < 100; k++) {
for (let i = 1; i < 30; i++) {
const children = [];
for (let j = 1; j < 30; j++) {
const child = [];
for (let k = 1; k < 30; k++) {

Copilot uses AI. Check for mistakes.
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