Skip to content

Commit

Permalink
WIP: refactor: useRovingTabindex to utilize IDs
Browse files Browse the repository at this point in the history 
This should allow us to more easily `setActiveElement` by its
ID, without having a reference to `HTMLElement`,
e.g. we can more easily set the initial chat list item.
This issue was mentioned in #4224:
> The initially "active" element is just the first chat item,
> and not the currently selected chat.
  • Loading branch information
@WofWca
WofWca committed Dec 1, 2024
1 parent fb7432f commit f9c8af2
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 61 deletions.
20 changes: 4 additions & 16 deletions packages/frontend/src/components/chat/ChatListItem.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,14 @@ function ChatListItemArchiveLink({
},
])

const ref = useRef<HTMLButtonElement>(null)

const {
tabIndex,
onKeydown: tabindexOnKeydown,
setAsActiveElement: tabindexSetAsActiveElement,
} = useRovingTabindex(ref)
} = useRovingTabindex()

return (
<button
ref={ref}
tabIndex={tabIndex}
onClick={onClick}
onKeyDown={tabindexOnKeydown}
Expand Down Expand Up @@ -222,17 +219,14 @@ function ChatListItemError({
}) {
log.info('Error Loading Chatlistitem ' + chatListItem.id, chatListItem.error)

const ref = useRef<HTMLButtonElement>(null)

const {
tabIndex,
onKeydown: tabindexOnKeydown,
setAsActiveElement: tabindexSetAsActiveElement,
} = useRovingTabindex(ref)
} = useRovingTabindex(chatListItem.id)

return (
<button
ref={ref}
tabIndex={tabIndex}
onClick={onClick}
onKeyDown={tabindexOnKeydown}
Expand Down Expand Up @@ -284,18 +278,15 @@ function ChatListItemNormal({
isSelected?: boolean
hover?: boolean
}) {
const ref = useRef<HTMLButtonElement>(null)

const {
tabIndex,
onKeydown: tabindexOnKeydown,
setAsActiveElement: tabindexSetAsActiveElement,
} = useRovingTabindex(ref)
} = useRovingTabindex(chatListItem.id)
// TODO `setAsActiveElement` if `isSelected` and `activeElement === null`

return (
<button
ref={ref}
tabIndex={tabIndex}
onClick={onClick}
onKeyDown={tabindexOnKeydown}
Expand Down Expand Up @@ -410,19 +401,16 @@ export const ChatListItemMessageResult = React.memo<{
}>(props => {
const { msr, onClick, queryStr } = props

const ref = useRef<HTMLButtonElement>(null)

const {
tabIndex,
onKeydown: tabindexOnKeydown,
setAsActiveElement: tabindexSetAsActiveElement,
} = useRovingTabindex(ref)
} = useRovingTabindex()

if (typeof msr === 'undefined') return <PlaceholderChatListItem />

return (
<button
ref={ref}
tabIndex={tabIndex}
onClick={onClick}
onKeyDown={tabindexOnKeydown}
Expand Down
172 changes: 127 additions & 45 deletions packages/frontend/src/contexts/RovingTabindex.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import React, {
useCallback,
useContext,
useEffect,
useRef,
// useEffect,
useState,
} from 'react'

Expand All @@ -30,15 +32,61 @@ import React, {
* Then `tabindex` is 0 for all the components,
* until `setAsActiveElement` is called explicitly.
* - lacks "Home", "End" key handling, and some other features.
*
* @param elementId any value to identify this element among others when using
* with `setActiveElement`.
* `null` must not be used - it's a special value.
*/
export function useRovingTabindex(elementRef: RefObject<HTMLElement>) {
export function useRovingTabindex(elementId?: unknown) {
const defaultId = useRef(Symbol())
if (elementId === undefined) {
elementId = defaultId
}

const context = useContext(RovingTabindexContext)

// const unsetAsActiveElement = context.unsetAsActiveElement
// useEffect(() => {
// return () => unsetAsActiveElement(elementId)
// // TODO fix: this is not gonna work because `unsetAsActiveElement`
// // changes every time we set a different active element.
// }, [])
// const unsetAsActiveElement = context.unsetAsActiveElement

// useEffect(() => {
// if (context.activeElementId !== elementId) {
// return
// }
// return () => {
// // unsetAsActiveElement(elementId)
// context.setActiveElement(null)
// }
// }, [context, context.activeElementId, elementId])

// const mounted = useRef<boolean>(true)
const unsetAsActiveTimeoutId = useRef<number>(-1)
useEffect(() => {
clearTimeout(unsetAsActiveTimeoutId.current)

if (context.activeElementId !== elementId) {
return
}

// A bit of a hack to unset the element as active when
// the component gets unmounted.
return () => {
unsetAsActiveTimeoutId.current = window.setTimeout(() => {
if (context.activeElementId === elementId) {
context.setActiveElement(null)
}
})
}
}, [context, elementId])

const tabIndex: 0 | -1 =
// If the active element has not been chosen yet,
// let' keep the default behavior (tabindex="0")
context.activeElement == null ||
context.activeElement === elementRef.current
context.activeElementId == null || context.activeElementId === elementId
? 0
: -1

Expand All @@ -64,17 +112,18 @@ export function useRovingTabindex(elementRef: RefObject<HTMLElement>) {
*/
// `elementRef.current` could be null, I am not sure
// if this means something to us practically.
setAsActiveElement: () => context.setActiveElement(elementRef.current),
activeElement: context.activeElement,
setAsActiveElement: () => context.setActiveElement(elementId),
activeElementId: context.activeElementId,
}
}

const log = getLogger('contexts/RovingTabindex')

type ContextValue = {
activeElement: HTMLElement | null
activeElementId: unknown | null
onKeydown: (event: React.KeyboardEvent) => void
setActiveElement: (element: HTMLElement | null) => void
setActiveElement: (elementId: unknown | null) => void
// unsetAsActiveElement: (elementId: unknown) => void
}

type ProviderProps = PropsWithChildren<{
Expand All @@ -92,9 +141,10 @@ type ProviderProps = PropsWithChildren<{
}>

export const RovingTabindexContext = createContext<ContextValue>({
activeElement: null,
activeElementId: null,
onKeydown: () => {},
setActiveElement: () => {},
// unsetAsActiveElement: () => {},
})

/** @see {@link useRovingTabindex} */
Expand All @@ -107,47 +157,57 @@ export function RovingTabindexProvider({
classNameOfTargetElements = 'roving-tabindex'
}

const [activeElement, setActiveElement] = useState<HTMLElement | null>(null)

// Ensure that the "active" element is actually in DOM,
// because it's the only one that is keyboard focusable.
// Otherwise it's impossible to focus the widget by pressing "Tab".
// If it's not in DOM, we need to either set another element
// as the active one, or `setActiveElement(null)` to make
// all the elements focusable.
//
// FYI instead of using a `MutationObserver` we could probaby have
// utilized React's "unmount" callback (i.e. `useEffect`'s cleanup function),
// but I failed at it.
// Let's keep things vanilla then I guess.
useEffect(() => {
if (activeElement == null) {
return
}
const [activeElementId, setActiveElementId] = useState<unknown | null>(null)

const observer = new MutationObserver(_mutations => {
if (!document.body.contains(activeElement)) {
// Another option is to select the next/closest element
// as the new active one.
setActiveElement(null)
}
})
// Maybe we could observe `wrapperElementRef` for performance,
// but IDK let's play it safe.
observer.observe(document.body, { childList: true, subtree: true })
return () => observer.disconnect()
}, [activeElement])
// // Ensure that the "active" element is actually in DOM,
// // because it's the only one that is keyboard focusable.
// // Otherwise it's impossible to focus the widget by pressing "Tab".
// // If it's not in DOM, we need to either set another element
// // as the active one, or `setActiveElement(null)` to make
// // all the elements focusable.
// //
// // FYI instead of using a `MutationObserver` we could probaby have
// // utilized React's "unmount" callback (i.e. `useEffect`'s cleanup function),
// // but I failed at it.
// // Let's keep things vanilla then I guess.
// useEffect(() => {
// if (activeElementId == null) {
// return
// }

// const observer = new MutationObserver(_mutations => {
// if (!document.body.contains(activeElementId)) {
// // Another option is to select the next/closest element
// // as the new active one.
// setActiveElementId(null)
// }
// })
// // Maybe we could observe `wrapperElementRef` for performance,
// // but IDK let's play it safe.
// observer.observe(document.body, { childList: true, subtree: true })
// return () => observer.disconnect()
// }, [activeElementId])

// const unsetAsActiveElement = (elementId: unknown) => {
// if (activeElementId === elementId) {
// setActiveElementId(null)
// }
// }

const onKeydown = useCallback(
(event: React.KeyboardEvent) => {
if (!wrapperElementRef.current) {
log.warn(
'Received keydown event, but there is no wrapperElement? How?',
activeElement
activeElementId
)
return
}
if (!activeElement) {
// TODO now that we utilize `document.activeElement`
// (or `event.target`?) to find the currently active element,
// do we even need to access `activeElementId`
// in `onKeydown` at all?
if (activeElementId === null) {
// This could happen either after the initial render,
// or if the active element was removed from DOM.
// Let's just wait for the user to focus another element
Expand All @@ -174,15 +234,29 @@ export function RovingTabindexProvider({
let oldActiveElementInd: number | undefined
for (let i = 0; i < eligibleElements.length; i++) {
const eligibleEl = eligibleElements[i]
if (eligibleEl === activeElement) {


// TODO maybe utilize `event.target` instead?
// When would it be beneficial?
// Well, I guess we wouldn't have to check `eligibleEl.contains()`.
// Is that all?
//
// I think `document.activeElement` just makes more sense,
// because this entire "library" is about managing focus.
if (
eligibleEl === document.activeElement ||
// also check `contains()` because we must support the active element
// having interactive children.
eligibleEl.contains(document.activeElement)
) {
oldActiveElementInd = i
break
}
}
if (oldActiveElementInd == undefined) {
log.warn(
'Could not find the currently active element in DOM',
activeElement
activeElementId
)
return
}
Expand All @@ -195,22 +269,30 @@ export function RovingTabindexProvider({
// the last or the first.
if (newActiveElement != undefined) {
const newActiveElement_ = newActiveElement as HTMLElement
setActiveElement(newActiveElement_)

// Here we could `setActiveElementId()`, but we don't know its ID.
// However, things will still work out fine because
// after we `newActiveElement_.focus()`,
// it will fire the onFocus event,
// which should invoke `setAsActiveElement`
// (as long as the user attached the event listeners properly).

// It is fine to `.focus()` here without wating for a render
// or `useEffect`, because elements with `tabindex="-1"`
// are still programmatically focusable.
newActiveElement_.focus()
}
},
[activeElement, classNameOfTargetElements, wrapperElementRef]
[activeElementId, classNameOfTargetElements, wrapperElementRef]
)

return (
<RovingTabindexContext.Provider
value={{
activeElement,
activeElementId,
onKeydown,
setActiveElement,
setActiveElement: setActiveElementId,
// unsetAsActiveElement,
}}
>
{children}
Expand Down

0 comments on commit f9c8af2

Please sign in to comment.