From 6af612acc1a7a09b319c17283b0f53132c84e0f5 Mon Sep 17 00:00:00 2001
From: Jo du Plessis <jo@joduplessis.com>
Date: Wed, 12 Jun 2024 14:37:44 +0200
Subject: [PATCH] refine autoFocus behaviour

---
 packages/core/src/hooks/focus.hook.ts     | 44 ++++++-----------------
 packages/core/src/input/input-popover.tsx | 23 ++++++++----
 packages/core/src/modal/modal.tsx         |  3 +-
 packages/core/src/popover/popover.css     |  2 +-
 packages/core/src/popover/popover.tsx     |  3 +-
 5 files changed, 31 insertions(+), 44 deletions(-)

diff --git a/packages/core/src/hooks/focus.hook.ts b/packages/core/src/hooks/focus.hook.ts
index 402ed07a..9a1a6eeb 100644
--- a/packages/core/src/hooks/focus.hook.ts
+++ b/packages/core/src/hooks/focus.hook.ts
@@ -2,11 +2,18 @@ import { useEffect, useRef } from 'react'
 import { documentObject, getKey } from '../helpers'
 import { CoreViewProps } from '../types'
 
-export const FOCUSABLE =
-    'a[href]:not([disabled]), button:not([disabled]), textarea:not([disabled]), input:not([disabled]), input[type="text"]:not([disabled]), input[type="radio"]:not([disabled]), input[type="checkbox"]:not([disabled]), select:not([disabled])'
+export const FOCUSABLE = [
+    'a[href]:not([disabled])',
+    'button:not([disabled])',
+    'input:not([disabled])',
+    'select:not([disabled])',
+    'textarea:not([disabled])',
+    '[tabindex]:not([tabindex="-1"])',
+    '[contenteditable]'
+]
 
 export type FocusTrapProps = {
-    focusable?: string
+    focusable?: string[]
     onEscape?: any
 } & CoreViewProps
 
@@ -30,40 +37,11 @@ export const useFocus = (focusable = FOCUSABLE, arrowNavigation = false) => {
                 firstFocusableEl.current?.focus()
             }
         }
-
-        /*         
-        Needs refactoring after the <Select/> keyboard navigation is done
-        
-        if (isTabNormal) {
-            if (documentObject.activeElement === lastFocusableEl.current) firstFocusableEl.current?.focus()
-        }
-
-        if (isTabReverse) {
-            if (documentObject.activeElement === firstFocusableEl.current) lastFocusableEl.current?.focus()
-        }
-
-        if (arrowNavigation) {
-            if (isUp) {
-                flag = true
-                elements[previousIndex].focus()
-            }
-
-            if (isDown) {
-                flag = true
-                elements[nextIndex].focus()
-            }
-        }
-
-        if (flag) {
-            e.stopPropagation()
-            e.preventDefault()
-        }    
-        */
     }
 
     const trapFocus = (el) => {
         containerRef.current = el
-        focusableEls.current = containerRef.current.querySelectorAll(focusable)
+        focusableEls.current = containerRef.current.querySelectorAll(focusable.join(','))
         lastFocusableEl.current = focusableEls.current[focusableEls.current.length - 1]
         firstFocusableEl.current = focusableEls.current[0]
         firstFocusableEl.current?.focus()
diff --git a/packages/core/src/input/input-popover.tsx b/packages/core/src/input/input-popover.tsx
index add62757..18488847 100644
--- a/packages/core/src/input/input-popover.tsx
+++ b/packages/core/src/input/input-popover.tsx
@@ -2,15 +2,25 @@ import React, { ReactNode, cloneElement, useRef } from 'react'
 import { Popover, PopoverProps, getKey, renderChildren, useId, useVisibility } from '../'
 
 export type InputPopoverProps = {
+    __openDelay?: number
     id?: string
+    firstTimeFocusOpen?: boolean
     defaultVisibility?: boolean
     popoverProps?: PopoverProps
-    children: ReactNode
+    children: ReactNode 
     content: ReactNode
 }
 
 export const InputPopover = (props: InputPopoverProps) => {
-    const { id, defaultVisibility = false, popoverProps = {}, children, content } = props
+    const { 
+        __openDelay = 100,
+        id, 
+        firstTimeFocusOpen = true,
+        defaultVisibility = false, 
+        popoverProps = {}, 
+        children, 
+        content 
+    } = props
     const childRef = useRef(null)
     const firstTimeFocus = useRef(false)
     const isOpen = useRef(false)
@@ -18,20 +28,20 @@ export const InputPopover = (props: InputPopoverProps) => {
     const internalId = useId(id)
 
     const handleFocus = (e) => {
-        if (!firstTimeFocus.current) {
+        if (!firstTimeFocus.current && firstTimeFocusOpen) {
             e.stopPropagation()
             e.preventDefault()
-            if (!isOpen.current) delayedShow(100)
+            if (!isOpen.current) delayedShow(__openDelay)
         }
     }
 
     const handleClick = (e) => {
-        if (!isOpen.current) delayedShow(100)
+        if (!isOpen.current) delayedShow(__openDelay)
     }
 
     const handleKeyDown = (e) => {
         const { isEnter } = getKey(e)
-        if (isEnter && !isOpen.current) delayedShow(100)
+        if (isEnter && !isOpen.current) delayedShow(__openDelay)
     }
 
     const handleDismiss = (e) => {
@@ -43,6 +53,7 @@ export const InputPopover = (props: InputPopoverProps) => {
 
     return (
         <Popover
+            autoFocus   
             targetId={internalId}
             isVisible={visible}
             content={content}
diff --git a/packages/core/src/modal/modal.tsx b/packages/core/src/modal/modal.tsx
index e98b7276..04508e3c 100644
--- a/packages/core/src/modal/modal.tsx
+++ b/packages/core/src/modal/modal.tsx
@@ -64,13 +64,12 @@ export const Modal = (props: ModalProps) => {
     const { trapFocus } = useFocus()
 
     const handleKeyDown = (e) => {
-        console.log('modal')
         const { isEscape } = getKey(e)
         if (isEscape && dismissOnEscape) onDismiss(e)
     }
 
     const handleBackgroundClick = (e: any) => {
-        // seems necessary when nesting modals/dialogs/alerts
+        // necessary when nesting modals/dialogs/alerts
         if (disableBackgroundEventPropagation) {
             e.preventDefault()
             e.stopPropagation()
diff --git a/packages/core/src/popover/popover.css b/packages/core/src/popover/popover.css
index a2a53ece..e42b6a42 100644
--- a/packages/core/src/popover/popover.css
+++ b/packages/core/src/popover/popover.css
@@ -20,7 +20,7 @@
 }
 
 .f-popover:focus {
-    outline: none
+    outline: none;
 }
 
 .f-popover:not(.is-ready) {
diff --git a/packages/core/src/popover/popover.tsx b/packages/core/src/popover/popover.tsx
index 48ef2c02..f14b8170 100644
--- a/packages/core/src/popover/popover.tsx
+++ b/packages/core/src/popover/popover.tsx
@@ -40,7 +40,7 @@ export type PopoverProps = {
 
 export const Popover = forwardRef((props: PopoverProps, ref) => {
     const {
-        __autoFocusTimeoutDelay = 200,
+        __autoFocusTimeoutDelay = 0,
         autoFocus = true,
         targetId,
         fixPosition,
@@ -96,7 +96,6 @@ export const Popover = forwardRef((props: PopoverProps, ref) => {
         if (autoFocus && showPopover) {
             setTimeout(() => {
                 containerRef.current?.focus()
-
             }, __autoFocusTimeoutDelay)
         }
     }, [