Skip to content
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

feat: add disabled api for panel #543

Merged
merged 12 commits into from
Oct 29, 2024

Conversation

aojunhao123
Copy link
Contributor

@aojunhao123 aojunhao123 commented Oct 16, 2024

Cascader底层组件rc-cascader的Panel模式支持传入disabled去禁用整个Panel区域

Summary by CodeRabbit

Summary by CodeRabbit

  • 新特性

    • Cascader.Panel 组件中添加了 disabled 属性,允许用户设置面板为非交互状态。
    • 增强了 Column 组件的功能,支持 disabled 属性,以更好地管理选项的禁用状态。
    • 示例中添加了一个按钮,允许用户动态切换面板的禁用状态。
  • 测试

    • Cascader.Panel 组件添加了新的测试用例,以验证禁用状态的行为。

Copy link

vercel bot commented Oct 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cascader ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 29, 2024 11:07am

Copy link

coderabbitai bot commented Oct 16, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

此拉取请求引入了对多个组件的更改,主要是为 Cascader.Panel 组件添加了一个新的 disabled 属性,以表示该面板为非交互式。同时,Column 组件的 ColumnProps 接口也更新,以支持新的 disabled 属性,影响选项的交互逻辑。其他相关组件的类型和逻辑也进行了相应的调整,以确保新的禁用状态在整个组件中一致地处理。此外,还为 Cascader.Panel 组件添加了新的测试用例,以验证禁用状态的正确性。

Changes

文件路径 更改摘要
examples/panel.tsx 添加 Cascader.Paneldisabled 属性,创建一个新的非交互式面板实例,并添加按钮以切换禁用状态。
src/OptionList/Column.tsx ColumnProps 接口中添加 disabled 属性,更新 Column 组件以使用该属性,修改选项交互逻辑。
src/OptionList/List.tsx 更新 RawOptionListProps 类型,添加 disabled 属性并修改选择逻辑以考虑该属性。
src/Panel.tsx 更新 PickTypePanelProps 类型,添加 disabled 属性,确保 RawOptionList 组件可以利用该属性。
tests/Panel.spec.tsx Cascader.Panel 组件添加新的测试用例,验证在 disabled 属性为 true 时的行为及样式。

Poem

在兔子洞里欢声笑,
新属性来真是妙。
面板禁用不再忙,
选项交互更轻松。
代码更新如春风,
兔子跳跃乐无穷! 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@aojunhao123 aojunhao123 changed the title feat/panel disabled feat: add disabled for panel Oct 16, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
examples/panel.tsx (1)

95-95: 新增的禁用状态面板示例很好,建议稍作改进

这个新增的禁用状态的 Cascader.Panel 实例很好地展示了新功能的使用。它与 PR 的目标一致,为开发者提供了清晰的使用示例。

为了进一步提高示例的清晰度,建议添加一个注释来解释这个禁用状态的面板的用途。例如:

+      {/* 展示禁用状态的 Cascader.Panel */}
       <Cascader.Panel disabled options={addressOptions} />

这样可以帮助其他开发者更快地理解这个示例的目的。

tests/Panel.spec.tsx (1)

101-112: 测试用例覆盖了禁用状态的基本功能,但可以进一步改进

这个测试用例很好地验证了 Cascader.Panel 组件的禁用状态。它检查了禁用类的存在,并确保点击选项不会触发 onChange 回调。然而,我们可以通过以下方式使测试更加全面:

  1. 验证所有选项都是非交互式的,而不仅仅是第一个选项。
  2. 检查选项在悬停或点击时的视觉状态是否保持不变。
  3. 验证是否正确设置了无障碍性所需的 ARIA 属性。

建议扩展测试用例以涵盖这些方面,从而提高测试的全面性和可靠性。例如:

it('disabled', () => {
  const onChange = jest.fn();
  const { container } = render(
    <Cascader.Panel options={options} onChange={onChange} disabled={true} />,
  );

  expect(container.querySelector('.rc-cascader-panel-disabled')).toBeTruthy();

  // 检查所有选项
  const allOptions = container.querySelectorAll('.rc-cascader-menu-item');
  allOptions.forEach((option) => {
    fireEvent.click(option);
    expect(onChange).not.toHaveBeenCalled();
    
    // 检查悬停状态
    fireEvent.mouseEnter(option);
    expect(option).not.toHaveClass('rc-cascader-menu-item-active');
    
    // 检查 ARIA 属性
    expect(option).toHaveAttribute('aria-disabled', 'true');
  });
});

这个扩展的测试用例将提供更全面的覆盖,确保禁用状态在各个方面都正确实现。

src/Panel.tsx (1)

72-72: 正确解构 disabled 属性

props 中解构 disabled 属性是正确的做法,这与之前在 PickType 中添加 disabled 相对应。这允许 Panel 组件接收和使用 disabled 属性。

建议:为了提高代码的可读性,可以考虑为 disabled 属性添加一个默认值,例如:

disabled = false,

这样可以明确指出该属性的默认行为。

src/OptionList/List.tsx (1)

35-44: 重命名 'disabled' 为 'panelDisabled' 以提高可读性

disabled 属性重命名为 panelDisabled 有助于明确其用途,提高了代码的可读性。这与之前在 RawOptionListProps 类型中的更改保持一致。

建议:为了保持一致性,考虑在 RawOptionListProps 类型定义中也使用 panelDisabled 而不是 disabled

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 4b44dba and 3f41343.

📒 Files selected for processing (5)
  • examples/panel.tsx (1 hunks)
  • src/OptionList/Column.tsx (7 hunks)
  • src/OptionList/List.tsx (4 hunks)
  • src/Panel.tsx (3 hunks)
  • tests/Panel.spec.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (14)
src/Panel.tsx (3)

38-39: 新增 disabled 属性符合需求

PickType 类型添加 disabled 属性是正确的。这个改动使得 PanelProps 类型能够包含 disabled 属性,从而实现了禁用整个面板区域的功能。这与 PR 的目标一致。


205-205: 正确传递 disabled 属性到 RawOptionList

disabled 属性传递给 RawOptionList 组件是正确的做法。这确保了禁用状态可以正确地应用到选项列表中。

建议:请确保 RawOptionList 组件已正确实现了对 disabled 属性的处理。可以通过以下步骤进行验证:

  1. 检查 RawOptionList 组件的实现,确保它正确使用了 disabled 属性。
  2. 添加或更新相关的单元测试,以验证 disabled 属性在 RawOptionList 中的行为。
  3. 进行手动测试,确保当 Panel 被禁用时,RawOptionList 中的所有选项都无法交互。

Line range hint 1-209: 总结:成功实现了 Panel 的禁用功能

本次更改成功地为 Panel 组件添加了 disabled 属性,实现了禁用整个面板区域的功能。主要修改包括:

  1. PickType 中添加 disabled 属性。
  2. Panel 组件中解构并使用 disabled 属性。
  3. disabled 属性传递给 RawOptionList 组件。

这些更改符合 PR 的目标,提高了 Cascader 组件的灵活性和可用性。建议进行充分的测试,特别是 RawOptionList 组件中 disabled 属性的行为,以确保功能正确实现。

src/OptionList/List.tsx (4)

24-31: 新增 'disabled' 属性以支持面板禁用功能

这个更改通过在 RawOptionListProps 类型中添加 disabled 属性,为 RawOptionList 组件引入了禁用整个面板的功能。这与 PR 的目标一致,增强了组件的灵活性。


119-123: 更新 isSelectable 函数以支持面板级禁用

isSelectable 函数的更新确保了当面板被禁用时(panelDisabled 为 true),所有选项都不可选择。这个改动正确地实现了面板级禁用功能,与 PR 的目标一致。

这个更改有效地阻止了用户在面板被禁用时与选项进行交互,提高了组件的可用性和一致性。


223-223: 将 panelDisabled 状态传递给 Column 组件

通过将 panelDisabled 添加到 columnProps 对象中,确保了禁用状态可以正确地传递给 Column 组件。这个更改是实现面板级禁用功能的必要步骤,与之前的修改保持一致。


241-241: 将 panelDisabled 属性传递给 Column 组件

这个更改确保了 panelDisabled 状态被正确地传递给 Column 组件,这对于在整个组件层次结构中实现面板级禁用功能是必要的。这与之前的修改保持一致,完成了禁用功能的实现。

建议:验证 Column 组件是否正确处理了 panelDisabled 属性,以确保禁用功能在所有层级都能正常工作。

src/OptionList/Column.tsx (7)

26-26: 新增属性 panelDisabled 有助于控制面板的禁用状态

ColumnProps 接口中添加了可选属性 panelDisabled?: boolean;,实现了对整个面板禁用状态的控制,增强了组件的灵活性。


42-42: 正确地解构了新的参数 panelDisabled

在函数组件的参数列表中添加 panelDisabled,确保组件内部可以访问并使用该属性。


59-60: 引入 isOptionDisabled 函数,统一禁用状态判断

定义了 isOptionDisabled = (optionDisabled?: boolean) => panelDisabled || optionDisabled;,统一了面板和选项的禁用状态判断逻辑,提升了代码的可读性和维护性。


122-122: triggerOpenPath 中正确应用了禁用状态检查

使用 isOptionDisabled(disabled) 代替原先的 disabled 判断,确保当面板或选项被禁用时,不会触发打开路径的逻辑。


134-134: 更新了选择条件,考虑了面板的禁用状态

triggerSelect 函数中,条件更新为 if (isSelectable(option) && !isOptionDisabled(disabled)),确保只有在面板未禁用且选项可选择的情况下,才会执行选择操作。


155-155: 调整了菜单项的禁用样式判断

在类名计算中,使用 isOptionDisabled(disabled),确保当面板或选项被禁用时,菜单项正确应用禁用样式。


192-192: 确保复选框在禁用状态下不可交互

在复选框组件中,将 disabled 属性设置为 isOptionDisabled(disabled) || disableCheckbox,保证当面板或选项被禁用时,复选框也被禁用,防止用户进行交互。

@aojunhao123 aojunhao123 changed the title feat: add disabled for panel feat: add disabled api for panel Oct 17, 2024
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.83%. Comparing base (4b44dba) to head (7691896).
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #543   +/-   ##
=======================================
  Coverage   99.83%   99.83%           
=======================================
  Files          23       23           
  Lines         611      614    +3     
  Branches      185      177    -8     
=======================================
+ Hits          610      613    +3     
  Misses          1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 59 to 60
const isOptionDisabled = (optionDisabled?: boolean) => panelDisabled || optionDisabled;

Copy link
Contributor

Choose a reason for hiding this comment

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

上次我尝试解决,也是 props 透传,感觉很丑陋。 或许可以用 context 的方式, 我也拿捏不准

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯嗯好的,我再去看一下

@@ -100,10 +116,11 @@ const RawOptionList = React.forwardRef<RefOptionListProps, RawOptionListProps>((
};

const isSelectable = (option: DefaultOptionType) => {
const { disabled } = option;
if (panelDisabled) return false;
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 Author

Choose a reason for hiding this comment

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

这个拆行指的是👀

src/OptionList/List.tsx Outdated Show resolved Hide resolved
src/OptionList/List.tsx Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 243125a and ef96118.

📒 Files selected for processing (3)
  • examples/panel.tsx (3 hunks)
  • src/OptionList/Column.tsx (7 hunks)
  • src/OptionList/List.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/OptionList/Column.tsx
  • src/OptionList/List.tsx
🔇 Additional comments (1)
examples/panel.tsx (1)

Line range hint 64-82: 状态管理和按钮实现完善!

状态管理使用了 React 的 useState Hook,切换按钮的实现简洁明了,并且按钮文本会根据状态动态更新,符合 React 最佳实践。

<Cascader.Panel
value={value}
options={addressOptions}
onChange={nextValue => {
console.log('Change:', nextValue);
setValue(nextValue);
}}
disabled={disabled}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议提取共同的 Panel 属性

多个 Cascader.Panel 组件都使用了相同的 disabled 属性。建议将共同的属性提取为一个对象,以提高代码的可维护性和减少重复。

+ const commonPanelProps = {
+   disabled,
+   options: addressOptions,
+ };

  return (
    <>
      {/* ... */}
      <Cascader.Panel
-       value={value}
-       options={addressOptions}
-       onChange={nextValue => {
-         console.log('Change:', nextValue);
-         setValue(nextValue);
-       }}
-       disabled={disabled}
+       {...commonPanelProps}
+       value={value}
+       onChange={nextValue => {
+         console.log('Change:', nextValue);
+         setValue(nextValue);
+       }}
      />

      <Cascader.Panel
+       {...commonPanelProps}
        checkable
        value={value2}
-       options={addressOptions}
-       disabled={disabled}
        onChange={nextValue => {
          console.log('Change:', nextValue);
          setValue2(nextValue);
        }}
      />

-     <Cascader.Panel options={addressOptions} disabled={disabled} direction="rtl" />
+     <Cascader.Panel {...commonPanelProps} direction="rtl" />

Also applies to: 101-101, 104-104

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
src/OptionList/Column.tsx (2)

59-59: 函数实现简洁明了!

isOptionDisabled 函数很好地处理了全局和选项级别的禁用状态。不过建议添加 JSDoc 注释来说明函数的用途。

建议添加如下注释:

+/** 
+ * 判断选项是否被禁用
+ * @param optionDisabled - 选项级别的禁用状态
+ * @returns 是否禁用
+ */
const isOptionDisabled = (optionDisabled?: boolean) => disabled || optionDisabled;

Line range hint 155-192: 建议增强无障碍性支持

为了提高组件的可访问性,建议:

  1. 添加适当的 aria-disabled 属性
  2. 确保键盘导航在禁用状态下的正确行为
  3. 添加适当的颜色对比度以提高视觉可访问性

建议添加如下属性:

 <li
   key={fullPathKey}
   className={classNames(menuItemPrefixCls, {
     [`${menuItemPrefixCls}-expand`]: !isMergedLeaf,
     [`${menuItemPrefixCls}-active`]: activeValue === value || activeValue === fullPathKey,
     [`${menuItemPrefixCls}-disabled`]: isOptionDisabled(optionDisabled),
     [`${menuItemPrefixCls}-loading`]: isLoading,
   })}
   style={dropdownMenuColumnStyle}
   role="menuitemcheckbox"
   title={title}
   aria-checked={checked}
+  aria-disabled={isOptionDisabled(optionDisabled)}
   data-path-key={fullPathKey}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between ef96118 and 9201339.

⛔ Files ignored due to path filters (1)
  • tests/__snapshots__/search.spec.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • src/OptionList/Column.tsx (9 hunks)
🔇 Additional comments (3)
src/OptionList/Column.tsx (3)

26-26: 接口变更看起来不错!

disabled 属性的类型定义清晰且符合 TypeScript 最佳实践。


65-65: 选项信息映射的修改合理!

解构和属性传递的方式保持了代码的一致性,命名清晰。

Also applies to: 87-87


122-124: 建议考虑使用 Context 优化禁用状态的传递

目前的实现通过属性传递(prop drilling)来处理禁用状态,这种方式虽然可行,但可能导致代码维护性降低。考虑使用 React Context 来管理禁用状态,这样可以:

  1. 减少属性传递链
  2. 提高代码可维护性
  3. 使组件之间的耦合度降低

让我们看看是否有其他组件也在传递 disabled 属性:

Also applies to: 134-136, 155-155, 192-192

src/OptionList/List.tsx Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 9201339 and 393749a.

📒 Files selected for processing (1)
  • src/OptionList/List.tsx (2 hunks)
🔇 Additional comments (2)
src/OptionList/List.tsx (2)

24-31: 类型定义更新正确

类型定义中添加 disabled 属性的方式符合 TypeScript 最佳实践。


35-44: 属性解构合理

从 props 中解构 disabled 的方式正确,且保持了字母顺序。

Comment on lines +119 to +126
if (disabled) {
return false;
}

const { disabled: optionDisabled } = option;
const isMergedLeaf = isLeaf(option, fieldNames);
return !disabled && (isMergedLeaf || changeOnSelect || multiple);

return !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议优化 isSelectable 函数的实现

当前实现逻辑正确,但可以更加简洁。建议合并条件判断,减少代码行数。

建议如下修改:

 const isSelectable = (option: DefaultOptionType) => {
-  if (disabled) {
-    return false;
-  }
-
   const { disabled: optionDisabled } = option;
   const isMergedLeaf = isLeaf(option, fieldNames);
-
-  return !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
+  return !disabled && !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
 };
📝 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
if (disabled) {
return false;
}
const { disabled: optionDisabled } = option;
const isMergedLeaf = isLeaf(option, fieldNames);
return !disabled && (isMergedLeaf || changeOnSelect || multiple);
return !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
const { disabled: optionDisabled } = option;
const isMergedLeaf = isLeaf(option, fieldNames);
return !disabled && !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);

@zombieJ zombieJ merged commit 3d53b0f into react-component:master Oct 29, 2024
10 of 11 checks passed
@aojunhao123 aojunhao123 deleted the feat/panel-disabled branch October 29, 2024 11:16
lihongxiangfrontend pushed a commit to lihongxiangfrontend/cascader that referenced this pull request Nov 19, 2024
* feat: add disabled api for panel

* feat: add test case

* docs: add demo

* fix: test case fixed

* feat: adjust demo position

* feat: improve demo

* chore: reorganize code for better readability

* test: update snapshot

* fix: lint fix

* chore: adjust code structure

* test: update snapshot

* chore: code optimization

---------

Co-authored-by: 小豪 <aojunhao@cai-inc.com>
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.

3 participants