-
Notifications
You must be signed in to change notification settings - Fork 28
feat: improve keyboard operation accessibility #285
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
Changes from 2 commits
b727013
c3e086c
78bb227
2f7630e
429831f
28be306
fbd0d56
f12a903
913982b
6817d51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import classNames from 'classnames'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import useMergedState from 'rc-util/lib/hooks/useMergedState'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import KeyCode from 'rc-util/lib/KeyCode'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import omit from 'rc-util/lib/omit'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { composeRef } from 'rc-util/lib/ref'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as React from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -84,6 +85,9 @@ const InternalSegmentedOption: React.FC<{ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| e: React.ChangeEvent<HTMLInputElement>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value: SegmentedRawOption, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onFocus: () => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onBlur: () => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onKeyDown: (e: React.KeyboardEvent) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }> = ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| prefixCls, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -94,11 +98,16 @@ const InternalSegmentedOption: React.FC<{ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onFocus, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onBlur, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onKeyDown, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (disabled) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Do not add focus style when clicking | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onBlur(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
aojunhao123 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange(event, value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -111,11 +120,13 @@ const InternalSegmentedOption: React.FC<{ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <input | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name={name} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className={`${prefixCls}-item-input`} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| aria-hidden="true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type="radio" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| disabled={disabled} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| checked={checked} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={handleChange} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onFocus={onFocus} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onBlur={onBlur} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onKeyDown={onKeyDown} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className={`${prefixCls}-item-label`} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -176,10 +187,68 @@ const Segmented = React.forwardRef<HTMLDivElement, SegmentedProps>( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const divProps = omit(restProps, ['children']); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ======================= Focus ======================== | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [isFocused, setIsFocused] = React.useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const currentIndex = segmentedOptions.findIndex( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (option) => option.value === rawValue, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleFocus = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsFocused(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleBlur = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsFocused(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ======================= Keyboard ======================== | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const getNextIndex = (current: number, offset: number) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (disabled) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return current; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const total = segmentedOptions.length; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nextIndex = (current + offset + total) % total; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (segmentedOptions[nextIndex]?.disabled) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return getNextIndex(nextIndex, offset); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nextIndex; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const getNextIndex = (current: number, offset: number) => { | |
| if (disabled) { | |
| return current; | |
| } | |
| const total = segmentedOptions.length; | |
| const nextIndex = (current + offset + total) % total; | |
| if (segmentedOptions[nextIndex]?.disabled) { | |
| return getNextIndex(nextIndex, offset); | |
| } | |
| return nextIndex; | |
| }; | |
| const getNextIndex = (current: number, offset: number) => { | |
| if (disabled) { | |
| return current; | |
| } | |
| const total = segmentedOptions.length; | |
| if (segmentedOptions.every(option => option.disabled)) { | |
| return current; | |
| } | |
| const nextIndex = (current + offset + total) % total; | |
| if (segmentedOptions[nextIndex]?.disabled) { | |
| return getNextIndex(nextIndex, offset); | |
| } | |
| return nextIndex; | |
| }; |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
使用 event.key 或 event.keyCode 替代已弃用的 event.which
在 handleKeyDown 函数中,event.which 属性已被弃用,不再推荐使用。建议改用 event.key 或 event.keyCode 来检测按键事件,以提高代码的兼容性和未来的可维护性。
建议的修改:
-switch (event.which) {
+switch (event.keyCode) {
case KeyCode.LEFT:
case KeyCode.UP:
event.preventDefault();
offset = -1;
break;
case KeyCode.RIGHT:
case KeyCode.DOWN:
event.preventDefault();
offset = 1;
break;
default:
return;
}或使用 event.key:
-switch (event.which) {
+switch (event.key) {
case 'ArrowLeft':
case 'ArrowUp':
event.preventDefault();
offset = -1;
break;
case 'ArrowRight':
case 'ArrowDown':
event.preventDefault();
offset = 1;
break;
default:
return;
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
手动调用 onBlur() 可能导致意外行为
在
handleChange函数中手动调用onBlur()可能会导致与正常的事件流不一致,可能引发意外的焦点丢失或其他副作用。建议重新评估这种调用的必要性,或者寻找更符合 React 事件处理规范的方法。