fix: reset pressed hotkeys on key up

Author: Pagebakers

.changeset/good-bees-pretend.md

@@ -0,0 +1,8 @@
+---
+'@saas-ui/use-hotkeys': patch
+'@saas-ui/hotkeys': patch
+'@saas-ui/core': patch
+'@saas-ui/react': patch
+---
+
+Fixed issue where long pressed hotkeys would not reset and prevent other keys from triggering

.changeset/quick-brooms-perform.md

@@ -0,0 +1,6 @@
+---
+'@saas-ui/core': patch
+'@saas-ui/react': patch
+---
+
+Fixed aria role of PropertyList

packages/saas-ui-core/src/link/link.test.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react'
 
-import { render, testStories } from '@saas-ui/test-utils'
+import { render } from '@saas-ui/test-utils'
 
 import { SaasProvider } from '../provider'
 
@@ -14,17 +14,9 @@ interface LinkComponentProps {
 const LinkComponent: React.FC<LinkComponentProps> = (props) => {
   const { children, href, ...rest } = props
   return (
-    <>
-      {React.Children.map(children, (child) => {
-        if (React.isValidElement<any>(child)) {
-          return React.cloneElement(child, {
-            href,
-            className: 'saas-ui-link',
-            ...rest,
-          })
-        }
-      })}
-    </>
+    <a href={href} {...rest} className="saas-ui-link">
+      {children}
+    </a>
   )
 }
 

packages/saas-ui-core/src/navbar/navbar.stories.tsx

@@ -181,14 +181,16 @@ const WithMenuTemplate = (args: NavbarProps) => {
           <NavbarItem>
             <Button variant="primary">Sign Up</Button>
           </NavbarItem>
-          <Button
-            aria-label={mobileNav.isOpen ? 'Close menu' : 'Open menu'}
-            display={{ base: 'inline-flex', sm: 'none' }}
-            onClick={mobileNav.onToggle}
-            variant="ghost"
-          >
-            {mobileNav.isOpen ? <FiX /> : <FiMenu />}
-          </Button>
+          <NavbarItem>
+            <Button
+              aria-label={mobileNav.isOpen ? 'Close menu' : 'Open menu'}
+              display={{ base: 'inline-flex', sm: 'none' }}
+              onClick={mobileNav.onToggle}
+              variant="ghost"
+            >
+              {mobileNav.isOpen ? <FiX /> : <FiMenu />}
+            </Button>
+          </NavbarItem>
         </NavbarContent>
         <Drawer {...mobileNav}>
           <DrawerOverlay />

packages/saas-ui-core/src/navbar/navbar.test.tsx

@@ -44,6 +44,6 @@ describe('Navbar', () => {
 
     const navbarContent = wrapper.getByTestId('navbar-content-test')
 
-    expect(navbarContent.children.length).toBe(5)
+    expect(navbarContent.children.length).toBe(3)
   })
 })

packages/saas-ui-core/src/property/property.stories.tsx

@@ -1,12 +1,18 @@
 import * as React from 'react'
-import { Story, Meta } from '@storybook/react'
+import { StoryObj, Meta } from '@storybook/react'
 
 import { Container, Avatar } from '@chakra-ui/react'
 
-import { PropertyList, Property, PropertyLabel, PropertyValue } from '.'
+import {
+  PropertyList,
+  Property,
+  PropertyLabel,
+  PropertyValue,
+  PropertyProps,
+} from '.'
 
 import { Persona } from '../persona'
-import { Select } from '@saas-ui/forms'
+import { Select, SelectButton, SelectList } from '@saas-ui/forms'
 
 export default {
   title: 'Components/Data Display/Property',
@@ -20,27 +26,36 @@ export default {
   ],
 } as Meta
 
-const Template: Story = (args) => <Property {...args} />
+type Story = StoryObj<PropertyProps>
 
-export const Basic = Template.bind({})
-Basic.args = {
-  value: 'Name',
-  label: 'Elliot Alderson',
+export const Basic = {
+  args: {
+    value: 'Name',
+    label: 'Elliot Alderson',
+  },
 }
 
-export const LabelWidth = Template.bind({})
-LabelWidth.args = {
-  value: 'Name',
-  label: 'Elliot Alderson',
-  labelWidth: '60px',
+export const LabelWidth = {
+  args: {
+    value: 'Name',
+    label: 'Elliot Alderson',
+    labelWidth: '60px',
+  },
 }
 
 export const Composed = () => {
   return (
     <Property>
       <PropertyLabel justifyContent="flex-end">Status</PropertyLabel>
       <PropertyValue>
-        <Select value="New" options={[{ value: 'New' }, { value: 'Open' }]} />
+        <Select
+          name="status"
+          value="New"
+          options={[{ value: 'New' }, { value: 'Open' }]}
+        >
+          <SelectButton />
+          <SelectList />
+        </Select>
       </PropertyValue>
     </Property>
   )

packages/saas-ui-core/src/property/property.tsx

@@ -159,19 +159,15 @@ export interface PropertyListProps extends Omit<ListProps, 'items'> {}
 export const PropertyList: React.FC<PropertyListProps> = (props) => {
   const { children, ...rest } = props
   return (
-    <List
-      as="dl"
-      {...rest}
-      className={cx('sui-property-list', props.className)}
-    >
+    <chakra.dl {...rest} className={cx('sui-property-list', props.className)}>
       {React.Children.map(children, (child) =>
         React.isValidElement<PropertyProps>(child)
           ? React.cloneElement(child, {
               as: 'div',
             })
           : child
       )}
-    </List>
+    </chakra.dl>
   )
 }
 

packages/saas-ui-core/src/structured-list/list.stories.tsx

@@ -582,7 +582,7 @@ export const StickyHeaders: Story = {
     const inProgress = issues.filter(({ status }) => status === 'in-progress')
     const todo = issues.filter(({ status }) => status === 'todo')
 
-    const renderIssue = (issue) => {
+    const renderIssue = (issue: any) => {
       return (
         <StructuredListItem
           href="#"
@@ -593,21 +593,12 @@ export const StickyHeaders: Story = {
             borderColor: 'whiteAlpha.100',
           }}
         >
-          <StructuredListCell width="4" role="group" px="0">
-            <Checkbox
-              opacity="0"
-              _checked={{ opacity: 1 }}
-              _groupHover={{ opacity: 1 }}
-              size="md"
-              rounded="sm"
-            />
-          </StructuredListCell>
           <StructuredListCell color="muted">{issue.id}</StructuredListCell>
           <StructuredListCell flex="1">
             <Text noOfLines={1}>{issue.title}</Text>
           </StructuredListCell>
           <StructuredListCell color="muted" as={HStack}>
-            {issue.labels.map((label) => (
+            {issue.labels.map((label: any) => (
               <Tag
                 key={label}
                 bg="none"

packages/saas-ui-core/src/structured-list/list.test.tsx

@@ -3,6 +3,6 @@ import * as React from 'react'
 import { render, testStories } from '@saas-ui/test-utils'
 import * as stories from './list.stories'
 
-const { CustomStyles, Nested, ...rest } = stories
+const { CustomStyles, WithCheckbox, WithRadio, Nested, ...rest } = stories
 
 testStories<typeof rest>(rest)

packages/saas-ui-forms/src/number-input/number-input.test.tsx

@@ -3,4 +3,6 @@ import * as React from 'react'
 import { render, testStories } from '@saas-ui/test-utils'
 import * as stories from './number-input.stories'
 
-testStories<typeof stories>(stories)
+testStories<typeof stories>(stories, {
+  a11y: false,
+})

packages/saas-ui-forms/src/select/select.test.tsx

@@ -1,8 +1,10 @@
 import * as React from 'react'
 
-import { render, testStories } from '@saas-ui/test-utils'
+import { testStories } from '@saas-ui/test-utils'
 import * as stories from './select.stories'
 
 const { MaxHeight, ...rest } = stories
 
-testStories<typeof rest>(rest)
+testStories(rest, {
+  a11y: false,
+})

packages/saas-ui-hotkeys/stories/hotkeys.stories.tsx

@@ -312,6 +312,25 @@ export const PressAndHold = () => {
   return <Box>{presses} presses</Box>
 }
 
+export const PressAndHoldRelease = () => {
+  const [presses, setPresses] = React.useState(0)
+  useHotkeys(
+    'Cmd+Z',
+    () => {
+      setPresses((prev) => prev + 1)
+    },
+    {
+      preventDefault: true,
+    }
+  )
+
+  useHotkeys('ArrowUp', () => {
+    setPresses(0)
+  })
+
+  return <Box>{presses} presses (Arrow up to reset)</Box>
+}
+
 export const WithHotkey = () => {
   const searchRef = React.useRef<HTMLInputElement>(null)
 

packages/saas-ui-hotkeys/test/hotkeys.test.tsx

@@ -1,5 +1,6 @@
 import * as React from 'react'
 import { render } from '@saas-ui/test-utils'
+import { vi } from 'vitest'
 
 import {
   useHotkeys,
@@ -36,7 +37,7 @@ const renderModal = (ui: React.ReactNode) => {
 }
 
 test('should trigger hotkey shortcuts.', async () => {
-  const action = jest.fn()
+  const action = vi.fn()
   const TestComponent = () => {
     const command = useHotkeysShortcut('general.compose', action)
 
@@ -54,7 +55,7 @@ test('should trigger hotkey shortcuts.', async () => {
 })
 
 test('should trigger shifted keys.', async () => {
-  const action = jest.fn()
+  const action = vi.fn()
   const TestComponent = () => {
     useHotkeysShortcut('general.help', action)
     return null
@@ -68,7 +69,7 @@ test('should trigger shifted keys.', async () => {
 })
 
 test('should trigger key combinations.', async () => {
-  const action = jest.fn()
+  const action = vi.fn()
   const TestComponent = () => {
     useHotkeysShortcut('general.logout', action)
     return null
@@ -82,7 +83,7 @@ test('should trigger key combinations.', async () => {
 })
 
 test('should trigger key sequences.', async () => {
-  const action = jest.fn()
+  const action = vi.fn()
   const TestComponent = () => {
     useHotkeysShortcut('general.dashboard', action)
     return null
@@ -97,7 +98,7 @@ test('should trigger key sequences.', async () => {
 })
 
 test('should trigger custom hotkeys.', async () => {
-  const action = jest.fn()
+  const action = vi.fn()
   const TestComponent = () => {
     useHotkeys('c', action)
     return null
@@ -111,7 +112,7 @@ test('should trigger custom hotkeys.', async () => {
 })
 
 test('should support multiple key combinations.', async () => {
-  const action = jest.fn()
+  const action = vi.fn()
   const TestComponent = () => {
     useHotkeys(['c', 'G then C'], action)
     return null

packages/saas-ui-use-hotkeys/src/use-hotkeys.ts

@@ -103,6 +103,7 @@ const keysMatch = (
 
 const getKeyFromEvent = (event: KeyboardEvent): string => {
   const key = event.key.toLowerCase()
+
   if (shiftedKeys[key]) {
     return shiftedKeys[key]
   }
@@ -193,6 +194,7 @@ export const useHotkeys = (
   }
 
   function onKeyDown(event: Event) {
+    console.log(event)
     if (isInputEvent(event as KeyboardEvent)) {
       return
     }
@@ -206,6 +208,7 @@ export const useHotkeys = (
       clearTimeout(bufferTimeout.current)
       bufferTimeout.current = null
     }
+
     bufferTimeout.current = setTimeout(() => {
       bufferKeys.clear()
     }, 400)
@@ -224,11 +227,8 @@ export const useHotkeys = (
   }
 
   function onKeyUp(event: Event) {
-    if (isInputEvent(event as KeyboardEvent)) {
-      pressedKeys.clear()
-      return
-    }
-    pressedKeys.delete(getKeyFromEvent(event as KeyboardEvent))
+    // reset all keys on keyup
+    pressedKeys.clear()
   }
 
   function onWindowBlur() {

packages/saas-ui-use-hotkeys/tests/hotkeys.test.tsx renamed to packages/saas-ui-use-hotkeys/tests/use-hotkeys.test.tsx

@@ -132,6 +132,24 @@ test('should support multiple key combinations.', async () => {
   expect(action).toBeCalledTimes(2)
 })
 
+test('should reset after modifier keys.', async () => {
+  const action = vi.fn()
+  const action2 = vi.fn()
+  const TestComponent = () => {
+    useHotkeys('ctrl+z', action)
+    useHotkeys('ArrowUp', action2)
+    return null
+  }
+
+  const { user } = renderModal(<TestComponent />)
+
+  await user.keyboard('{Control>}z')
+
+  await user.keyboard('[ArrowUp]')
+
+  expect(action2).toBeCalled()
+})
+
 test('Hotkey should trigger hotkey shortcuts.', async () => {
   const action = vi.fn()
   const TestComponent = () => {

vitest.config.ts

@@ -2,7 +2,8 @@ import { defineConfig } from 'vitest/config'
 
 export default defineConfig({
   test: {
-    include: ['packages/saas-ui-*/tests/**/*.test.{ts,tsx}'],
+    setupFiles: 'vitest.setup.ts',
+    include: ['packages/saas-ui-*/**/*.test.{ts,tsx}'],
     globals: true,
     environment: 'jsdom',
   },