Skip to content

chore(island-ui): Update reakit to latest version of ariakit #18848

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ const AppealCaseFilesOverview = () => {
</Box>
<Box marginLeft={3}>
<ContextMenu
dataTestId="contextMenu"
items={[
{
title: formatMessage(contextMenu.openFile),
Expand All @@ -179,8 +178,7 @@ const AppealCaseFilesOverview = () => {
]
: []),
]}
menuLabel="Opna valmöguleika á skjali"
disclosure={
render={
<IconButton
icon="ellipsisVertical"
colorScheme="transparent"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ describe('ContextMenu', () => {
render(
<ContextMenu
title="Innskráning"
menuLabel="innskráning"
items={[{ title: 'Einstaklingur' }, { title: 'Fyrirtæki', href: '#' }]}
/>,
)
Expand Down
159 changes: 81 additions & 78 deletions apps/judicial-system/web/src/components/ContextMenu/ContextMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import { cloneElement, forwardRef, ReactElement } from 'react'
import { forwardRef, ReactElement, useState } from 'react'
import { useIntl } from 'react-intl'
import cn from 'classnames'
import { Menu, MenuButton, MenuItem, useMenuState } from 'reakit/Menu'
import { useRouter } from 'next/router'
import {
Menu,
MenuButton,
MenuItem,
MenuProvider,
useMenuStore,
} from '@ariakit/react'

import {
Box,
Expand All @@ -11,7 +18,6 @@ import {
IconMapIcon,
useBoxStyles,
} from '@island.is/island-ui/core'
import { TestSupport } from '@island.is/island-ui/utils'

import { useCaseList } from '../../utils/hooks'
import { contextMenu as strings } from './ContextMenu.strings'
Expand All @@ -27,20 +33,14 @@ export interface ContextMenuItem {
export type MenuItems = ContextMenuItem[]

interface ContextMenuProps {
// Aria label for menu
menuLabel: string

// Menu items
items: ContextMenuItem[]

// Text in the menu button
title?: string

// Custom element to be used as the menu button
disclosure?: ReactElement

// Space between menu and button
offset?: [string | number, string | number]
render?: ReactElement
}

export const useContextMenu = () => {
Expand All @@ -60,21 +60,12 @@ export const useContextMenu = () => {
}
}

const ContextMenu = forwardRef<HTMLElement, ContextMenuProps & TestSupport>(
({ disclosure, menuLabel, items, title, dataTestId, offset }, ref) => {
const menu = useMenuState({
placement: 'bottom-end',
unstable_offset: offset ?? [0, 4],
})

const menuBoxStyle = useBoxStyles({
component: 'div',
background: 'white',
display: 'flex',
flexDirection: 'column',
borderRadius: 'large',
zIndex: 10,
})
export const ContextMenu = forwardRef<HTMLButtonElement, ContextMenuProps>(
({ render, items, title }, ref) => {
const [open, setOpen] = useState(false)
const [isLoading, setIsLoading] = useState(false)
const router = useRouter()
const menu = useMenuStore({ open, setOpen })

const menuItemBoxStyle = useBoxStyles({
component: 'button',
Expand All @@ -89,64 +80,76 @@ const ContextMenu = forwardRef<HTMLElement, ContextMenuProps & TestSupport>(
variant: 'default',
})

const menuBoxStyle = useBoxStyles({
component: 'div',
background: 'white',
display: 'flex',
flexDirection: 'column',
borderRadius: 'large',
zIndex: 10,
marginTop: 1,
})

const handleClick = (evt: React.MouseEvent, item: ContextMenuItem) => {
evt.stopPropagation()
evt.preventDefault()

setOpen(false)
setIsLoading(true)

if (item.href) {
router.push(item.href)
return
}

if (item.onClick) {
item.onClick()
return
}
}

Comment on lines +93 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Loading state never resets when onClick does not navigate

setIsLoading(true) is set, but false is never restored when an item’s onClick handler performs an in‑place action (e.g. opens a modal).
Consider wrapping the action in a try/finally and resetting the flag, or make onClick return a promise you can await.

- setIsLoading(true)
+ setIsLoading(true)
+
+const complete = () => setIsLoading(false)
 
 if (item.href) {
   router.push(item.href)
-  return
+  complete()
+  return
 }
 
 if (item.onClick) {
-  item.onClick()
-  return
+  const maybePromise = item.onClick()
+  if (maybePromise?.finally) {
+    maybePromise.finally(complete)
+  } else {
+    complete()
+  }
+  return
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleClick = (evt: React.MouseEvent, item: ContextMenuItem) => {
evt.stopPropagation()
evt.preventDefault()
setOpen(false)
setIsLoading(true)
if (item.href) {
router.push(item.href)
return
}
if (item.onClick) {
item.onClick()
return
}
}
const handleClick = (evt: React.MouseEvent, item: ContextMenuItem) => {
evt.stopPropagation()
evt.preventDefault()
setOpen(false)
setIsLoading(true)
const complete = () => setIsLoading(false)
if (item.href) {
router.push(item.href)
complete()
return
}
if (item.onClick) {
const maybePromise = item.onClick()
if (maybePromise?.finally) {
maybePromise.finally(complete)
} else {
complete()
}
return
}
}

return (
<>
{disclosure ? (
<MenuButton
ref={ref}
{...menu}
{...disclosure.props}
dataTestId={dataTestId}
>
{(disclosureProps) => cloneElement(disclosure, disclosureProps)}
</MenuButton>
) : (
<MenuButton as={Button} icon="add" {...menu} dataTestId={dataTestId}>
{title}
</MenuButton>
)}
<MenuProvider>
<MenuButton
render={render ?? <Button icon="add" loading={isLoading} />}
store={menu}
ref={ref}
>
{render ? null : title}
</MenuButton>
<Menu
{...menu}
aria-label={menuLabel}
className={cn(styles.menu, menuBoxStyle)}
render={<ul className={cn(styles.menu, menuBoxStyle)} />}
store={menu}
unmountOnHide
>
{items.map((item, index) => {
let anchorProps = {}
if (item.href) {
anchorProps = {
href: item.href,
as: 'a',
}
}
return (
<MenuItem
{...menu}
{...anchorProps}
key={`${item.title}_${index}`}
onClick={(evt) => {
evt.stopPropagation()
menu.hide()
if (item.onClick) {
item.onClick()
}
}}
className={cn(
menuItemBoxStyle,
menuItemTextStyle,
styles.menuItem,
)}
>
{item.icon && (
<Box display="flex" marginRight={2}>
<Icon icon={item.icon} type="outline" color="blue400" />
{items?.map((item, index) => (
<MenuItem
key={`${item.title}_${index}`}
render={
<Box component="li" width="full">
<Box
component={item.href ? 'a' : 'button'}
href={item.href ?? undefined}
onClick={(evt) => handleClick(evt, item)}
className={cn(
menuItemBoxStyle,
menuItemTextStyle,
styles.menuItem,
)}
>
{item.icon && (
<Box display="flex" marginRight={2}>
<Icon icon={item.icon} type="outline" color="blue400" />
</Box>
)}
{item.title}
</Box>
)}
{item.title}
</MenuItem>
)
})}
</Box>
}
/>
))}
</Menu>
</>
</MenuProvider>
)
},
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { forwardRef, MouseEvent } from 'react'
import cn from 'classnames'
import { Button } from 'reakit'
import { Button } from '@ariakit/react/button'

import { Box, Icon, IconMapIcon } from '@island.is/island-ui/core'

Expand Down Expand Up @@ -33,7 +33,7 @@ const IconButton = forwardRef<HTMLButtonElement, Props>(({ ...props }, ref) => {
}
onClick={(evt) => onClick && onClick(evt)}
disabled={disabled}
aria-label="Opna valmöguleika fyrir mál"
aria-label="Valmynd"
>
<Icon
icon={icon}
Expand Down
3 changes: 1 addition & 2 deletions apps/judicial-system/web/src/components/Table/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,8 @@ const Table: FC<TableProps> = (props) => {
</motion.div>
) : (
<ContextMenu
menuLabel={`Valmynd fyrir mál ${row.courtCaseNumber}`}
items={generateContextMenuItems(row)}
disclosure={
render={
<motion.div
className={styles.smallContainer}
key={row.id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,7 @@ const CreateCaseButton: FC<CreateCaseButtonProps> = (props) => {

return (
<Box marginTop={[2, 2, 0]}>
<ContextMenu
dataTestId="createCaseDropdown"
menuLabel="Tegund kröfu"
items={items}
title={formatMessage(m.createCaseButton)}
offset={[0, 8]}
/>
<ContextMenu title="Nýtt mál" items={items} />
</Box>
)
}
Expand Down
2 changes: 1 addition & 1 deletion libs/island-ui/core/src/lib/DatePicker/DatePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
timeFormat,
dateFormatWithTime,
} from '@island.is/shared/constants'
import { VisuallyHidden } from 'reakit'
import { VisuallyHidden } from '@ariakit/react'
import range from 'lodash/range'

import { Icon } from '../IconRC/Icon'
Expand Down
14 changes: 4 additions & 10 deletions libs/island-ui/core/src/lib/Input/Input.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import cn from 'classnames'
import React, { forwardRef, useLayoutEffect, useRef, useState } from 'react'
import { VisuallyHidden } from 'reakit'
import React, { FC, forwardRef, useLayoutEffect, useRef, useState } from 'react'
import { VisuallyHidden } from '@ariakit/react'
import { resolveResponsiveProp } from '../../utils/responsiveProp'
import { Box } from '../Box/Box'
import { UseBoxStylesProps } from '../Box/useBoxStyles'
Expand Down Expand Up @@ -255,14 +255,8 @@ export const Input = forwardRef(
},
)

function AsideIcons({
icon,
buttons = [],
size,
loading,
hasError,
hasLabel,
}: AsideProps) {
const AsideIcons: FC<AsideProps> = (props) => {
const { icon, buttons = [], size, loading, hasError, hasLabel } = props
const displayedIcon: InputIcon | undefined = hasError
? { name: 'warning' }
: icon
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"@apollo/gateway": "2.1.1",
"@apollo/subgraph": "2.1.1",
"@apollo/utils.keyvadapter": "3.0.0",
"@ariakit/react": "0.4.17",
"@aws-sdk/client-s3": "3.662.0",
"@aws-sdk/client-sqs": "3.662.0",
"@aws-sdk/credential-provider-node": "3.662.0",
Expand Down
34 changes: 34 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,39 @@ __metadata:
languageName: node
linkType: hard

"@ariakit/core@npm:0.4.15":
version: 0.4.15
resolution: "@ariakit/core@npm:0.4.15"
checksum: 10/525fd3f5506304d81a4aa1ec192dc5fb88940063d29893e2553a5766c15f3647ff5fe902b318304d404838a248c75c06b1f01ba0582657ad370b3c44379dcef4
languageName: node
linkType: hard

"@ariakit/react-core@npm:0.4.17":
version: 0.4.17
resolution: "@ariakit/react-core@npm:0.4.17"
dependencies:
"@ariakit/core": "npm:0.4.15"
"@floating-ui/dom": "npm:^1.0.0"
use-sync-external-store: "npm:^1.2.0"
peerDependencies:
react: ^17.0.0 || ^18.0.0 || ^19.0.0
react-dom: ^17.0.0 || ^18.0.0 || ^19.0.0
checksum: 10/4e2055ebfa886bc411297059be033590e7b9631c4282f4cc1a09c771fd92421aa65d850baf311c28d778e80fdbfac34e8aec034cbb53c0d26810ca99daa9469e
languageName: node
linkType: hard

"@ariakit/react@npm:0.4.17":
version: 0.4.17
resolution: "@ariakit/react@npm:0.4.17"
dependencies:
"@ariakit/react-core": "npm:0.4.17"
peerDependencies:
react: ^17.0.0 || ^18.0.0 || ^19.0.0
react-dom: ^17.0.0 || ^18.0.0 || ^19.0.0
checksum: 10/44c0fbbdafcdf81914ed93b20c873a176170203f620f7b5faa9a2e2443527cdc94db1b7525dcdba5aa48b76ea747eaddf03563fad1dac458a206307ff9e11fda
languageName: node
linkType: hard

"@aw-web-design/x-default-browser@npm:1.4.126":
version: 1.4.126
resolution: "@aw-web-design/x-default-browser@npm:1.4.126"
Expand Down Expand Up @@ -39546,6 +39579,7 @@ __metadata:
"@apollo/gateway": "npm:2.1.1"
"@apollo/subgraph": "npm:2.1.1"
"@apollo/utils.keyvadapter": "npm:3.0.0"
"@ariakit/react": "npm:0.4.17"
"@aws-sdk/client-s3": "npm:3.662.0"
"@aws-sdk/client-ses": "npm:3.145.0"
"@aws-sdk/client-sqs": "npm:3.662.0"
Expand Down
Loading