Update ListboxOptions to be li elements instead of button

17 files changed, 119 insertions, 77 deletions

diff --git a/desktop/packages/mullvad-vpn/src/renderer/components/toggle-list-item/ToggleListItem.tsx b/desktop/packages/mullvad-vpn/src/renderer/components/toggle-list-item/ToggleListItem.tsx
index a536d50a36..e025d94d84 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/components/toggle-list-item/ToggleListItem.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/components/toggle-list-item/ToggleListItem.tsx
@@ -1,4 +1,5 @@
 import { ListItem, ListItemProps } from '../../lib/components/list-item';
+import { ListItemItemProps } from '../../lib/components/list-item/components';
 import { Switch, SwitchProps } from '../../lib/components/switch';
 import { ToggleListItemLabel, ToggleListItemSwitch } from './components';
 
@@ -6,9 +7,10 @@ export type ToggleListItemProps = ListItemProps & {
   footer?: string;
   checked?: SwitchProps['checked'];
   onCheckedChange?: SwitchProps['onCheckedChange'];
-};
+} & Pick<ListItemItemProps, 'ref'>;
 
 function ToggleListItem({
+  ref,
   children,
   footer,
   checked,
@@ -18,7 +20,7 @@ function ToggleListItem({
 }: ToggleListItemProps) {
   return (
     <ListItem disabled={disabled} {...props}>
-      <ListItem.Item>
+      <ListItem.Item ref={ref}>
         <ListItem.Content>
           <Switch checked={checked} onCheckedChange={onCheckedChange} disabled={disabled}>
             {children}
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/ListItem.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/ListItem.tsx
index 67898736b5..b52bc51d53 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/ListItem.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/ListItem.tsx
@@ -1,6 +1,5 @@
 import React from 'react';
 
-import { Flex } from '../flex';
 import {
   ListItemContent,
   ListItemFooter,
@@ -21,14 +20,12 @@ export type ListItemProps = {
   disabled?: boolean;
   animation?: ListItemAnimation;
   children: React.ReactNode;
-} & React.ComponentPropsWithRef<'div'>;
+};
 
-const ListItem = ({ level = 0, disabled, animation, children, ...props }: ListItemProps) => {
+const ListItem = ({ level = 0, disabled, animation, children }: ListItemProps) => {
   return (
     <ListItemProvider level={level} disabled={disabled} animation={animation}>
-      <Flex $flexDirection="column" $flex={1} {...props}>
-        {children}
-      </Flex>
+      {children}
     </ListItemProvider>
   );
 };
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-item/ListItemItem.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-item/ListItemItem.tsx
index 40e98bd3b7..36abcefebf 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-item/ListItemItem.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-item/ListItemItem.tsx
@@ -1,10 +1,11 @@
+import React from 'react';
 import styled, { css, RuleSet } from 'styled-components';
 
 import { useAnimation, useBackgroundColor } from './hooks';
 
-export interface ListItemItemProps {
+export type ListItemItemProps = {
   children: React.ReactNode;
-}
+} & React.ComponentPropsWithRef<'div'>;
 
 export const StyledListItemItem = styled.div<{
   $backgroundColor: string;
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-trigger/ListItemTrigger.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-trigger/ListItemTrigger.tsx
index 67861fa8eb..bec5508821 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-trigger/ListItemTrigger.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/list-item/components/list-item-trigger/ListItemTrigger.tsx
@@ -2,11 +2,10 @@ import { forwardRef } from 'react';
 import styled, { css } from 'styled-components';
 
 import { colors } from '../../../../foundations';
-import { ListItemProps } from '../../ListItem';
 import { useListItem } from '../../ListItemContext';
 import { StyledListItemItem } from '../list-item-item';
 
-const StyledButton = styled.button<Pick<ListItemProps, 'disabled'>>`
+const StyledButton = styled.button<{ $disabled?: boolean }>`
   display: flex;
   width: 100%;
   background-color: transparent;
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/Listbox.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/Listbox.tsx
index db51468d28..4633ac8a77 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/Listbox.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/Listbox.tsx
@@ -13,9 +13,7 @@ function Listbox<T>({ value, onValueChange, children, ...props }: ListboxProps<T
 
   return (
     <ListboxProvider labelId={labelId} value={value} onValueChange={onValueChange}>
-      <ListItem role="listbox" aria-labelledby={labelId} {...props}>
-        {children}
-      </ListItem>
+      <ListItem {...props}>{children}</ListItem>
     </ListboxProvider>
   );
 }
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/index.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/index.ts
index 51441d576d..43f433a2ea 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/index.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/index.ts
@@ -1,4 +1,4 @@
 export * from './listbox-context';
+export * from './listbox-label';
 export * from './listbox-option';
 export * from './listbox-options';
-export * from './listbox-label';
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/ListboxOption.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/ListboxOption.tsx
index 2b8f6fca51..0423f98ba8 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/ListboxOption.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/ListboxOption.tsx
@@ -1,15 +1,15 @@
 import { ListItem, ListItemProps } from '../../../list-item';
-import { ListItemTriggerProps } from '../../../list-item/components';
-import { ListboxOptionLabel } from './components';
-import { ListboxOptionProvider } from './components/listbox-option-context/ListboxOptionContext';
-import { ListboxOptionIcon } from './components/listbox-option-icon';
-import { ListboxOptionItem } from './components/listbox-option-item/ListboxOptionItem';
-import { ListboxOptionTrigger } from './components/listbox-option-trigger/ListboxOptionTrigger';
+import {
+  ListboxOptionIcon,
+  ListboxOptionItem,
+  ListboxOptionLabel,
+  ListboxOptionProvider,
+  ListboxOptionTrigger,
+} from './components';
 
-export type ListboxOptionProps<T> = ListItemProps &
-  Pick<ListItemTriggerProps, 'onClick'> & {
-    value: T;
-  };
+export type ListboxOptionProps<T> = ListItemProps & {
+  value: T;
+};
 
 function ListboxOption<T>({ value, children, ...props }: ListboxOptionProps<T>) {
   return (
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/index.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/index.ts
index 545ca06af1..f41b29f151 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/index.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/index.ts
@@ -1 +1,5 @@
+export * from './listbox-option-context';
+export * from './listbox-option-icon';
 export * from './listbox-option-label';
+export * from './listbox-option-item';
+export * from './listbox-option-trigger';
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/ListboxOptionContext.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/ListboxOptionContext.tsx
index 098f5631aa..2a87c7e703 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/ListboxOptionContext.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/ListboxOptionContext.tsx
@@ -20,13 +20,10 @@ export function useListboxOptionContext<T>(): ListboxOptionContext<T> {
   return context;
 }
 
-export function ListboxOptionProvider<T>({ value, children }: ListboxOptionProviderProps<T>) {
+export function ListboxOptionProvider<T>({ children, ...props }: ListboxOptionProviderProps<T>) {
   const TypedListboxOptionContext = ListboxOptionContext as React.Context<ListboxOptionContext<T>>;
   return (
-    <TypedListboxOptionContext.Provider
-      value={{
-        value,
-      }}>
+    <TypedListboxOptionContext.Provider value={props}>
       {children}
     </TypedListboxOptionContext.Provider>
   );
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/index.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/index.ts
index e69de29bb2..f14e2d2d4f 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/index.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-context/index.ts
@@ -0,0 +1 @@
+export * from './ListboxOptionContext';
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/ListboxOptionItem.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/ListboxOptionItem.tsx
index c6950c297f..b3e43d25da 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/ListboxOptionItem.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/ListboxOptionItem.tsx
@@ -4,7 +4,7 @@ import { colors } from '../../../../../../foundations';
 import { ListItem } from '../../../../../list-item';
 import { ListItemItemProps } from '../../../../../list-item/components';
 import { useListboxContext } from '../../../listbox-context';
-import { useListboxOptionContext } from '../listbox-option-context/ListboxOptionContext';
+import { useListboxOptionContext } from '../listbox-option-context';
 
 export type ListItemOptionItemProps = ListItemItemProps;
 
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/index.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/index.ts
index e69de29bb2..0855e6efa9 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/index.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-item/index.ts
@@ -0,0 +1 @@
+export * from './ListboxOptionItem';
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/ListboxOptionTrigger.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/ListboxOptionTrigger.tsx
index 13dc8fc2aa..03f56f4f1f 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/ListboxOptionTrigger.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/ListboxOptionTrigger.tsx
@@ -1,33 +1,63 @@
 import React from 'react';
-import styled from 'styled-components';
+import styled, { css } from 'styled-components';
 
 import { colors } from '../../../../../../foundations';
-import { ListItem } from '../../../../../list-item';
-import { ListItemTriggerProps } from '../../../../../list-item/components';
+import { useListItem } from '../../../../../list-item/ListItemContext';
 import { useListboxContext } from '../../../listbox-context';
-import { useListboxOptionContext } from '../listbox-option-context/ListboxOptionContext';
-import { StyledListItemOptionItem } from '../listbox-option-item/ListboxOptionItem';
+import { useListboxOptionContext } from '../';
+import { StyledListItemOptionItem } from '../';
 
-export type ListboxOptionTriggerProps = ListItemTriggerProps;
+export type ListboxOptionTriggerProps = React.ComponentPropsWithRef<'li'>;
 
-export const StyledListItemOptionTrigger = styled(ListItem.Trigger)`
-  &&[aria-selected='true'] {
-    &:hover {
-      ${StyledListItemOptionItem} {
-        background-color: ${colors.green};
-      }
-    }
-    &:active {
-      ${StyledListItemOptionItem} {
-        background-color: ${colors.green};
-      }
-    }
+export const StyledListItemOptionTrigger = styled.li<{ $disabled?: boolean }>`
+  display: flex;
+  width: 100%;
+  background-color: transparent;
+
+  &&:focus-visible {
+    outline: 2px solid ${colors.white};
+    outline-offset: -2px;
+    z-index: 10;
   }
+
+  ${({ $disabled }) => {
+    if (!$disabled) {
+      return css`
+        &&:hover {
+          ${StyledListItemOptionItem} {
+            background-color: ${colors.whiteOnBlue10};
+          }
+        }
+
+        &&:active {
+          ${StyledListItemOptionItem} {
+            background-color: ${colors.whiteOnBlue20};
+          }
+        }
+
+        &&[aria-selected='true'] {
+          &:hover {
+            ${StyledListItemOptionItem} {
+              background-color: ${colors.green};
+            }
+          }
+          &:active {
+            ${StyledListItemOptionItem} {
+              background-color: ${colors.green};
+            }
+          }
+        }
+      `;
+    }
+
+    return null;
+  }}
 `;
 
 export const ListboxOptionTrigger = ({ children, ...props }: ListboxOptionTriggerProps) => {
-  const triggerRef = React.useRef<HTMLButtonElement>(null);
   const { value } = useListboxOptionContext();
+  const { disabled } = useListItem();
+  const triggerRef = React.useRef<HTMLLIElement>(null);
 
   const {
     value: selectedValue,
@@ -38,7 +68,9 @@ export const ListboxOptionTrigger = ({ children, ...props }: ListboxOptionTrigge
   const selected = value === selectedValue;
   const focused = value === focusedValue;
 
-  const onTriggerClick = React.useCallback(async () => {
+  const tabIndex = !disabled && selected && focusedValue === undefined ? 0 : -1;
+
+  const handleTriggerClick = React.useCallback(async () => {
     if (onValueChange) {
       await onValueChange(value);
     }
@@ -55,15 +87,31 @@ export const ListboxOptionTrigger = ({ children, ...props }: ListboxOptionTrigge
     setFocusedValue(value);
   }, [focused, setFocusedValue, value]);
 
-  // TODO: can focus logic be cleaned up?
+  const handleClick = !disabled ? handleTriggerClick : undefined;
+
+  const handleKeyDown = React.useCallback(
+    async (event: React.KeyboardEvent) => {
+      if (disabled) return;
+      if (event.key === 'Enter' || event.key === ' ') {
+        event.preventDefault();
+        if (onValueChange) {
+          await onValueChange(value);
+        }
+      }
+    },
+    [disabled, onValueChange, value],
+  );
+
   return (
     <StyledListItemOptionTrigger
       ref={triggerRef}
       role="option"
       aria-selected={selected}
-      tabIndex={selected && focusedValue === undefined ? 0 : -1}
-      onClick={onTriggerClick}
+      tabIndex={tabIndex}
+      onClick={handleClick}
+      onKeyDown={handleKeyDown}
       onFocus={onFocus}
+      $disabled={disabled}
       {...props}>
       {children}
     </StyledListItemOptionTrigger>
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/index.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/index.ts
index e69de29bb2..8eb2999c36 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/index.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-option/components/listbox-option-trigger/index.ts
@@ -0,0 +1 @@
+export * from './ListboxOptionTrigger';
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/ListboxOptions.tsx b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/ListboxOptions.tsx
index 8d5b8d0631..dc11214e8b 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/ListboxOptions.tsx
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/ListboxOptions.tsx
@@ -8,20 +8,20 @@ export type ListboxOptionsProps = {
 };
 
 export function ListboxOptions({ children }: ListboxOptionsProps) {
-  const { setFocusedValue } = useListboxContext();
-  const ref = React.useRef<HTMLDivElement>(null);
+  const { labelId, setFocusedValue } = useListboxContext();
+  const ref = React.useRef<HTMLUListElement>(null);
   const values = useChildrenValues(children);
 
   const handleKeyboardNavigation = useHandleKeyboardNavigation(values);
   const onKeyDown = React.useCallback(
-    async (event: React.KeyboardEvent) => {
-      await handleKeyboardNavigation(event);
+    (event: React.KeyboardEvent) => {
+      handleKeyboardNavigation(event);
     },
     [handleKeyboardNavigation],
   );
 
   const onBlur = React.useCallback(
-    (e: React.FocusEvent<HTMLDivElement>) => {
+    (e: React.FocusEvent<HTMLUListElement>) => {
       const container = ref.current;
       const nextFocus = e.relatedTarget as Node | null;
 
@@ -34,8 +34,8 @@ export function ListboxOptions({ children }: ListboxOptionsProps) {
   );
 
   return (
-    <div ref={ref} onKeyDown={onKeyDown} onBlur={onBlur}>
+    <ul ref={ref} role="listbox" onKeyDown={onKeyDown} onBlur={onBlur} aria-labelledby={labelId}>
       {children}
-    </div>
+    </ul>
   );
 }
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useChildrenValues.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useChildrenValues.ts
index 1ed0fe3a11..29e58da9cd 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useChildrenValues.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useChildrenValues.ts
@@ -6,7 +6,7 @@ export function useChildrenValues(children: React.ReactNode[]): string[] {
 
     React.Children.forEach(children, (child) => {
       if (React.isValidElement<{ value?: string }>(child)) {
-        if ('value' in child.props && typeof child.props.value === 'string') {
+        if ('value' in child.props && typeof child.props.value !== 'undefined') {
           values.push(child.props.value);
         }
       }
diff --git a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useHandleKeyboardNavigation.ts b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useHandleKeyboardNavigation.ts
index e4adfc7aa0..e705cfba64 100644
--- a/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useHandleKeyboardNavigation.ts
+++ b/desktop/packages/mullvad-vpn/src/renderer/lib/components/listbox/components/listbox-options/hooks/useHandleKeyboardNavigation.ts
@@ -3,23 +3,16 @@ import React from 'react';
 import { useListboxContext } from '../../listbox-context';
 
 export const useHandleKeyboardNavigation = <T>(options: T[]) => {
-  const {
-    value: selectedValue,
-    focusedValue,
-    setFocusedValue,
-    onValueChange,
-  } = useListboxContext<T>();
+  const { value: selectedValue, focusedValue, setFocusedValue } = useListboxContext<T>();
 
   return React.useCallback(
-    async (event: React.KeyboardEvent) => {
-      if (event.key === 'Space' || event.key === 'Enter') {
-        if (onValueChange && focusedValue) {
-          await onValueChange(focusedValue);
-        }
-      }
+    (event: React.KeyboardEvent) => {
+      let value: T;
+      if (focusedValue !== undefined) value = focusedValue;
+      else if (selectedValue !== undefined) value = selectedValue;
+      else value = options[0];
 
       // Use roving tabindex to determine the next focusable option
-      const value = focusedValue || selectedValue || options[0];
       let nextValue: T | undefined;
       if (event.key === 'ArrowUp') {
         event.preventDefault();
@@ -45,6 +38,6 @@ export const useHandleKeyboardNavigation = <T>(options: T[]) => {
         setFocusedValue(nextValue);
       }
     },
-    [focusedValue, selectedValue, options, onValueChange, setFocusedValue],
+    [focusedValue, selectedValue, options, setFocusedValue],
   );
 };