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/droprdown accessibility #48499

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

Feat/droprdown accessibility #48499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2e2b3e8
feat: add needed aria attributes to 'menu' and 'dropdownTrigger'
KirilCycle Apr 15, 2024
55aaebf
feat: aria-expanded value depednds on menu visibility
KirilCycle Apr 15, 2024
d5be51b
open dropdown menu on 'space'and 'enter' keypress
KirilCycle Apr 15, 2024
f920a09
fix: dropdown menu on first item focus works with a tag inside list item
KirilCycle Apr 16, 2024
bdfc21a
fix: make 'menuLabel' required & remove temType='button' from'Interna…
KirilCycle Apr 16, 2024
c246375
fix: remove menu ref & use 'onInnerOpenChange' to open the menu on ke…
KirilCycle Apr 17, 2024
223f5cb
fix: use keyDown event instead of onFocus to open menu
KirilCycle Apr 17, 2024
c4bf6bf
fix: focus will move to last focused item
KirilCycle Apr 17, 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
43 changes: 27 additions & 16 deletions components/dropdown/dropdown.tsx
View file
Open in desktop
Expand Up @@ -43,7 +43,8 @@ export type DropdownArrowOptions = {
};

export interface DropdownProps {
menu?: MenuProps;
menu: MenuProps;
menuLabel?: string;
Copy link
Author

Choose a reason for hiding this comment

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

Can i make this prop as required ? Each menu must have unique label, as i use it to bound menu btn and menu itself

autoFocus?: boolean;
arrow?: boolean | DropdownArrowOptions;
trigger?: ('click' | 'hover' | 'contextMenu')[];
Expand Down Expand Up @@ -96,6 +97,7 @@ const Dropdown: CompoundedComponent = (props) => {
overlayStyle,
open,
onOpenChange,
menuLabel = Math.random(),
// Deprecated
visible,
onVisibleChange,
Expand Down Expand Up @@ -177,6 +179,17 @@ const Dropdown: CompoundedComponent = (props) => {

const child = React.Children.only(children) as React.ReactElement<any>;

// =========================== Open ============================
const [mergedOpen, setOpen] = useMergedState(false, {
value: open ?? visible,
});

const onInnerOpenChange = useEvent((nextOpen: boolean) => {
onOpenChange?.(nextOpen, { source: 'trigger' });
onVisibleChange?.(nextOpen);
setOpen(nextOpen);
});

const dropdownTrigger = cloneElement(child, {
className: classNames(
`${prefixCls}-trigger`,
Expand All @@ -186,6 +199,16 @@ const Dropdown: CompoundedComponent = (props) => {
child.props.className,
),
disabled,
onKeyDown: (e: KeyboardEvent) => {
if (e.key === 'Enter' || e.code === 'Space') {
onInnerOpenChange(true)
};
},
id: `${menuLabel}-button`,
role: 'button',
'aria-haspopup': true,
'aria-expanded': mergedOpen,
'aria-controls': `${menuLabel}-menu`,
});

const triggerActions = disabled ? [] : trigger;
Expand All @@ -194,17 +217,6 @@ const Dropdown: CompoundedComponent = (props) => {
alignPoint = true;
}

// =========================== Open ============================
const [mergedOpen, setOpen] = useMergedState(false, {
value: open ?? visible,
});

const onInnerOpenChange = useEvent((nextOpen: boolean) => {
onOpenChange?.(nextOpen, { source: 'trigger' });
onVisibleChange?.(nextOpen);
setOpen(nextOpen);
});

// =========================== Overlay ============================
const overlayClassNameCustomized = classNames(
overlayClassName,
Expand Down Expand Up @@ -238,7 +250,9 @@ const Dropdown: CompoundedComponent = (props) => {

let overlayNode: React.ReactNode;
if (menu?.items) {
overlayNode = <Menu {...menu} />;
overlayNode = (
<Menu aria-labelledby={`${menuLabel}-button`} id={`${menuLabel}-menu`} {...menu} />
);
} else if (typeof overlay === 'function') {
overlayNode = overlay();
} else {
Expand Down Expand Up @@ -276,10 +290,8 @@ const Dropdown: CompoundedComponent = (props) => {
</OverrideProvider>
);
};

// =========================== zIndex ============================
const [zIndex, contextZIndex] = useZIndex('Dropdown', overlayStyle?.zIndex as number);

// ============================ Render ============================
let renderNode = (
<RcDropdown
Expand Down Expand Up @@ -324,7 +336,6 @@ function postPureProps(props: DropdownProps) {
},
};
}

// We don't care debug panel
const PurePanel = genPurePanel(Dropdown, 'dropdown', (prefixCls) => prefixCls, postPureProps);

Expand Down
19 changes: 19 additions & 0 deletions components/menu/index.tsx
View file
Open in desktop
Expand Up @@ -54,6 +54,25 @@ const Menu = forwardRef<MenuRef, MenuProps>((props, ref) => {
menu: menuRef.current,
focus: (options) => {
menuRef.current?.focus(options);
let itemToFocus = null

if (menuRef.current?.list) {
const allListItems = menuRef.current.list.querySelectorAll('li');
allListItems.forEach((listItem,i) => {
if (listItem?.className.includes('active')) {
itemToFocus = menuRef.current?.list.querySelector(`li:nth-child(${i + 1})`)
}
});
}

if (!itemToFocus) {
itemToFocus = menuRef.current?.list.querySelector('li:first-child')
}

if (itemToFocus) {
const firstClickableChild = itemToFocus.querySelector('a');
firstClickableChild?.focus()
}
},
}));
return <InternalMenu ref={menuRef} {...props} {...context} />;
Expand Down