Skip to content

Commit 794bf82

Browse files
authored
fix(next): version diff view shows correct document title in step nav (#13713)
This PR fixes two bugs in the version diff view SetStepNav component ### Bug 1: Document title isn't shown correctly in the step navigation if the field of `useAsTitle` is nested inside a presentational field. The StepNav shows the title of the document consistently throughout every view except the version diff view. In the version diff view, the document title is always `[Untitled]` if the field of `useAsTitle` is nested inside presentational fields. Below is a video demo of the bug: https://github.com/user-attachments/assets/23cb140a-b6d3-4d39-babf-5e4878651869 This happens because the fields of the collection/global aren't flattened inside SetStepNav and thus the component is not accessing the field data correctly. This results in the title being `null` causing the fallback title to be shown. ### Bug 2: Step navigation shows the title of the version viewed, not the current version The StepNav component takes the title of the current version viewed. This causes the second part of the navigation path to change between versions which is inconsistent between other views and doesn't seem intentional, although it could be. Below is a video of the bug with the first bug fixed by flattening the fields: https://github.com/user-attachments/assets/e5beb9b3-8e2e-4232-b1e5-5cce720e46b9 This happens due to the fact that the title is taken from the `useAsTitle` field of the **viewed** version rather than the **current** version. This bug is fixed by using the `useDocumentTitle` hook from the ui package instead of passing the version's `useAsTitle` data down the component tree. The final state of the step navigation is shown in the following video: https://github.com/user-attachments/assets/a69d5088-e7ee-43be-8f47-d9775d43dde9 I also added a test to test that the title part in the step navigation stays consistent between versions and implicitly also tests that the document title is shown correctly in the step nav if the field of `useAsTitle` is a nested inside a presentational field.
1 parent 9f0573d commit 794bf82

File tree

8 files changed

+49
-49
lines changed

8 files changed

+49
-49
lines changed

packages/next/src/views/Version/Default/SetStepNav.tsx

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import type { ClientCollectionConfig, ClientGlobalConfig } from 'payload'
44
import type React from 'react'
55

66
import { getTranslation } from '@payloadcms/translations'
7-
import { useConfig, useLocale, useStepNav, useTranslation } from '@payloadcms/ui'
8-
import { fieldAffectsData, formatAdminURL } from 'payload/shared'
7+
import { useConfig, useDocumentTitle, useLocale, useStepNav, useTranslation } from '@payloadcms/ui'
8+
import { formatAdminURL } from 'payload/shared'
99
import { useEffect } from 'react'
1010

1111
export const SetStepNav: React.FC<{
@@ -15,20 +15,19 @@ export const SetStepNav: React.FC<{
1515
readonly isTrashed?: boolean
1616
versionToCreatedAtFormatted?: string
1717
versionToID?: string
18-
versionToUseAsTitle?: Record<string, string> | string
1918
}> = ({
2019
id,
2120
collectionConfig,
2221
globalConfig,
2322
isTrashed,
2423
versionToCreatedAtFormatted,
2524
versionToID,
26-
versionToUseAsTitle,
2725
}) => {
2826
const { config } = useConfig()
2927
const { setStepNav } = useStepNav()
3028
const { i18n, t } = useTranslation()
3129
const locale = useLocale()
30+
const { title } = useDocumentTitle()
3231

3332
useEffect(() => {
3433
const {
@@ -38,24 +37,7 @@ export const SetStepNav: React.FC<{
3837
if (collectionConfig) {
3938
const collectionSlug = collectionConfig.slug
4039

41-
const useAsTitle = collectionConfig.admin?.useAsTitle || 'id'
4240
const pluralLabel = collectionConfig.labels?.plural
43-
let docLabel = `[${t('general:untitled')}]`
44-
45-
const fields = collectionConfig.fields
46-
47-
const titleField = fields.find(
48-
(f) => fieldAffectsData(f) && 'name' in f && f.name === useAsTitle,
49-
)
50-
51-
if (titleField && versionToUseAsTitle) {
52-
docLabel =
53-
'localized' in titleField && titleField.localized
54-
? versionToUseAsTitle?.[locale.code] || docLabel
55-
: versionToUseAsTitle
56-
} else if (useAsTitle === 'id') {
57-
docLabel = String(id)
58-
}
5941

6042
const docBasePath: `/${string}` = isTrashed
6143
? `/collections/${collectionSlug}/trash/${id}`
@@ -83,7 +65,7 @@ export const SetStepNav: React.FC<{
8365

8466
nav.push(
8567
{
86-
label: docLabel,
68+
label: title,
8769
url: formatAdminURL({
8870
adminRoute,
8971
path: docBasePath,
@@ -139,7 +121,7 @@ export const SetStepNav: React.FC<{
139121
i18n,
140122
collectionConfig,
141123
globalConfig,
142-
versionToUseAsTitle,
124+
title,
143125
versionToCreatedAtFormatted,
144126
versionToID,
145127
])

packages/next/src/views/Version/Default/index.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ export const DefaultVersionView: React.FC<DefaultVersionsViewProps> = ({
4040
VersionToCreatedAtLabel,
4141
versionToID,
4242
versionToStatus,
43-
versionToUseAsTitle,
4443
}) => {
4544
const { config, getEntityConfig } = useConfig()
4645
const { code } = useLocale()
@@ -275,7 +274,6 @@ export const DefaultVersionView: React.FC<DefaultVersionsViewProps> = ({
275274
isTrashed={isTrashed}
276275
versionToCreatedAtFormatted={versionToCreatedAtFormatted}
277276
versionToID={versionToID}
278-
versionToUseAsTitle={versionToUseAsTitle}
279277
/>
280278
<Gutter className={`${baseClass}__diff-wrap`}>
281279
<SelectedLocalesContext value={{ selectedLocales: locales.map((locale) => locale.name) }}>

packages/next/src/views/Version/Default/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,4 @@ export type DefaultVersionsViewProps = {
2121
VersionToCreatedAtLabel: React.ReactNode
2222
versionToID?: string
2323
versionToStatus?: string
24-
versionToUseAsTitle?: string
2524
}

packages/next/src/views/Version/RenderFieldsToDiff/fields/Relationship/generateLabelFromValue.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import type { PayloadRequest, RelationshipField, TypeWithID } from 'payload'
22

3-
import { fieldAffectsData, fieldIsPresentationalOnly, fieldShouldBeLocalized } from 'payload/shared'
3+
import {
4+
fieldAffectsData,
5+
fieldIsPresentationalOnly,
6+
fieldShouldBeLocalized,
7+
flattenTopLevelFields,
8+
} from 'payload/shared'
49

510
import type { PopulatedRelationshipValue } from './index.js'
611

@@ -32,7 +37,12 @@ export const generateLabelFromValue = ({
3237
const relatedCollection = req.payload.collections[relationTo].config
3338

3439
const useAsTitle = relatedCollection?.admin?.useAsTitle
35-
const useAsTitleField = relatedCollection.fields.find(
40+
41+
const flattenedRelatedCollectionFields = flattenTopLevelFields(relatedCollection.fields, {
42+
moveSubFieldsToTop: true,
43+
})
44+
45+
const useAsTitleField = flattenedRelatedCollectionFields.find(
3646
(f) => fieldAffectsData(f) && !fieldIsPresentationalOnly(f) && f.name === useAsTitle,
3747
)
3848
let titleFieldIsLocalized = false

packages/next/src/views/Version/index.tsx

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -411,11 +411,6 @@ export async function VersionView(props: DocumentViewServerProps) {
411411
})
412412
}
413413

414-
const useAsTitleFieldName = collectionConfig?.admin?.useAsTitle || 'id'
415-
const versionToUseAsTitle =
416-
useAsTitleFieldName === 'id'
417-
? String(versionTo.parent)
418-
: versionTo.version?.[useAsTitleFieldName]
419414
return (
420415
<DefaultVersionView
421416
canUpdate={docPermissions?.update}
@@ -430,7 +425,6 @@ export async function VersionView(props: DocumentViewServerProps) {
430425
VersionToCreatedAtLabel={formatPill({ doc: versionTo, labelStyle: 'pill' })}
431426
versionToID={versionTo.id}
432427
versionToStatus={versionTo.version?._status}
433-
versionToUseAsTitle={versionToUseAsTitle}
434428
/>
435429
)
436430
}

test/versions/collections/Drafts.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,17 @@ const DraftPosts: CollectionConfig = {
5555
},
5656
fields: [
5757
{
58-
name: 'title',
59-
type: 'text',
60-
label: 'Title',
61-
localized: true,
62-
required: true,
63-
unique: true,
58+
type: 'group',
59+
fields: [
60+
{
61+
name: 'title',
62+
type: 'text',
63+
label: 'Title',
64+
localized: true,
65+
required: true,
66+
unique: true,
67+
},
68+
],
6469
},
6570
{
6671
name: 'description',

test/versions/e2e.spec.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ describe('Versions', () => {
236236
const row2 = page.locator('tbody .row-2')
237237
const versionID = await row2.locator('.cell-id').textContent()
238238
await page.goto(`${savedDocURL}/versions/${versionID}`)
239-
await expect(page.locator('.render-field-diffs')).toBeVisible()
239+
await expect(page.locator('.render-field-diffs').first()).toBeVisible()
240240
await page.locator('.restore-version__restore-as-draft-button').click()
241241
await page.locator('button:has-text("Confirm")').click()
242242
await page.waitForURL(savedDocURL)
@@ -259,7 +259,7 @@ describe('Versions', () => {
259259
const row2 = page.locator('tbody .row-2')
260260
const versionID = await row2.locator('.cell-id').textContent()
261261
await page.goto(`${savedDocURL}/versions/${versionID}`)
262-
await expect(page.locator('.render-field-diffs')).toBeVisible()
262+
await expect(page.locator('.render-field-diffs').first()).toBeVisible()
263263
await page.locator('.restore-version .popup__trigger-wrap button').click()
264264
await page.getByRole('button', { name: 'Restore as draft' }).click()
265265
await page.locator('button:has-text("Confirm")').click()
@@ -1482,6 +1482,7 @@ describe('Versions', () => {
14821482
describe('Versions diff view', () => {
14831483
let postID: string
14841484
let versionID: string
1485+
let oldVersionID: string
14851486
let diffID: string
14861487
let versionDiffID: string
14871488

@@ -1507,7 +1508,7 @@ describe('Versions', () => {
15071508
draft: true,
15081509
depth: 0,
15091510
data: {
1510-
title: 'draft post',
1511+
title: 'current draft post title',
15111512
description: 'draft description',
15121513
blocksField: [
15131514
{
@@ -1521,14 +1522,15 @@ describe('Versions', () => {
15211522

15221523
const versions = await payload.findVersions({
15231524
collection: draftCollectionSlug,
1524-
limit: 1,
1525+
limit: 2,
15251526
depth: 0,
15261527
where: {
15271528
parent: { equals: postID },
15281529
},
15291530
})
15301531

15311532
versionID = versions.docs[0].id
1533+
oldVersionID = versions.docs[1].id
15321534

15331535
const diffDoc = (
15341536
await payload.find({
@@ -1554,7 +1556,7 @@ describe('Versions', () => {
15541556
versionDiffID = versionDiff.id
15551557
})
15561558

1557-
async function navigateToDraftVersionView() {
1559+
async function navigateToDraftVersionView(versionID: string) {
15581560
const versionURL = `${serverURL}/admin/collections/${draftCollectionSlug}/${postID}/versions/${versionID}`
15591561
await page.goto(versionURL)
15601562
await expect(page.locator('.render-field-diffs').first()).toBeVisible()
@@ -1567,12 +1569,22 @@ describe('Versions', () => {
15671569
}
15681570

15691571
test('should render diff', async () => {
1570-
await navigateToDraftVersionView()
1572+
await navigateToDraftVersionView(versionID)
15711573
expect(true).toBe(true)
15721574
})
15731575

1576+
test('should show the current version title in step nav for all versions', async () => {
1577+
await navigateToDraftVersionView(versionID)
1578+
// Document title part of the step nav should be the current version title
1579+
await expect(page.locator('.step-nav')).toContainText('current draft post title')
1580+
1581+
await navigateToDraftVersionView(oldVersionID)
1582+
// Document title part of the step nav should still be the current version title
1583+
await expect(page.locator('.step-nav')).toContainText('current draft post title')
1584+
})
1585+
15741586
test('should render diff for nested fields', async () => {
1575-
await navigateToDraftVersionView()
1587+
await navigateToDraftVersionView(versionID)
15761588

15771589
const blocksDiffLabel = page.getByText('Blocks Field', { exact: true })
15781590
await expect(blocksDiffLabel).toBeVisible()
@@ -1591,7 +1603,7 @@ describe('Versions', () => {
15911603
})
15921604

15931605
test('should render diff collapser for nested fields', async () => {
1594-
await navigateToDraftVersionView()
1606+
await navigateToDraftVersionView(versionID)
15951607

15961608
const blocksDiffLabel = page.getByText('Blocks Field', { exact: true })
15971609
await expect(blocksDiffLabel).toBeVisible()

test/versions/int.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ describe('Versions', () => {
752752

753753
expect(updateManyResult.docs).toHaveLength(0)
754754
expect(updateManyResult.errors).toStrictEqual([
755-
{ id: doc.id, message: 'The following field is invalid: Title' },
755+
{ id: doc.id, message: 'The following field is invalid: Group > Title' },
756756
])
757757
})
758758

0 commit comments

Comments
 (0)