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.

Sign up for GitHub

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

Jump to bottom

feat: improve a11y behavior #354

Merged
merged 2 commits into from
Oct 28, 2024
Merged

feat: improve a11y behavior #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6af214c
feat: improve a11y behavior
afc163 Oct 28, 2024
9387e53
feat: improve a11y behavior
afc163 Oct 28, 2024
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
4 changes: 2 additions & 2 deletions assets/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
margin: 0 16px 0 auto;
}
}
.@{prefixCls}-header-collapsible-only {
.@{prefixCls}-collapsible-header {
cursor: default;
.@{prefixCls}-header-text {
cursor: pointer;
Expand All @@ -68,7 +68,7 @@
cursor: pointer;
}
}
.@{prefixCls}-icon-collapsible-only {
.@{prefixCls}-collapsible-icon {
cursor: default;
.@{prefixCls}-expand-icon {
cursor: pointer;
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"@rc-component/father-plugin": "^1.0.1",
"@testing-library/jest-dom": "^6.1.4",
"@testing-library/react": "^14.1.2",
"@testing-library/user-event": "^14.5.2",
"@types/classnames": "^2.2.9",
"@types/jest": "^29.4.0",
"@types/react": "^18.0.0",
Expand Down
63 changes: 28 additions & 35 deletions src/Panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,37 +29,39 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop
} = props;

const disabled = collapsible === 'disabled';
const collapsibleHeader = collapsible === 'header';
const collapsibleIcon = collapsible === 'icon';

const ifExtraExist = extra !== null && extra !== undefined && typeof extra !== 'boolean';

const handleItemClick = () => {
onItemClick?.(panelKey!);
};

const handleKeyDown = (e: React.KeyboardEvent) => {
if (e.key === 'Enter' || e.keyCode === KeyCode.ENTER || e.which === KeyCode.ENTER) {
handleItemClick();
}
const collapsibleProps = {
onClick: () => {
onItemClick?.(panelKey);
},
onKeyDown: (e: React.KeyboardEvent) => {
if (e.key === 'Enter' || e.keyCode === KeyCode.ENTER || e.which === KeyCode.ENTER) {
onItemClick?.(panelKey);
}
},
role: accordion ? 'tab' : 'button',
['aria-expanded']: isActive,
['aria-disabled']: disabled,
tabIndex: disabled ? -1 : 0,
};

// ======================== Icon ========================
let iconNode = typeof expandIcon === 'function' ? expandIcon(props) : <i className="arrow" />;
if (iconNode) {
iconNode = (
<div
className={`${prefixCls}-expand-icon`}
onClick={['header', 'icon'].includes(collapsible as string) ? handleItemClick : undefined}
>
{iconNode}
</div>
);
}
const iconNodeInner =
typeof expandIcon === 'function' ? expandIcon(props) : <i className="arrow" />;
const iconNode = iconNodeInner && (
<div
className={`${prefixCls}-expand-icon`}
{...(['header', 'icon'].includes(collapsible) ? collapsibleProps : {})}
>
{iconNodeInner}
</div>
);

const collapsePanelClassNames = classNames(
`${prefixCls}-item`,
{
[`${prefixCls}-item`]: true,
[`${prefixCls}-item-active`]: isActive,
[`${prefixCls}-item-disabled`]: disabled,
},
Expand All @@ -68,37 +70,28 @@ const CollapsePanel = React.forwardRef<HTMLDivElement, CollapsePanelProps>((prop

const headerClassName = classNames(
headerClass,
`${prefixCls}-header`,
{
[`${prefixCls}-header`]: true,
[`${prefixCls}-header-collapsible-only`]: collapsibleHeader,
[`${prefixCls}-icon-collapsible-only`]: collapsibleIcon,
[`${prefixCls}-collapsible-${collapsible}`]: !!collapsible,
},
customizeClassNames.header,
);

// ======================== HeaderProps ========================
const headerProps: React.HTMLAttributes<HTMLDivElement> = {
className: headerClassName,
'aria-expanded': isActive,
'aria-disabled': disabled,
onKeyDown: handleKeyDown,
style: styles.header,
role: accordion ? 'tab' : 'button',
...(['header', 'icon'].includes(collapsible) ? {} : collapsibleProps),
};

if (!collapsibleHeader && !collapsibleIcon) {
headerProps.onClick = handleItemClick;
headerProps.tabIndex = disabled ? -1 : 0;
}

// ======================== Render ========================
return (
<div {...resetProps} ref={ref} className={collapsePanelClassNames}>
<div {...headerProps}>
{showArrow && iconNode}
<span
className={`${prefixCls}-header-text`}
onClick={collapsible === 'header' ? handleItemClick : undefined}
{...(collapsible === 'header' ? collapsibleProps : {})}
Comment on lines 83 to +94
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

需要扩展无障碍测试覆盖范围

代码已经实现了基本的无障碍属性（aria-expanded、aria-disabled、role等），但测试覆盖度不够全面：

  • 仅测试了基础的属性存在性
  • 缺少键盘交互测试
  • 缺少不同 collapsible 模式下的无障碍行为测试

建议：

  • 添加键盘导航测试用例（Enter、Space 键触发）
  • 为不同的 collapsible 模式添加专门的无障碍测试
  • 添加 ARIA 属性在不同状态下的动态变化测试
🔗 Analysis chain

建议进行无障碍功能测试

无障碍属性的实现看起来不错，但建议：

  1. 使用屏幕阅读器进行实际测试
  2. 验证不同 collapsible 模式下的朗读体验
  3. 确保键盘导航的流畅性
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否有相关的无障碍测试用例

# 搜索测试文件中的无障碍相关测试
rg -l "aria-|role=|keyboard|screenreader|a11y" --type ts --type tsx | grep -i "test"

Length of output: 112

Script:

#!/bin/bash
# 搜索测试文件中的无障碍相关测试，修正文件类型参数
rg -l "aria-|role=|keyboard|screenreader|a11y" -g "*.test.*" -g "*.spec.*"

# 检查组件中的无障碍属性实现
rg "aria-|role=" src/Panel.tsx -A 2

Length of output: 374

Script:

#!/bin/bash
# 检查测试文件中的具体无障碍测试内容
rg "aria-|role=|keyboard|screenreader|a11y" tests/index.spec.tsx -A 3

# 检查快照文件中的无障碍属性实现
rg "aria-|role" tests/__snapshots__/index.spec.tsx.snap -A 2

Length of output: 1076

>
{header}
</span>
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/index.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`collapse props items should work with nested 1`] = `
<div
aria-disabled="true"
aria-expanded="false"
class="rc-collapse-header"
class="rc-collapse-header rc-collapse-collapsible-disabled"
role="button"
tabindex="-1"
>
Expand Down