Skip to content

Commit

Permalink
fix: More ariakit upgrade fixes (#817)
Browse files Browse the repository at this point in the history
* Remove hack to hide tooltips when clicking to dismiss a button popover
* Simplify Menu's onItemSelect handling internally
* Hide menus when they lose focus
  • Loading branch information
gnapse authored Mar 7, 2024
1 parent bb396b9 commit c95df9a
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 57 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

Reactist follows [semantic versioning](https://semver.org/) and doesn't introduce breaking changes (API-wise) in minor or patch releases. However, the appearance of a component might change in a minor or patch release so keep an eye on redesigns and make sure your app still looks and feels like you expect it.

# v24.1.1-beta

- [Fix] It was possible to leave a tooltip in a state in which it remained visible all the time. This release fixes the issue.
- [Fix] Auto-close menus when they lose focus to elements other than their own content or their sub-menus.

# v24.1.0-beta

- [Feat] Include changes from [v23.2.0](#v2320) in the beta release
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"email": "[email protected]",
"url": "http://doist.com"
},
"version": "24.1.0-beta",
"version": "24.1.1-beta",
"license": "MIT",
"homepage": "https://github.com/Doist/reactist#readme",
"repository": {
Expand Down
23 changes: 11 additions & 12 deletions src/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type NativeProps<E extends HTMLElement> = React.DetailedHTMLProps<React.HTMLAttr

type MenuContextState = {
menuStore: MenuStore
handleItemSelect: (value: string | null | undefined) => void
handleItemSelect?: (value: string | null | undefined) => void
getAnchorRect: (() => { x: number; y: number }) | null
setAnchorRect: (rect: { x: number; y: number } | null) => void
}
Expand Down Expand Up @@ -76,16 +76,9 @@ function Menu({ children, onItemSelect, ...props }: MenuProps) {
const getAnchorRect = React.useMemo(() => (anchorRect ? () => anchorRect : null), [anchorRect])
const menuStore = useMenuStore({ focusLoop: true, ...props })

const handleItemSelect = React.useCallback(
function handleItemSelect(value: string | null | undefined) {
onItemSelect?.(value)
},
[onItemSelect],
)

const value: MenuContextState = React.useMemo(
() => ({ menuStore, handleItemSelect, getAnchorRect, setAnchorRect }),
[menuStore, handleItemSelect, getAnchorRect, setAnchorRect],
() => ({ menuStore, handleItemSelect: onItemSelect, getAnchorRect, setAnchorRect }),
[menuStore, onItemSelect, getAnchorRect, setAnchorRect],
)

return <MenuContext.Provider value={value}>{children}</MenuContext.Provider>
Expand Down Expand Up @@ -163,6 +156,12 @@ const MenuList = polymorphicComponent<'div', MenuListProps>(function MenuList(
className={classNames('reactist_menulist', exceptionallySetClassName)}
getAnchorRect={getAnchorRect ?? undefined}
modal={modal}
onBlur={(event) => {
if (!event.relatedTarget) return
if (event.currentTarget.contains(event.relatedTarget)) return
if (event.relatedTarget?.closest('[role^="menu"]')) return
menuStore.hide()
}}
/>
</Portal>
) : null
Expand Down Expand Up @@ -249,7 +248,7 @@ const MenuItem = polymorphicComponent<'button', MenuItemProps>(function MenuItem
const onSelectResult: unknown =
onSelect && !event.defaultPrevented ? onSelect() : undefined
const shouldClose = onSelectResult !== false && hideOnSelect
handleItemSelect(value)
handleItemSelect?.(value)
if (shouldClose) hide()
},
[onSelect, onClick, handleItemSelect, hideOnSelect, hide, value],
Expand Down Expand Up @@ -307,7 +306,7 @@ const SubMenu = React.forwardRef<HTMLDivElement, SubMenuProps>(function SubMenu(
const handleSubItemSelect = React.useCallback(
function handleSubItemSelect(value: string | null | undefined) {
if (onItemSelect) onItemSelect(value)
parentMenuItemSelect(value)
parentMenuItemSelect?.(value)
parentMenuHide()
},
[parentMenuHide, parentMenuItemSelect, onItemSelect],
Expand Down
43 changes: 1 addition & 42 deletions src/tooltip/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,50 +95,9 @@ function Tooltip({
throw new Error('Tooltip: String refs cannot be used as they cannot be forwarded')
}

/**
* Prevents the tooltip from automatically firing on focus all the time. This is to prevent
* tooltips from showing when the trigger element is focused back after a popover or dialog that
* it opened was closed. See link below for more details.
* @see https://github.com/ariakit/ariakit/discussions/749
*/
function handleFocus(event: React.FocusEvent<HTMLDivElement>) {
// If focus is not followed by a key up event, does it mean that it's not an intentional
// keyboard focus? Not sure but it seems to work.
// This may be resolved soon in an upcoming version of ariakit:
// https://github.com/ariakit/ariakit/issues/750
function handleKeyUp(event: Event) {
const eventKey = (event as KeyboardEvent).key
if (eventKey !== 'Escape' && eventKey !== 'Enter' && eventKey !== 'Space') {
tooltip.show()
}
}
event.currentTarget.addEventListener('keyup', handleKeyUp, { once: true })
event.preventDefault() // Prevent tooltip.show from being called by TooltipReference
child?.props?.onFocus?.(event)
}

function handleBlur(event: React.FocusEvent<HTMLDivElement>) {
tooltip.hide()
child?.props?.onBlur?.(event)
}

return (
<>
<TooltipAnchor
render={(anchorProps) => {
// Let child props override anchor props so user can specify attributes like tabIndex
// Also, do not apply the child's props to TooltipAnchor as props like `as` can create problems
// by applying the replacement component/element twice
return React.cloneElement(child, {
...child.props,
...anchorProps,
onFocus: handleFocus,
onBlur: handleBlur,
})
}}
store={tooltip}
ref={child.ref}
/>
<TooltipAnchor render={child} store={tooltip} ref={child.ref} />
{isOpen && content ? (
<Box
as={AriakitTooltip}
Expand Down

0 comments on commit c95df9a

Please sign in to comment.