Skip to content

feat: add disabled api for panel #543

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

Merged
merged 12 commits into from
Oct 29, 2024
Merged
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
13 changes: 12 additions & 1 deletion examples/panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ export default () => {

const [value2, setValue2] = React.useState<string[][]>([]);

const [disabled, setDisabled] = React.useState(false);

return (
<>
<h1>Panel</h1>
Expand All @@ -71,13 +73,21 @@ export default () => {
>
Set Value
</button>
<button
onClick={() => {
setDisabled(prev => !prev);
}}
>
{disabled ? 'enable panel' : 'disable panel'}
</button>
<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

/>

<Cascader.Panel
Expand All @@ -88,9 +98,10 @@ export default () => {
console.log('Change:', nextValue);
setValue2(nextValue);
}}
disabled={disabled}
/>

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

<Cascader.Panel notFoundContent="Empty!!!" />
</>
Expand Down
12 changes: 8 additions & 4 deletions src/OptionList/Column.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface ColumnProps<OptionType extends DefaultOptionType = DefaultOptio
halfCheckedSet: Set<React.Key>;
loadingKeys: React.Key[];
isSelectable: (option: DefaultOptionType) => boolean;
disabled?: boolean;
}

export default function Column<OptionType extends DefaultOptionType = DefaultOptionType>({
Expand All @@ -38,6 +39,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt
halfCheckedSet,
loadingKeys,
isSelectable,
disabled: propsDisabled,
}: ColumnProps<OptionType>) {
const menuPrefixCls = `${prefixCls}-menu`;
const menuItemPrefixCls = `${prefixCls}-menu-item`;
Expand All @@ -54,6 +56,8 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt

const hoverOpen = expandTrigger === 'hover';

const isOptionDisabled = (disabled?: boolean) => propsDisabled || disabled;

// ============================ Option ============================
const optionInfoList = React.useMemo(
() =>
Expand Down Expand Up @@ -115,7 +119,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt
}) => {
// >>>>> Open
const triggerOpenPath = () => {
if (disabled) {
if (isOptionDisabled(disabled)) {
return;
}
const nextValueCells = [...fullPath];
Expand All @@ -127,7 +131,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt

// >>>>> Selection
const triggerSelect = () => {
if (isSelectable(option)) {
if (isSelectable(option) && !isOptionDisabled(disabled)) {
onSelect(fullPath, isMergedLeaf);
}
};
Expand All @@ -148,7 +152,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt
[`${menuItemPrefixCls}-expand`]: !isMergedLeaf,
[`${menuItemPrefixCls}-active`]:
activeValue === value || activeValue === fullPathKey,
[`${menuItemPrefixCls}-disabled`]: disabled,
[`${menuItemPrefixCls}-disabled`]: isOptionDisabled(disabled),
[`${menuItemPrefixCls}-loading`]: isLoading,
})}
style={dropdownMenuColumnStyle}
Expand Down Expand Up @@ -185,7 +189,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt
prefixCls={`${prefixCls}-checkbox`}
checked={checked}
halfChecked={halfChecked}
disabled={disabled || disableCheckbox}
disabled={isOptionDisabled(disabled) || disableCheckbox}
disableCheckbox={disableCheckbox}
onClick={(e: React.MouseEvent<HTMLSpanElement>) => {
if (disableCheckbox) {
Expand Down
28 changes: 24 additions & 4 deletions src/OptionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,27 @@ import useKeyboard from './useKeyboard';

export type RawOptionListProps = Pick<
ReturnType<typeof useBaseProps>,
'prefixCls' | 'multiple' | 'searchValue' | 'toggleOpen' | 'notFoundContent' | 'direction' | 'open'
| 'prefixCls'
| 'multiple'
| 'searchValue'
| 'toggleOpen'
| 'notFoundContent'
| 'direction'
| 'open'
| 'disabled'
>;

const RawOptionList = React.forwardRef<RefOptionListProps, RawOptionListProps>((props, ref) => {
const { prefixCls, multiple, searchValue, toggleOpen, notFoundContent, direction, open } = props;
const {
prefixCls,
multiple,
searchValue,
toggleOpen,
notFoundContent,
direction,
open,
disabled,
} = props;

const containerRef = React.useRef<HTMLDivElement>(null);
const rtl = direction === 'rtl';
Expand Down Expand Up @@ -100,10 +116,14 @@ const RawOptionList = React.forwardRef<RefOptionListProps, RawOptionListProps>((
};

const isSelectable = (option: DefaultOptionType) => {
const { disabled } = option;
if (disabled) {
return false;
}

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

return !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
Comment on lines +119 to +126
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);

};

const onPathSelect = (valuePath: SingleValueType, leaf: boolean, fromKeyboard = false) => {
Expand Down
5 changes: 4 additions & 1 deletion src/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ export type PickType =
| 'className'
| 'style'
| 'direction'
| 'notFoundContent';
| 'notFoundContent'
| 'disabled';

export type PanelProps<
OptionType extends DefaultOptionType = DefaultOptionType,
Expand Down Expand Up @@ -68,6 +69,7 @@ export default function Panel<
loadingIcon,
direction,
notFoundContent = 'Not Found',
disabled,
} = props as Pick<InternalCascaderProps, PickType>;

// ======================== Multiple ========================
Expand Down Expand Up @@ -200,6 +202,7 @@ export default function Panel<
toggleOpen={noop}
open
direction={direction}
disabled={disabled}
/>
)}
</div>
Expand Down
13 changes: 13 additions & 0 deletions tests/Panel.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,17 @@ describe('Cascader.Panel', () => {
const checkedLi = container.querySelector('[aria-checked="true"]');
expect(checkedLi?.textContent).toEqual('Little');
});

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

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

const selectOption = container.querySelector('.rc-cascader-menu-item')!;
fireEvent.click(selectOption);
expect(onChange).not.toHaveBeenCalled();
});
});
Loading