Skip to content

Commit 2477fc6

Browse files
authored
fix(ui): custom block labels stale when reordering blocks (#11367)
When blocks have custom row labels, those row labels become stale when reordering blocks. After moving a block, for example, the row label will jump back the original block until form state returns with the proper rendering order. This is especially evident on slow networks.
1 parent 3778180 commit 2477fc6

File tree

2 files changed

+68
-13
lines changed

2 files changed

+68
-13
lines changed

packages/ui/src/forms/Form/fieldReducer.ts

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -180,31 +180,52 @@ export function fieldReducer(state: FormState, action: FieldAction): FormState {
180180

181181
case 'MOVE_ROW': {
182182
const { moveFromIndex, moveToIndex, path } = action
183-
const { remainingFields, rows } = separateRows(path, state)
184183

185-
// copy the row to move
186-
const copyOfMovingRow = rows[moveFromIndex]
187-
// delete the row by index
188-
rows.splice(moveFromIndex, 1)
189-
// insert row copyOfMovingRow back in
190-
rows.splice(moveToIndex, 0, copyOfMovingRow)
184+
// Handle moving rows on the top-level, i.e. `array.0.text` -> `array.1.text`
185+
const { remainingFields, rows: topLevelRows } = separateRows(path, state)
186+
const copyOfMovingRow = topLevelRows[moveFromIndex]
187+
topLevelRows.splice(moveFromIndex, 1)
188+
topLevelRows.splice(moveToIndex, 0, copyOfMovingRow)
191189

192190
// modify array/block internal row state (i.e. collapsed, blockType)
193-
const rowStateCopy = [...(state[path]?.rows || [])]
194-
const movingRowState = { ...rowStateCopy[moveFromIndex] }
195-
rowStateCopy.splice(moveFromIndex, 1)
196-
rowStateCopy.splice(moveToIndex, 0, movingRowState)
191+
const rowsWithinField = [...(state[path]?.rows || [])]
192+
const copyOfMovingRow2 = { ...rowsWithinField[moveFromIndex] }
193+
rowsWithinField.splice(moveFromIndex, 1)
194+
rowsWithinField.splice(moveToIndex, 0, copyOfMovingRow2)
197195

198196
const newState = {
199197
...remainingFields,
200-
...flattenRows(path, rows),
198+
...flattenRows(path, topLevelRows),
201199
[path]: {
202200
...state[path],
203201
requiresRender: true,
204-
rows: rowStateCopy,
202+
rows: rowsWithinField,
205203
},
206204
}
207205

206+
// Do the same for custom components, i.e. `array.customComponents.RowLabels[0]` -> `array.customComponents.RowLabels[1]`
207+
// Do this _after_ initializing `newState` to avoid adding the `customComponents` key to the state if it doesn't exist
208+
if (newState[path]?.customComponents?.RowLabels) {
209+
const customComponents = {
210+
...newState[path].customComponents,
211+
RowLabels: [...newState[path].customComponents.RowLabels],
212+
}
213+
214+
// Ensure the array grows if necessary
215+
if (moveToIndex >= customComponents.RowLabels.length) {
216+
customComponents.RowLabels.length = moveToIndex + 1
217+
}
218+
219+
const copyOfMovingLabel = customComponents.RowLabels[moveFromIndex]
220+
221+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
222+
customComponents.RowLabels.splice(moveFromIndex, 1)
223+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
224+
customComponents.RowLabels.splice(moveToIndex, 0, copyOfMovingLabel)
225+
226+
newState[path].customComponents = customComponents
227+
}
228+
208229
return newState
209230
}
210231

test/fields/collections/Blocks/e2e.spec.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { BrowserContext, Page } from '@playwright/test'
33
import { expect, test } from '@playwright/test'
44
import { addBlock } from 'helpers/e2e/addBlock.js'
55
import { openBlocksDrawer } from 'helpers/e2e/openBlocksDrawer.js'
6+
import { reorderBlocks } from 'helpers/e2e/reorderBlocks.js'
67
import path from 'path'
78
import { fileURLToPath } from 'url'
89

@@ -289,6 +290,39 @@ describe('Block fields', () => {
289290
})
290291

291292
describe('row manipulation', () => {
293+
test('moving rows should immediately move custom row labels', async () => {
294+
await page.goto(url.create)
295+
296+
// first ensure that the first block has the custom header, and that the second block doesn't
297+
298+
await expect(
299+
page.locator('#field-blocks #blocks-row-0 .blocks-field__block-header'),
300+
).toHaveText('Custom Block Label: Content 01')
301+
302+
const secondBlockHeader = page.locator(
303+
'#field-blocks #blocks-row-1 .blocks-field__block-header',
304+
)
305+
306+
await expect(secondBlockHeader.locator('.blocks-field__block-pill')).toHaveText('Number')
307+
308+
await expect(secondBlockHeader.locator('input[id="blocks.1.blockName"]')).toHaveValue(
309+
'Second block',
310+
)
311+
312+
await reorderBlocks({
313+
page,
314+
fieldName: 'blocks',
315+
fromBlockIndex: 0,
316+
toBlockIndex: 1,
317+
})
318+
319+
// Important: do _not_ poll here, use `textContent()` instead of `toHaveText()`
320+
// This will prevent Playwright from polling for the change to the DOM
321+
expect(
322+
await page.locator('#field-blocks #blocks-row-1 .blocks-field__block-header').textContent(),
323+
).toMatch(/^Custom Block Label: Content/)
324+
})
325+
292326
describe('react hooks', () => {
293327
test('should add 2 new block rows', async () => {
294328
await page.goto(url.create)

0 commit comments

Comments
 (0)