Skip to content

Commit

Permalink
fix(popover): voiceover closes when focusing an item fixed
Browse files Browse the repository at this point in the history
  • Loading branch information
jrgarciadev committed Aug 20, 2023
1 parent 445d8a5 commit 800f148
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 30 deletions.
8 changes: 6 additions & 2 deletions packages/components/popover/__tests__/popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,15 @@ describe("Popover", () => {
expect(onClose).toHaveBeenCalledTimes(1);
});

it("should hide the popover on blur ", () => {
it("should hide the popover on blur (shouldCloseOnBlur=true)", () => {
const onClose = jest.fn();

const wrapper = render(
<Popover isOpen onOpenChange={(isOpen) => (!isOpen ? onClose() : undefined)}>
<Popover
isOpen
shouldCloseOnBlur
onOpenChange={(isOpen) => (!isOpen ? onClose() : undefined)}
>
<PopoverTrigger>
<button>Open popover</button>
</PopoverTrigger>
Expand Down
3 changes: 2 additions & 1 deletion packages/components/popover/src/popover-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const PopoverContent = forwardRef<"div", PopoverContentProps>((props, _) => {
getArrowProps,
getDialogProps,
getBackdropProps,
isNonModal,
onClose,
} = usePopoverContext();

Expand All @@ -55,7 +56,7 @@ const PopoverContent = forwardRef<"div", PopoverContentProps>((props, _) => {

const content = (
<>
<DismissButton onDismiss={onClose} />
{!isNonModal && <DismissButton onDismiss={onClose} />}
<Component {...getDialogProps(mergeProps(dialogProps, otherProps))} ref={dialogRef}>
{typeof children === "function" ? children(titleProps) : children}
{arrowContent}
Expand Down
21 changes: 11 additions & 10 deletions packages/components/popover/src/use-aria-popover.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {RefObject, useEffect} from "react";
import {AriaPopoverProps, useOverlay, PopoverAria, useOverlayPosition} from "@react-aria/overlays";
import {
AriaPopoverProps,
useOverlay,
PopoverAria,
useOverlayPosition,
AriaOverlayProps,
} from "@react-aria/overlays";
import {OverlayPlacement, toReactAriaPlacement, ariaHideOutside} from "@nextui-org/aria-utils";
import {OverlayTriggerState} from "@react-stately/overlays";
import {mergeProps} from "@react-aria/utils";
Expand All @@ -22,7 +28,7 @@ export interface Props {
scrollRef?: RefObject<HTMLElement>;
}

export type ReactAriaPopoverProps = Props & Omit<AriaPopoverProps, "placement">;
export type ReactAriaPopoverProps = Props & Omit<AriaPopoverProps, "placement"> & AriaOverlayProps;

/**
* Provides the behavior and accessibility implementation for a popover component.
Expand All @@ -41,6 +47,7 @@ export function useReactAriaPopover(
scrollRef,
shouldFlip,
boundaryElement,
shouldCloseOnBlur = false,
placement: placementProp = "top",
containerPadding,
isNonModal: isNonModalProp,
Expand All @@ -54,15 +61,9 @@ export function useReactAriaPopover(
{
isOpen: state.isOpen,
onClose: state.close,
shouldCloseOnBlur: true,
isDismissable: !isNonModal,
shouldCloseOnBlur,
isDismissable: isNonModal,
isKeyboardDismissDisabled,
shouldCloseOnInteractOutside(element) {
// Don't close if the click is within the trigger or the popover itself
let trigger = triggerRef?.current;

return !trigger || !trigger.contains(element);
},
},
popoverRef,
);
Expand Down
12 changes: 7 additions & 5 deletions packages/components/popover/src/use-popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {getArrowPlacement, getShouldUseAxisPlacement} from "@nextui-org/aria-uti
import {popover} from "@nextui-org/theme";
import {mergeProps, mergeRefs} from "@react-aria/utils";
import {clsx, dataAttr} from "@nextui-org/shared-utils";
import {useId, useMemo, useCallback, useRef} from "react";
import {useMemo, useCallback, useRef} from "react";

import {useReactAriaPopover, ReactAriaPopoverProps} from "./use-aria-popover";

Expand Down Expand Up @@ -86,9 +86,11 @@ export function usePopover(originalProps: UsePopoverProps) {
isOpen,
defaultOpen,
onOpenChange,
isNonModal = true,
shouldFlip = true,
containerPadding = 12,
shouldBlockScroll = false,
shouldCloseOnBlur,
portalContainer,
placement: placementProp = "top",
triggerType = "dialog",
Expand All @@ -105,7 +107,6 @@ export function usePopover(originalProps: UsePopoverProps) {
} = props;

const Component = as || "div";
const popoverId = useId();

const domRef = useDOMRef(ref);

Expand Down Expand Up @@ -136,10 +137,12 @@ export function usePopover(originalProps: UsePopoverProps) {
} = useReactAriaPopover(
{
triggerRef,
isNonModal,
popoverRef: domRef,
placement: placementProp,
offset: offset,
scrollRef,
shouldCloseOnBlur,
boundaryElement,
crossOffset,
shouldFlip,
Expand Down Expand Up @@ -167,7 +170,6 @@ export function usePopover(originalProps: UsePopoverProps) {
ref: domRef,
...mergeProps(popoverProps, otherProps, props),
style: mergeProps(popoverProps.style, otherProps.style, props.style),
id: popoverId,
});

const getDialogProps: PropGetter = (props = {}) => ({
Expand All @@ -191,14 +193,13 @@ export function usePopover(originalProps: UsePopoverProps) {
const getTriggerProps = useCallback<PropGetter>(
(props = {}, _ref: Ref<any> | null | undefined = null) => {
return {
"aria-controls": popoverId,
"aria-haspopup": "dialog",
...mergeProps(triggerProps, props),
className: slots.trigger({class: clsx(classNames?.trigger, props.className)}),
ref: mergeRefs(_ref, triggerRef),
};
},
[isOpen, popoverId, state, triggerProps, triggerRef],
[isOpen, state, triggerProps, triggerRef],
);

const getBackdropProps = useCallback<PropGetter>(
Expand Down Expand Up @@ -233,6 +234,7 @@ export function usePopover(originalProps: UsePopoverProps) {
showArrow,
triggerRef,
placement,
isNonModal,
portalContainer,
isOpen: state.isOpen,
onClose: state.close,
Expand Down
16 changes: 8 additions & 8 deletions packages/components/select/src/select.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {Listbox} from "@nextui-org/listbox";
import {Popover, PopoverContent, PopoverTrigger} from "@nextui-org/popover";
import {ScrollShadow} from "@nextui-org/scroll-shadow";
import {ChevronDownIcon} from "@nextui-org/shared-icons";
import {Spinner} from "@nextui-org/spinner";
import {forwardRef} from "@nextui-org/system";
import {FocusScope} from "@react-aria/focus";
// import {FocusScope} from "@react-aria/focus";
import {ScrollShadow} from "@nextui-org/scroll-shadow";
import {VisuallyHidden} from "@react-aria/visually-hidden";
import {cloneElement, ForwardedRef, ReactElement, Ref, useMemo} from "react";

Expand Down Expand Up @@ -109,19 +109,19 @@ function Select<T extends object>(props: Props<T>, ref: ForwardedRef<HTMLSelectE
{startContent}
<span {...getValueProps()}>
{renderSelectedItem}
<VisuallyHidden>,</VisuallyHidden>
{state.selectedItems && <VisuallyHidden>,</VisuallyHidden>}
</span>
{endContent}
</div>
{renderIndicator}
</Component>
</PopoverTrigger>
<PopoverContent>
<FocusScope contain restoreFocus>
<ScrollShadow {...getListboxWrapperProps()}>
<Listbox {...getListboxProps()} />
</ScrollShadow>
</FocusScope>
{/* <FocusScope contain restoreFocus> */}
<ScrollShadow {...getListboxWrapperProps()}>
<Listbox {...getListboxProps()} />
</ScrollShadow>
{/* </FocusScope> */}
</PopoverContent>
</Popover>
{helperWrapper}
Expand Down
12 changes: 9 additions & 3 deletions packages/components/select/src/use-select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ export function useSelect<T extends object>(originalProps: UseSelectProps<T>) {
renderValue,
onSelectionChange,
placeholder,
disallowEmptySelection = true,
selectionMode = "single",
spinnerRef,
scrollRef: scrollRefProp,
Expand Down Expand Up @@ -207,6 +208,7 @@ export function useSelect<T extends object>(originalProps: UseSelectProps<T>) {
...props,
isOpen,
selectionMode,
disallowEmptySelection,
children: children as CollectionChildren<T>,
isRequired: originalProps?.isRequired,
isDisabled: originalProps?.isDisabled,
Expand All @@ -231,7 +233,11 @@ export function useSelect<T extends object>(originalProps: UseSelectProps<T>) {
});

const {labelProps, triggerProps, valueProps, menuProps, descriptionProps, errorMessageProps} =
useMultiSelect({...props, isDisabled: originalProps?.isDisabled}, state, triggerRef);
useMultiSelect(
{...props, disallowEmptySelection, isDisabled: originalProps?.isDisabled},
state,
triggerRef,
);

const {isPressed, buttonProps} = useAriaButton(triggerProps, triggerRef);

Expand Down Expand Up @@ -416,7 +422,7 @@ export function useSelect<T extends object>(originalProps: UseSelectProps<T>) {
className: slots.listbox({
class: clsx(classNames?.listbox, props?.className),
}),
...mergeProps(menuProps, userListboxProps, props),
...mergeProps(userListboxProps, props, menuProps),
} as ListboxProps;
};

Expand All @@ -427,7 +433,7 @@ export function useSelect<T extends object>(originalProps: UseSelectProps<T>) {
triggerRef,
ref: popoverRef,
scrollRef: listboxRef,

triggerType: "listbox",
className: slots.popover({
class: clsx(classNames?.popover, props.className),
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/hooks/use-aria-multiselect/src/use-multiselect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export function useMultiSelect<T>(
}
};

// Typeahead functionality - imitating default `<select>` behaviour.
// Typeahead functionality - imitating default `<select>` behavior.
const {typeSelectProps} = useTypeSelect({
keyboardDelegate: delegate,
selectionManager: state.selectionManager,
Expand Down

0 comments on commit 800f148

Please sign in to comment.