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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add native input DOM to follow standard behavior of HTML #1018

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: add native input DOM to follow standard behavior of HTML #1018

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
82e600c
feat: add native input DOM to follow standard behavior of HTML
nnmax Jan 5, 2024
0a8c1d7
test: update snapshots
nnmax Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Expand Up @@ -132,6 +132,7 @@ export default () => (
| optionRender | Custom rendering options | (oriOption: FlattenOptionData\<BaseOptionType\> , info: { index: number }) => React.ReactNode | - |
| labelRender | Custom rendering label | (props: LabelInValueType) => React.ReactNode | - |
| maxCount | The max number of items can be selected | number | - |
| nativeInputProps | Passing props to the native input | `React.InputHTMLAttributes<HTMLInputElement>` | - |

### Methods

Expand Down
3 changes: 3 additions & 0 deletions src/BaseSelect.tsx
View file
Open in desktop
Expand Up @@ -133,6 +133,7 @@ export interface BaseSelectProps extends BaseSelectPrivateProps, React.AriaAttri
tagRender?: (props: CustomTagProps) => React.ReactElement;
direction?: 'ltr' | 'rtl';
maxLength?: number;
nativeInputProps?: React.InputHTMLAttributes<HTMLInputElement>;

// MISC
tabIndex?: number;
Expand Down Expand Up @@ -219,6 +220,7 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref)
tagRender,
direction,
omitDomProps,
nativeInputProps,

// Value
displayValues,
Expand Down Expand Up @@ -792,6 +794,7 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref)
onSearchSubmit={onInternalSearchSubmit}
onRemove={onSelectorRemove}
tokenWithEnter={tokenWithEnter}
nativeInputProps={nativeInputProps}
/>
)}
</SelectTrigger>
Expand Down
33 changes: 33 additions & 0 deletions src/SelectNativeInput.tsx
View file
Open in desktop
@@ -0,0 +1,33 @@
import classNames from 'classnames';
import React from 'react';

const visuallyHidden: React.CSSProperties = {
position: 'absolute',
overflow: 'hidden',
width: 1,
height: 1,
border: 0,
margin: -1,
padding: 0,
clip: 'rect(0 0 0 0)',
whiteSpace: 'nowrap',
};

interface SelectNativeInputProps extends React.InputHTMLAttributes<HTMLInputElement> {
prefixCls?: string;
}

export default React.forwardRef<HTMLInputElement, SelectNativeInputProps>(
function SelectNativeInput(props, ref) {
const { prefixCls, className, style, ...rest } = props;

return (
<input
ref={ref}
className={classNames(`${prefixCls}-native-input`, className)}
style={{ ...visuallyHidden, ...style }}
{...rest}
/>
);
},
);
12 changes: 12 additions & 0 deletions src/Selector/MultipleSelector.tsx
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@ import Input from './Input';
import useLayoutEffect from '../hooks/useLayoutEffect';
import type { DisplayValueType, RenderNode, CustomTagProps, RawValueType } from '../BaseSelect';
import { getTitle } from '../utils/commonUtil';
import SelectNativeInput from '../SelectNativeInput';

function itemKey(value: DisplayValueType) {
return value.key ?? value.value;
Expand Down Expand Up @@ -42,6 +43,7 @@ const SelectSelector: React.FC<SelectorProps> = (props) => {
const {
id,
prefixCls,
nativeInputProps,

values,
open,
Expand Down Expand Up @@ -232,9 +234,19 @@ const SelectSelector: React.FC<SelectorProps> = (props) => {
/>
);

const selectNativeInputValue = values
.filter((item) => item.value !== null || item.value !== undefined)
.map((item) => String(item.value))
.join(',');

return (
<>
{selectionNode}
<SelectNativeInput
value={selectNativeInputValue}
prefixCls={prefixCls}
{...nativeInputProps}
/>
{!values.length && !inputValue && (
<span className={`${selectionPrefixCls}-placeholder`}>{placeholder}</span>
)}
Expand Down
8 changes: 8 additions & 0 deletions src/Selector/SingleSelector.tsx
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@ import pickAttrs from 'rc-util/lib/pickAttrs';
import Input from './Input';
import type { InnerSelectorProps } from '.';
import { getTitle } from '../utils/commonUtil';
import SelectNativeInput from '../SelectNativeInput';

interface SelectorProps extends InnerSelectorProps {
inputElement: React.ReactElement;
Expand All @@ -24,6 +25,7 @@ const SingleSelector: React.FC<SelectorProps> = (props) => {
values,
placeholder,
tabIndex,
nativeInputProps,

showSearch,
searchValue,
Expand Down Expand Up @@ -106,6 +108,12 @@ const SingleSelector: React.FC<SelectorProps> = (props) => {
/>
</span>

<SelectNativeInput
Copy link
Member

Choose a reason for hiding this comment

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

There exist a input for search usage. Seems that one should be aria hidden?

Copy link
Author

Choose a reason for hiding this comment

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

The role of the search input is a combobox, which contains many aria-* attributes that should not be aria hidden.

<SelectNativeInput /> is already aria hidden.

Copy link
Member

Choose a reason for hiding this comment

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

Why make SelectNativeInput aria hidden?

Copy link
Author

Choose a reason for hiding this comment

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

I've changed the code:

export default React.forwardRef<HTMLInputElement, SelectNativeInputProps>(
  function SelectNativeInput(props, ref) {
    const { prefixCls, className, ...rest } = props;

    return (
      <input
-       aria-hidden
-       tabIndex={-1}
        type="hidden"
-       readOnly
        ref={ref}
        className={classNames(`${prefixCls}-native-input`, className)}
        {...rest}
      />
    );
  },
);

Thank you. You're right. aria-hidden shouldn't be set. But type="hidden" was already set. The screen reader still can't read it.

Should we remove the type='hidden' and add some accessibility attributes to make it accessible to screen readers?

Copy link
Member

Choose a reason for hiding this comment

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

I guess could use opacity: 0 + 0 width & height to let it invisible on the screen but still workable with reader.

Copy link
Author

Choose a reason for hiding this comment

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

done

import classNames from 'classnames';
import React from 'react';

+ const visuallyHidden: React.CSSProperties = {
+   position: 'absolute',
+   overflow: 'hidden',
+   width: 1,
+   height: 1,
+   border: 0,
+   margin: -1,
+   padding: 0,
+   clip: 'rect(0 0 0 0)',
+   whiteSpace: 'nowrap',
+ };

interface SelectNativeInputProps extends React.InputHTMLAttributes<HTMLInputElement> {
  prefixCls?: string;
}

export default React.forwardRef<HTMLInputElement, SelectNativeInputProps>(
  function SelectNativeInput(props, ref) {
    const { prefixCls, className, style, ...rest } = props;

    return (
      <input
-       type="hidden"
        ref={ref}
        className={classNames(`${prefixCls}-native-input`, className)}
+       style={{ ...visuallyHidden, ...style }}
        {...rest}
      />
    );
  },
);

value={item?.value}
prefixCls={prefixCls}
{...nativeInputProps}
/>

{/* Display value */}
{!combobox && item ? (
<span
Expand Down
4 changes: 3 additions & 1 deletion src/Selector/index.tsx
View file
Open in desktop
Expand Up @@ -15,14 +15,15 @@ import type { ScrollTo } from 'rc-virtual-list/lib/List';
import MultipleSelector from './MultipleSelector';
import SingleSelector from './SingleSelector';
import useLock from '../hooks/useLock';
import type { CustomTagProps, DisplayValueType, Mode, RenderNode } from '../BaseSelect';
import type { BaseSelectProps, CustomTagProps, DisplayValueType, Mode, RenderNode } from '../BaseSelect';
import { isValidateOpenKey } from '../utils/keyUtil';

export interface InnerSelectorProps {
prefixCls: string;
id: string;
mode: Mode;
title?: string;
nativeInputProps?: BaseSelectProps['nativeInputProps']

inputRef: React.Ref<HTMLInputElement | HTMLTextAreaElement>;
placeholder?: React.ReactNode;
Expand Down Expand Up @@ -65,6 +66,7 @@ export interface SelectorProps {
autoClearSearchValue: boolean;
inputElement: JSX.Element;
maxLength?: number;
nativeInputProps?: BaseSelectProps['nativeInputProps']

autoFocus?: boolean;
activeDescendantId?: string;
Expand Down
10 changes: 5 additions & 5 deletions tests/Accessibility.test.tsx
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@ describe('Select.Accessibility', () => {
it('pass aria info to internal input', () => {
const MySelect = Select as any;
const wrapper = mount(<MySelect aria-label="light" data-attr="bamboo" useless="2333" />);
expect(wrapper.find('input').props()).toEqual(
expect(wrapper.find('.rc-select-selection-search-input').props()).toEqual(
expect.objectContaining({
'aria-label': 'light',
}),
Expand Down Expand Up @@ -48,20 +48,20 @@ describe('Select.Accessibility', () => {
);

// First Match
wrapper.find('input').simulate('change', { target: { value: 'b' } });
wrapper.find('.rc-select-selection-search-input').simulate('change', { target: { value: 'b' } });
jest.runAllTimers();

expectOpen(wrapper);
expect(
wrapper.find('.rc-select-item-option-active .rc-select-item-option-content').text(),
).toEqual('Bamboo');

wrapper.find('input').simulate('keyDown', { which: KeyCode.ENTER });
wrapper.find('.rc-select-selection-search-input').simulate('keyDown', { which: KeyCode.ENTER });
expectOpen(wrapper, false);

// Next Match
wrapper.find('input').simulate('change', { target: { value: '' } });
wrapper.find('input').simulate('change', { target: { value: 'g' } });
wrapper.find('.rc-select-selection-search-input').simulate('change', { target: { value: '' } });
wrapper.find('.rc-select-selection-search-input').simulate('change', { target: { value: 'g' } });
jest.runAllTimers();

expectOpen(wrapper);
Expand Down