Skip to content

Commit

Permalink
Fix potential wrong focus setting
Browse files Browse the repository at this point in the history
  • Loading branch information
tujoworker committed Sep 12, 2024
1 parent ebb2565 commit d131df9
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 22 deletions.
18 changes: 12 additions & 6 deletions packages/dnb-eufemia/src/extensions/forms/Iterate/Array/Array.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import useDataValue from '../../hooks/useDataValue'
import { useSwitchContainerMode } from '../hooks'

import type { ContainerMode, ElementChild, Props, Value } from './types'
import type { Identifier, Path } from '../../types'
import type { Identifier } from '../../types'

/**
* Deprecated, as it is supported by all major browsers and Node.js >=v18
Expand Down Expand Up @@ -99,7 +99,11 @@ function ArrayComponent(props: Props) {
const modesRef = useRef<
Record<
Identifier,
{ current: ContainerMode; previous?: ContainerMode }
{
current: ContainerMode
previous?: ContainerMode
options?: { omitFocusManagement?: boolean }
}
>
>({})
const valueWhileClosingRef = useRef<Value>()
Expand Down Expand Up @@ -161,13 +165,15 @@ function ArrayComponent(props: Props) {
containerMode: modesRef.current[id].current,
previousContainerMode: modesRef.current[id].previous,
initialContainerMode: containerMode || 'auto',
switchContainerMode: (mode: ContainerMode) => {
modeOptions: modesRef.current[id].options,
switchContainerMode: (mode, options = {}) => {
modesRef.current[id].previous = modesRef.current[id].current
modesRef.current[id].current = mode
modesRef.current[id].options = options
delete isNewRef.current?.[id]
forceUpdate()
},
handleChange: (path: Path, value: unknown) => {
handleChange: (path, value) => {
const newArrayValue = structuredClone(arrayValue)

// Make sure we have a new object reference,
Expand All @@ -177,7 +183,7 @@ function ArrayComponent(props: Props) {
pointer.set(newArrayValue, path, value)
handleChange(newArrayValue)
},
handlePush: (element: unknown) => {
handlePush: (element) => {
hadPushRef.current = true
handleChange([...(arrayValue || []), element])
},
Expand All @@ -203,7 +209,7 @@ function ArrayComponent(props: Props) {
},

// - Called when cancel button press
restoreOriginalValue: (value: unknown) => {
restoreOriginalValue: (value) => {
if (value) {
const newArrayValue = structuredClone(arrayValue)
newArrayValue[index] = value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ function ArrayItemArea(props: Props & FlexContainerProps) {
const { hasError, hasSubmitError } =
useContext(FieldBoundaryContext) || {}
localContextRef.current = useContext(IterateItemContext) || {}
const omitFocusManagementRef = useRef(false)
const nextFocusElementRef = useRef<HTMLElement>()
const { isNew, value } = localContextRef.current
if (hasSubmitError || !value) {
Expand All @@ -69,8 +68,7 @@ function ArrayItemArea(props: Props & FlexContainerProps) {
) {
// - Set the container mode to "edit" if we have an error
if (hasError && !isNew) {
omitFocusManagementRef.current = true
switchContainerMode('edit')
switchContainerMode('edit', { omitFocusManagement: true })
}
}
}, [hasError, hasSubmitError, isNew, mode])
Expand Down Expand Up @@ -113,19 +111,18 @@ function ArrayItemArea(props: Props & FlexContainerProps) {
const setFocus = useCallback(
(state) => {
if (
!omitFocusManagementRef.current &&
localContextRef.current.modeOptions?.omitFocusManagement !==
true &&
!hasSubmitError &&
containerMode === mode && // ensure we match the correct mode
containerMode !== previousContainerMode // ensure we have a new mode
) {
if (state === 'opened') {
localContextRef.current?.elementRef?.current?.focus?.()
localContextRef.current.elementRef?.current?.focus?.()
} else if (state === 'closed') {
nextFocusElementRef.current?.focus?.()
}
}

omitFocusManagementRef.current = false
},
[containerMode, hasSubmitError, mode, previousContainerMode]
)
Expand All @@ -135,7 +132,7 @@ function ArrayItemArea(props: Props & FlexContainerProps) {
(state) => {
if (!openRef.current && isRemoving.current) {
isRemoving.current = false
localContextRef.current?.fulfillRemove?.()
localContextRef.current.fulfillRemove?.()
}

setFocus(state)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ describe('ArrayItemArea', () => {
)

expect(switchContainerMode).toHaveBeenCalledTimes(1)
expect(switchContainerMode).toHaveBeenLastCalledWith('edit')
expect(switchContainerMode).toHaveBeenLastCalledWith('edit', {
omitFocusManagement: true,
})
})

it('opens component based on "open" prop', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import { render, fireEvent, waitFor } from '@testing-library/react'
import IterateItemContext from '../../IterateItemContext'
import { Field, Form, Iterate } from '../../..'
import { Field, Form, Iterate, Value } from '../../..'
import userEvent from '@testing-library/user-event'
import nbNO from '../../../constants/locales/nb-NO'

Expand Down Expand Up @@ -190,6 +190,105 @@ describe('EditContainer and ViewContainer', () => {
expect(containerMode).toBe('edit')
})

it('should only set focus on newly added element when containerMode changes', async () => {
const containerMode = []

const ContextConsumer = () => {
const context = React.useContext(IterateItemContext)
containerMode.push(context.containerMode)

return null
}

render(
<Form.Handler>
<Iterate.Array path="/items">
<Iterate.ViewContainer>
<Value.String path="/foo" />
</Iterate.ViewContainer>
<Iterate.EditContainer>
<Field.String path="/foo" />
</Iterate.EditContainer>
<ContextConsumer />
</Iterate.Array>
<Iterate.PushButton path="/items" pushValue="value" />
</Form.Handler>
)

expect(document.body).toHaveFocus()
expect(containerMode).toEqual([])

const button = document.querySelector('button')
await userEvent.click(button)

expect(containerMode).toEqual(['edit', 'edit', 'edit'])

{
const elements = document.querySelectorAll(
'.dnb-forms-iterate__element'
)
expect(elements).toHaveLength(1)

const firstElement = elements[0]
const [, editBlock] = Array.from(
firstElement.querySelectorAll('.dnb-forms-section-block')
)

expect(
document.querySelector('.dnb-forms-iterate-push-button')
).toHaveFocus()
await waitFor(() => {
expect(editBlock).toHaveStyle('height: auto;')
})
expect(firstElement).toHaveFocus()
}

await userEvent.click(button)

{
expect(containerMode).toEqual([
'edit',
'edit',
'edit',
'edit',
'edit',
'edit',
])

const elements = document.querySelectorAll(
'.dnb-forms-iterate__element'
)
expect(elements).toHaveLength(2)

const firstElement = elements[0]
const secondElement = elements[1]
const [, editBlock] = Array.from(
secondElement.querySelectorAll('.dnb-forms-section-block')
)

expect(
document.querySelector('.dnb-forms-iterate-push-button')
).toHaveFocus()
await waitFor(() => {
expect(editBlock).toHaveStyle('height: auto;')
})
expect(
firstElement.querySelector('.dnb-forms-section-block')
).toHaveStyle('height: 0;')
expect(secondElement).toHaveFocus()
expect(containerMode).toEqual([
'edit',
'edit',
'edit',
'edit',
'edit',
'edit',
'view',
'edit',
])
}
})

it('should set focus on __element when containerMode changes', async () => {
render(
<Iterate.Array value={['foo', 'bar']}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import React from 'react'
import { Path } from '../types'
import { ContainerMode, ElementChild } from './Array/types'

export type ModeOptions = {
omitFocusManagement?: boolean
}

export interface IterateItemContextState {
id?: string
index?: number
Expand All @@ -14,7 +18,11 @@ export interface IterateItemContextState {
initialContainerMode?: ContainerMode
containerRef?: React.RefObject<HTMLDivElement>
elementRef?: React.RefObject<HTMLDivElement>
switchContainerMode?: (mode: ContainerMode) => void
modeOptions?: ModeOptions
switchContainerMode?: (
mode: ContainerMode,
options?: ModeOptions
) => void
handleChange?: (path: Path, value: unknown) => void
handleRemove?: ({ keepItems }?: { keepItems?: boolean }) => void
handlePush?: (value: unknown) => void
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,30 @@
import { useCallback, useContext, useEffect, useRef } from 'react'
import { ContainerMode } from '../Array'
import FieldBoundaryContext from '../../DataContext/FieldBoundary/FieldBoundaryContext'
import IterateItemContext from '../IterateItemContext'
import FieldBoundaryContext, {
FieldBoundaryContextState,
} from '../../DataContext/FieldBoundary/FieldBoundaryContext'
import IterateItemContext, {
IterateItemContextState,
} from '../IterateItemContext'
import { Path } from '../../types'

type GlobalCacheHash = string
type GlobalCacheId = string
type GlobalCache = {
index: IterateItemContextState['index']
hasError: FieldBoundaryContextState['hasError']
count: number
switchContainerMode: IterateItemContextState['switchContainerMode']
}
type GlobalCacheItem = {
[string: GlobalCacheHash]: GlobalCache
}

const globalContainerModeRef = { current: undefined }
const globalCache = {}
const globalCache: Record<
GlobalCacheHash,
Record<GlobalCacheId, GlobalCache>
> = {}

/**
* This is a helper for the Iterate component.
Expand Down Expand Up @@ -34,7 +53,7 @@ export default function useSwitchContainerMode({

useEffect(() => {
if (hash && iterateItemContext) {
globalCache[hash] = globalCache[hash] || {}
globalCache[hash] = globalCache[hash] || ({} as GlobalCacheItem)
const { index, arrayValue, switchContainerMode } = iterateItemContext
globalCache[hash][id] = {
index,
Expand Down Expand Up @@ -62,7 +81,7 @@ export default function useSwitchContainerMode({
const item = data[id]
const mode = item && fn?.(item)
if (mode) {
item.switchContainerMode(mode)
item.switchContainerMode(mode, { omitFocusManagement: true })
}
}
},
Expand Down

0 comments on commit d131df9

Please sign in to comment.