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.
This commit is contained in:
@@ -4,8 +4,8 @@ import type { ClientCollectionConfig, ClientGlobalConfig } from 'payload'
|
||||
import type React from 'react'
|
||||
|
||||
import { getTranslation } from '@payloadcms/translations'
|
||||
import { useConfig, useLocale, useStepNav, useTranslation } from '@payloadcms/ui'
|
||||
import { fieldAffectsData, formatAdminURL } from 'payload/shared'
|
||||
import { useConfig, useDocumentTitle, useLocale, useStepNav, useTranslation } from '@payloadcms/ui'
|
||||
import { formatAdminURL } from 'payload/shared'
|
||||
import { useEffect } from 'react'
|
||||
|
||||
export const SetStepNav: React.FC<{
|
||||
@@ -15,7 +15,6 @@ export const SetStepNav: React.FC<{
|
||||
readonly isTrashed?: boolean
|
||||
versionToCreatedAtFormatted?: string
|
||||
versionToID?: string
|
||||
versionToUseAsTitle?: Record<string, string> | string
|
||||
}> = ({
|
||||
id,
|
||||
collectionConfig,
|
||||
@@ -23,12 +22,12 @@ export const SetStepNav: React.FC<{
|
||||
isTrashed,
|
||||
versionToCreatedAtFormatted,
|
||||
versionToID,
|
||||
versionToUseAsTitle,
|
||||
}) => {
|
||||
const { config } = useConfig()
|
||||
const { setStepNav } = useStepNav()
|
||||
const { i18n, t } = useTranslation()
|
||||
const locale = useLocale()
|
||||
const { title } = useDocumentTitle()
|
||||
|
||||
useEffect(() => {
|
||||
const {
|
||||
@@ -38,24 +37,7 @@ export const SetStepNav: React.FC<{
|
||||
if (collectionConfig) {
|
||||
const collectionSlug = collectionConfig.slug
|
||||
|
||||
const useAsTitle = collectionConfig.admin?.useAsTitle || 'id'
|
||||
const pluralLabel = collectionConfig.labels?.plural
|
||||
let docLabel = `[${t('general:untitled')}]`
|
||||
|
||||
const fields = collectionConfig.fields
|
||||
|
||||
const titleField = fields.find(
|
||||
(f) => fieldAffectsData(f) && 'name' in f && f.name === useAsTitle,
|
||||
)
|
||||
|
||||
if (titleField && versionToUseAsTitle) {
|
||||
docLabel =
|
||||
'localized' in titleField && titleField.localized
|
||||
? versionToUseAsTitle?.[locale.code] || docLabel
|
||||
: versionToUseAsTitle
|
||||
} else if (useAsTitle === 'id') {
|
||||
docLabel = String(id)
|
||||
}
|
||||
|
||||
const docBasePath: `/${string}` = isTrashed
|
||||
? `/collections/${collectionSlug}/trash/${id}`
|
||||
@@ -83,7 +65,7 @@ export const SetStepNav: React.FC<{
|
||||
|
||||
nav.push(
|
||||
{
|
||||
label: docLabel,
|
||||
label: title,
|
||||
url: formatAdminURL({
|
||||
adminRoute,
|
||||
path: docBasePath,
|
||||
@@ -139,7 +121,7 @@ export const SetStepNav: React.FC<{
|
||||
i18n,
|
||||
collectionConfig,
|
||||
globalConfig,
|
||||
versionToUseAsTitle,
|
||||
title,
|
||||
versionToCreatedAtFormatted,
|
||||
versionToID,
|
||||
])
|
||||
|
||||
@@ -40,7 +40,6 @@ export const DefaultVersionView: React.FC<DefaultVersionsViewProps> = ({
|
||||
VersionToCreatedAtLabel,
|
||||
versionToID,
|
||||
versionToStatus,
|
||||
versionToUseAsTitle,
|
||||
}) => {
|
||||
const { config, getEntityConfig } = useConfig()
|
||||
const { code } = useLocale()
|
||||
@@ -275,7 +274,6 @@ export const DefaultVersionView: React.FC<DefaultVersionsViewProps> = ({
|
||||
isTrashed={isTrashed}
|
||||
versionToCreatedAtFormatted={versionToCreatedAtFormatted}
|
||||
versionToID={versionToID}
|
||||
versionToUseAsTitle={versionToUseAsTitle}
|
||||
/>
|
||||
<Gutter className={`${baseClass}__diff-wrap`}>
|
||||
<SelectedLocalesContext value={{ selectedLocales: locales.map((locale) => locale.name) }}>
|
||||
|
||||
@@ -21,5 +21,4 @@ export type DefaultVersionsViewProps = {
|
||||
VersionToCreatedAtLabel: React.ReactNode
|
||||
versionToID?: string
|
||||
versionToStatus?: string
|
||||
versionToUseAsTitle?: string
|
||||
}
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import type { PayloadRequest, RelationshipField, TypeWithID } from 'payload'
|
||||
|
||||
import { fieldAffectsData, fieldIsPresentationalOnly, fieldShouldBeLocalized } from 'payload/shared'
|
||||
import {
|
||||
fieldAffectsData,
|
||||
fieldIsPresentationalOnly,
|
||||
fieldShouldBeLocalized,
|
||||
flattenTopLevelFields,
|
||||
} from 'payload/shared'
|
||||
|
||||
import type { PopulatedRelationshipValue } from './index.js'
|
||||
|
||||
@@ -32,7 +37,12 @@ export const generateLabelFromValue = ({
|
||||
const relatedCollection = req.payload.collections[relationTo].config
|
||||
|
||||
const useAsTitle = relatedCollection?.admin?.useAsTitle
|
||||
const useAsTitleField = relatedCollection.fields.find(
|
||||
|
||||
const flattenedRelatedCollectionFields = flattenTopLevelFields(relatedCollection.fields, {
|
||||
moveSubFieldsToTop: true,
|
||||
})
|
||||
|
||||
const useAsTitleField = flattenedRelatedCollectionFields.find(
|
||||
(f) => fieldAffectsData(f) && !fieldIsPresentationalOnly(f) && f.name === useAsTitle,
|
||||
)
|
||||
let titleFieldIsLocalized = false
|
||||
|
||||
@@ -411,11 +411,6 @@ export async function VersionView(props: DocumentViewServerProps) {
|
||||
})
|
||||
}
|
||||
|
||||
const useAsTitleFieldName = collectionConfig?.admin?.useAsTitle || 'id'
|
||||
const versionToUseAsTitle =
|
||||
useAsTitleFieldName === 'id'
|
||||
? String(versionTo.parent)
|
||||
: versionTo.version?.[useAsTitleFieldName]
|
||||
return (
|
||||
<DefaultVersionView
|
||||
canUpdate={docPermissions?.update}
|
||||
@@ -430,7 +425,6 @@ export async function VersionView(props: DocumentViewServerProps) {
|
||||
VersionToCreatedAtLabel={formatPill({ doc: versionTo, labelStyle: 'pill' })}
|
||||
versionToID={versionTo.id}
|
||||
versionToStatus={versionTo.version?._status}
|
||||
versionToUseAsTitle={versionToUseAsTitle}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -55,12 +55,17 @@ const DraftPosts: CollectionConfig = {
|
||||
},
|
||||
fields: [
|
||||
{
|
||||
name: 'title',
|
||||
type: 'text',
|
||||
label: 'Title',
|
||||
localized: true,
|
||||
required: true,
|
||||
unique: true,
|
||||
type: 'group',
|
||||
fields: [
|
||||
{
|
||||
name: 'title',
|
||||
type: 'text',
|
||||
label: 'Title',
|
||||
localized: true,
|
||||
required: true,
|
||||
unique: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'description',
|
||||
|
||||
@@ -236,7 +236,7 @@ describe('Versions', () => {
|
||||
const row2 = page.locator('tbody .row-2')
|
||||
const versionID = await row2.locator('.cell-id').textContent()
|
||||
await page.goto(`${savedDocURL}/versions/${versionID}`)
|
||||
await expect(page.locator('.render-field-diffs')).toBeVisible()
|
||||
await expect(page.locator('.render-field-diffs').first()).toBeVisible()
|
||||
await page.locator('.restore-version__restore-as-draft-button').click()
|
||||
await page.locator('button:has-text("Confirm")').click()
|
||||
await page.waitForURL(savedDocURL)
|
||||
@@ -259,7 +259,7 @@ describe('Versions', () => {
|
||||
const row2 = page.locator('tbody .row-2')
|
||||
const versionID = await row2.locator('.cell-id').textContent()
|
||||
await page.goto(`${savedDocURL}/versions/${versionID}`)
|
||||
await expect(page.locator('.render-field-diffs')).toBeVisible()
|
||||
await expect(page.locator('.render-field-diffs').first()).toBeVisible()
|
||||
await page.locator('.restore-version .popup__trigger-wrap button').click()
|
||||
await page.getByRole('button', { name: 'Restore as draft' }).click()
|
||||
await page.locator('button:has-text("Confirm")').click()
|
||||
@@ -1482,6 +1482,7 @@ describe('Versions', () => {
|
||||
describe('Versions diff view', () => {
|
||||
let postID: string
|
||||
let versionID: string
|
||||
let oldVersionID: string
|
||||
let diffID: string
|
||||
let versionDiffID: string
|
||||
|
||||
@@ -1507,7 +1508,7 @@ describe('Versions', () => {
|
||||
draft: true,
|
||||
depth: 0,
|
||||
data: {
|
||||
title: 'draft post',
|
||||
title: 'current draft post title',
|
||||
description: 'draft description',
|
||||
blocksField: [
|
||||
{
|
||||
@@ -1521,7 +1522,7 @@ describe('Versions', () => {
|
||||
|
||||
const versions = await payload.findVersions({
|
||||
collection: draftCollectionSlug,
|
||||
limit: 1,
|
||||
limit: 2,
|
||||
depth: 0,
|
||||
where: {
|
||||
parent: { equals: postID },
|
||||
@@ -1529,6 +1530,7 @@ describe('Versions', () => {
|
||||
})
|
||||
|
||||
versionID = versions.docs[0].id
|
||||
oldVersionID = versions.docs[1].id
|
||||
|
||||
const diffDoc = (
|
||||
await payload.find({
|
||||
@@ -1554,7 +1556,7 @@ describe('Versions', () => {
|
||||
versionDiffID = versionDiff.id
|
||||
})
|
||||
|
||||
async function navigateToDraftVersionView() {
|
||||
async function navigateToDraftVersionView(versionID: string) {
|
||||
const versionURL = `${serverURL}/admin/collections/${draftCollectionSlug}/${postID}/versions/${versionID}`
|
||||
await page.goto(versionURL)
|
||||
await expect(page.locator('.render-field-diffs').first()).toBeVisible()
|
||||
@@ -1567,12 +1569,22 @@ describe('Versions', () => {
|
||||
}
|
||||
|
||||
test('should render diff', async () => {
|
||||
await navigateToDraftVersionView()
|
||||
await navigateToDraftVersionView(versionID)
|
||||
expect(true).toBe(true)
|
||||
})
|
||||
|
||||
test('should show the current version title in step nav for all versions', async () => {
|
||||
await navigateToDraftVersionView(versionID)
|
||||
// Document title part of the step nav should be the current version title
|
||||
await expect(page.locator('.step-nav')).toContainText('current draft post title')
|
||||
|
||||
await navigateToDraftVersionView(oldVersionID)
|
||||
// Document title part of the step nav should still be the current version title
|
||||
await expect(page.locator('.step-nav')).toContainText('current draft post title')
|
||||
})
|
||||
|
||||
test('should render diff for nested fields', async () => {
|
||||
await navigateToDraftVersionView()
|
||||
await navigateToDraftVersionView(versionID)
|
||||
|
||||
const blocksDiffLabel = page.getByText('Blocks Field', { exact: true })
|
||||
await expect(blocksDiffLabel).toBeVisible()
|
||||
@@ -1591,7 +1603,7 @@ describe('Versions', () => {
|
||||
})
|
||||
|
||||
test('should render diff collapser for nested fields', async () => {
|
||||
await navigateToDraftVersionView()
|
||||
await navigateToDraftVersionView(versionID)
|
||||
|
||||
const blocksDiffLabel = page.getByText('Blocks Field', { exact: true })
|
||||
await expect(blocksDiffLabel).toBeVisible()
|
||||
|
||||
@@ -752,7 +752,7 @@ describe('Versions', () => {
|
||||
|
||||
expect(updateManyResult.docs).toHaveLength(0)
|
||||
expect(updateManyResult.errors).toStrictEqual([
|
||||
{ id: doc.id, message: 'The following field is invalid: Title' },
|
||||
{ id: doc.id, message: 'The following field is invalid: Group > Title' },
|
||||
])
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user