fix(ui): prevent document drawer from remounting on save (#13005)
Supersedes #12992. Partially closes #12975. Right now autosave-enabled documents opened within a drawer will unnecessarily remount on every autosave interval, causing loss of input focus, etc. This makes it nearly impossible to edit these documents, especially if the interval is very short. But the same is true for non-autosave documents when "manually" saving, e.g. pressing the "save draft" or "publish changes" buttons. This has gone largely unnoticed, however, as the user has already lost focus of the form to interact with these controls, and they somewhat expect this behavior or at least accept it. Now, the form remains mounted across autosave events and the user's cursor never loses focus. Much better. Before: https://github.com/user-attachments/assets/a159cdc0-21e8-45f6-a14d-6256e53bc3df After: https://github.com/user-attachments/assets/cd697439-1cd3-4033-8330-a5642f7810e8 Related: #12842 --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1210689077645986
This commit is contained in:
@@ -42,14 +42,16 @@ export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
|
|||||||
|
|
||||||
const [DocumentView, setDocumentView] = useState<React.ReactNode>(undefined)
|
const [DocumentView, setDocumentView] = useState<React.ReactNode>(undefined)
|
||||||
const [isLoading, setIsLoading] = useState(true)
|
const [isLoading, setIsLoading] = useState(true)
|
||||||
const hasRenderedDocument = useRef(false)
|
const hasInitialized = useRef(false)
|
||||||
|
|
||||||
const getDocumentView = useCallback(
|
const getDocumentView = useCallback(
|
||||||
(docID?: number | string) => {
|
(docID?: number | string, showLoadingIndicator: boolean = false) => {
|
||||||
const controller = handleAbortRef(abortGetDocumentViewRef)
|
const controller = handleAbortRef(abortGetDocumentViewRef)
|
||||||
|
|
||||||
const fetchDocumentView = async () => {
|
const fetchDocumentView = async () => {
|
||||||
|
if (showLoadingIndicator) {
|
||||||
setIsLoading(true)
|
setIsLoading(true)
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const result = await renderDocument({
|
const result = await renderDocument({
|
||||||
@@ -141,13 +143,13 @@ export const DocumentDrawerContent: React.FC<DocumentDrawerProps> = ({
|
|||||||
)
|
)
|
||||||
|
|
||||||
const clearDoc = useCallback(() => {
|
const clearDoc = useCallback(() => {
|
||||||
getDocumentView()
|
getDocumentView(undefined, true)
|
||||||
}, [getDocumentView])
|
}, [getDocumentView])
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!DocumentView && !hasRenderedDocument.current) {
|
if (!DocumentView && !hasInitialized.current) {
|
||||||
getDocumentView(existingDocID)
|
getDocumentView(existingDocID, true)
|
||||||
hasRenderedDocument.current = true
|
hasInitialized.current = true
|
||||||
}
|
}
|
||||||
}, [DocumentView, getDocumentView, existingDocID])
|
}, [DocumentView, getDocumentView, existingDocID])
|
||||||
|
|
||||||
|
|||||||
28
packages/ui/src/hooks/useControllableState.ts
Normal file
28
packages/ui/src/hooks/useControllableState.ts
Normal file
@@ -0,0 +1,28 @@
|
|||||||
|
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A hook for managing state that can be controlled by props but also overridden locally.
|
||||||
|
* Props always take precedence if they change, but local state can override them temporarily.
|
||||||
|
*/
|
||||||
|
export function useControllableState<T>(
|
||||||
|
propValue: T,
|
||||||
|
defaultValue?: T,
|
||||||
|
): [T, (value: ((prev: T) => T) | T) => void] {
|
||||||
|
const [localValue, setLocalValue] = useState<T>(propValue ?? defaultValue)
|
||||||
|
const initialRenderRef = useRef(true)
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
if (initialRenderRef.current) {
|
||||||
|
initialRenderRef.current = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
setLocalValue(propValue)
|
||||||
|
}, [propValue])
|
||||||
|
|
||||||
|
const setValue = useCallback((value: ((prev: T) => T) | T) => {
|
||||||
|
setLocalValue(value)
|
||||||
|
}, [])
|
||||||
|
|
||||||
|
return [localValue, setValue]
|
||||||
|
}
|
||||||
@@ -1,9 +1,10 @@
|
|||||||
'use client'
|
'use client'
|
||||||
import type { ClientUser, DocumentPreferences, SanitizedDocumentPermissions } from 'payload'
|
import type { ClientUser, DocumentPreferences } from 'payload'
|
||||||
|
|
||||||
import * as qs from 'qs-esm'
|
import * as qs from 'qs-esm'
|
||||||
import React, { createContext, use, useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
import React, { createContext, use, useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||||
|
|
||||||
|
import { useControllableState } from '../../hooks/useControllableState.js'
|
||||||
import { useAuth } from '../../providers/Auth/index.js'
|
import { useAuth } from '../../providers/Auth/index.js'
|
||||||
import { requests } from '../../utilities/api.js'
|
import { requests } from '../../utilities/api.js'
|
||||||
import { formatDocTitle } from '../../utilities/formatDocTitle/index.js'
|
import { formatDocTitle } from '../../utilities/formatDocTitle/index.js'
|
||||||
@@ -45,12 +46,11 @@ const DocumentInfo: React.FC<
|
|||||||
versionCount: versionCountFromProps,
|
versionCount: versionCountFromProps,
|
||||||
} = props
|
} = props
|
||||||
|
|
||||||
const [docPermissions, setDocPermissions] =
|
const [docPermissions, setDocPermissions] = useControllableState(docPermissionsFromProps)
|
||||||
useState<SanitizedDocumentPermissions>(docPermissionsFromProps)
|
|
||||||
|
|
||||||
const [hasSavePermission, setHasSavePermission] = useState<boolean>(hasSavePermissionFromProps)
|
const [hasSavePermission, setHasSavePermission] = useControllableState(hasSavePermissionFromProps)
|
||||||
|
|
||||||
const [hasPublishPermission, setHasPublishPermission] = useState<boolean>(
|
const [hasPublishPermission, setHasPublishPermission] = useControllableState(
|
||||||
hasPublishPermissionFromProps,
|
hasPublishPermissionFromProps,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -101,15 +101,24 @@ const DocumentInfo: React.FC<
|
|||||||
unpublishedVersionCountFromProps,
|
unpublishedVersionCountFromProps,
|
||||||
)
|
)
|
||||||
|
|
||||||
const [documentIsLocked, setDocumentIsLocked] = useState<boolean | undefined>(isLockedFromProps)
|
const [documentIsLocked, setDocumentIsLocked] = useControllableState<boolean | undefined>(
|
||||||
const [currentEditor, setCurrentEditor] = useState<ClientUser | null>(currentEditorFromProps)
|
isLockedFromProps,
|
||||||
const [lastUpdateTime, setLastUpdateTime] = useState<number>(lastUpdateTimeFromProps)
|
)
|
||||||
const [savedDocumentData, setSavedDocumentData] = useState(initialData)
|
const [currentEditor, setCurrentEditor] = useControllableState<ClientUser | null>(
|
||||||
const [uploadStatus, setUploadStatus] = useState<'failed' | 'idle' | 'uploading'>('idle')
|
currentEditorFromProps,
|
||||||
|
)
|
||||||
|
const [lastUpdateTime, setLastUpdateTime] = useControllableState<number>(lastUpdateTimeFromProps)
|
||||||
|
const [savedDocumentData, setSavedDocumentData] = useControllableState(initialData)
|
||||||
|
const [uploadStatus, setUploadStatus] = useControllableState<'failed' | 'idle' | 'uploading'>(
|
||||||
|
'idle',
|
||||||
|
)
|
||||||
|
|
||||||
const updateUploadStatus = useCallback((status: 'failed' | 'idle' | 'uploading') => {
|
const updateUploadStatus = useCallback(
|
||||||
|
(status: 'failed' | 'idle' | 'uploading') => {
|
||||||
setUploadStatus(status)
|
setUploadStatus(status)
|
||||||
}, [])
|
},
|
||||||
|
[setUploadStatus],
|
||||||
|
)
|
||||||
|
|
||||||
const { getPreference, setPreference } = usePreferences()
|
const { getPreference, setPreference } = usePreferences()
|
||||||
const { code: locale } = useLocale()
|
const { code: locale } = useLocale()
|
||||||
@@ -170,7 +179,7 @@ const DocumentInfo: React.FC<
|
|||||||
console.error('Failed to unlock the document', error)
|
console.error('Failed to unlock the document', error)
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[serverURL, api, globalSlug],
|
[serverURL, api, globalSlug, setDocumentIsLocked],
|
||||||
)
|
)
|
||||||
|
|
||||||
const updateDocumentEditor = useCallback(
|
const updateDocumentEditor = useCallback(
|
||||||
@@ -279,7 +288,7 @@ const DocumentInfo: React.FC<
|
|||||||
(json) => {
|
(json) => {
|
||||||
setSavedDocumentData(json)
|
setSavedDocumentData(json)
|
||||||
},
|
},
|
||||||
[],
|
[setSavedDocumentData],
|
||||||
)
|
)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -436,10 +436,15 @@ describe('Access Control', () => {
|
|||||||
const documentDrawer = page.locator(`[id^=doc-drawer_${createNotUpdateCollectionSlug}_1_]`)
|
const documentDrawer = page.locator(`[id^=doc-drawer_${createNotUpdateCollectionSlug}_1_]`)
|
||||||
await expect(documentDrawer).toBeVisible()
|
await expect(documentDrawer).toBeVisible()
|
||||||
await expect(documentDrawer.locator('#action-save')).toBeVisible()
|
await expect(documentDrawer.locator('#action-save')).toBeVisible()
|
||||||
|
|
||||||
await documentDrawer.locator('#field-name').fill('name')
|
await documentDrawer.locator('#field-name').fill('name')
|
||||||
await expect(documentDrawer.locator('#field-name')).toHaveValue('name')
|
await expect(documentDrawer.locator('#field-name')).toHaveValue('name')
|
||||||
await documentDrawer.locator('#action-save').click()
|
|
||||||
await expect(page.locator('.payload-toast-container')).toContainText('successfully')
|
await saveDocAndAssert(
|
||||||
|
page,
|
||||||
|
`[id^=doc-drawer_${createNotUpdateCollectionSlug}_1_] #action-save`,
|
||||||
|
)
|
||||||
|
|
||||||
await expect(documentDrawer.locator('#action-save')).toBeHidden()
|
await expect(documentDrawer.locator('#action-save')).toBeHidden()
|
||||||
await expect(documentDrawer.locator('#field-name')).toBeDisabled()
|
await expect(documentDrawer.locator('#field-name')).toBeDisabled()
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -95,7 +95,6 @@ export interface Config {
|
|||||||
'auth-collection': AuthCollection;
|
'auth-collection': AuthCollection;
|
||||||
'payload-locked-documents': PayloadLockedDocument;
|
'payload-locked-documents': PayloadLockedDocument;
|
||||||
'payload-preferences': PayloadPreference;
|
'payload-preferences': PayloadPreference;
|
||||||
'payload-sessions': PayloadSession;
|
|
||||||
'payload-migrations': PayloadMigration;
|
'payload-migrations': PayloadMigration;
|
||||||
};
|
};
|
||||||
collectionsJoins: {};
|
collectionsJoins: {};
|
||||||
@@ -126,7 +125,6 @@ export interface Config {
|
|||||||
'auth-collection': AuthCollectionSelect<false> | AuthCollectionSelect<true>;
|
'auth-collection': AuthCollectionSelect<false> | AuthCollectionSelect<true>;
|
||||||
'payload-locked-documents': PayloadLockedDocumentsSelect<false> | PayloadLockedDocumentsSelect<true>;
|
'payload-locked-documents': PayloadLockedDocumentsSelect<false> | PayloadLockedDocumentsSelect<true>;
|
||||||
'payload-preferences': PayloadPreferencesSelect<false> | PayloadPreferencesSelect<true>;
|
'payload-preferences': PayloadPreferencesSelect<false> | PayloadPreferencesSelect<true>;
|
||||||
'payload-sessions': PayloadSessionsSelect<false> | PayloadSessionsSelect<true>;
|
|
||||||
'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
|
'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
|
||||||
};
|
};
|
||||||
db: {
|
db: {
|
||||||
@@ -232,6 +230,13 @@ export interface User {
|
|||||||
hash?: string | null;
|
hash?: string | null;
|
||||||
loginAttempts?: number | null;
|
loginAttempts?: number | null;
|
||||||
lockUntil?: string | null;
|
lockUntil?: string | null;
|
||||||
|
sessions?:
|
||||||
|
| {
|
||||||
|
id: string;
|
||||||
|
createdAt?: string | null;
|
||||||
|
expiresAt: string;
|
||||||
|
}[]
|
||||||
|
| null;
|
||||||
password?: string | null;
|
password?: string | null;
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
@@ -249,6 +254,13 @@ export interface PublicUser {
|
|||||||
hash?: string | null;
|
hash?: string | null;
|
||||||
loginAttempts?: number | null;
|
loginAttempts?: number | null;
|
||||||
lockUntil?: string | null;
|
lockUntil?: string | null;
|
||||||
|
sessions?:
|
||||||
|
| {
|
||||||
|
id: string;
|
||||||
|
createdAt?: string | null;
|
||||||
|
expiresAt: string;
|
||||||
|
}[]
|
||||||
|
| null;
|
||||||
password?: string | null;
|
password?: string | null;
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
@@ -740,6 +752,13 @@ export interface AuthCollection {
|
|||||||
_verificationToken?: string | null;
|
_verificationToken?: string | null;
|
||||||
loginAttempts?: number | null;
|
loginAttempts?: number | null;
|
||||||
lockUntil?: string | null;
|
lockUntil?: string | null;
|
||||||
|
sessions?:
|
||||||
|
| {
|
||||||
|
id: string;
|
||||||
|
createdAt?: string | null;
|
||||||
|
expiresAt: string;
|
||||||
|
}[]
|
||||||
|
| null;
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
@@ -893,26 +912,6 @@ export interface PayloadPreference {
|
|||||||
updatedAt: string;
|
updatedAt: string;
|
||||||
createdAt: string;
|
createdAt: string;
|
||||||
}
|
}
|
||||||
/**
|
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
|
||||||
* via the `definition` "payload-sessions".
|
|
||||||
*/
|
|
||||||
export interface PayloadSession {
|
|
||||||
id: string;
|
|
||||||
session: string;
|
|
||||||
expiration: string;
|
|
||||||
user:
|
|
||||||
| {
|
|
||||||
relationTo: 'users';
|
|
||||||
value: string | User;
|
|
||||||
}
|
|
||||||
| {
|
|
||||||
relationTo: 'public-users';
|
|
||||||
value: string | PublicUser;
|
|
||||||
};
|
|
||||||
updatedAt: string;
|
|
||||||
createdAt: string;
|
|
||||||
}
|
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
* via the `definition` "payload-migrations".
|
* via the `definition` "payload-migrations".
|
||||||
@@ -939,6 +938,13 @@ export interface UsersSelect<T extends boolean = true> {
|
|||||||
hash?: T;
|
hash?: T;
|
||||||
loginAttempts?: T;
|
loginAttempts?: T;
|
||||||
lockUntil?: T;
|
lockUntil?: T;
|
||||||
|
sessions?:
|
||||||
|
| T
|
||||||
|
| {
|
||||||
|
id?: T;
|
||||||
|
createdAt?: T;
|
||||||
|
expiresAt?: T;
|
||||||
|
};
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
@@ -954,6 +960,13 @@ export interface PublicUsersSelect<T extends boolean = true> {
|
|||||||
hash?: T;
|
hash?: T;
|
||||||
loginAttempts?: T;
|
loginAttempts?: T;
|
||||||
lockUntil?: T;
|
lockUntil?: T;
|
||||||
|
sessions?:
|
||||||
|
| T
|
||||||
|
| {
|
||||||
|
id?: T;
|
||||||
|
createdAt?: T;
|
||||||
|
expiresAt?: T;
|
||||||
|
};
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
@@ -1294,6 +1307,13 @@ export interface AuthCollectionSelect<T extends boolean = true> {
|
|||||||
_verificationToken?: T;
|
_verificationToken?: T;
|
||||||
loginAttempts?: T;
|
loginAttempts?: T;
|
||||||
lockUntil?: T;
|
lockUntil?: T;
|
||||||
|
sessions?:
|
||||||
|
| T
|
||||||
|
| {
|
||||||
|
id?: T;
|
||||||
|
createdAt?: T;
|
||||||
|
expiresAt?: T;
|
||||||
|
};
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
@@ -1317,17 +1337,6 @@ export interface PayloadPreferencesSelect<T extends boolean = true> {
|
|||||||
updatedAt?: T;
|
updatedAt?: T;
|
||||||
createdAt?: T;
|
createdAt?: T;
|
||||||
}
|
}
|
||||||
/**
|
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
|
||||||
* via the `definition` "payload-sessions_select".
|
|
||||||
*/
|
|
||||||
export interface PayloadSessionsSelect<T extends boolean = true> {
|
|
||||||
session?: T;
|
|
||||||
expiration?: T;
|
|
||||||
user?: T;
|
|
||||||
updatedAt?: T;
|
|
||||||
createdAt?: T;
|
|
||||||
}
|
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
* via the `definition` "payload-migrations_select".
|
* via the `definition` "payload-migrations_select".
|
||||||
|
|||||||
@@ -363,6 +363,43 @@ describe('Document View', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe('drawers', () => {
|
describe('drawers', () => {
|
||||||
|
test('document drawers do not unmount across save events', async () => {
|
||||||
|
// Navigate to a post document
|
||||||
|
await navigateToDoc(page, postsUrl)
|
||||||
|
|
||||||
|
// Open the relationship drawer
|
||||||
|
await page
|
||||||
|
.locator('.field-type.relationship .relationship--single-value__drawer-toggler')
|
||||||
|
.click()
|
||||||
|
|
||||||
|
const drawer = page.locator('[id^=doc-drawer_posts_1_]')
|
||||||
|
const drawerEditView = drawer.locator('.drawer__content .collection-edit')
|
||||||
|
await expect(drawerEditView).toBeVisible()
|
||||||
|
|
||||||
|
const drawerTitleField = drawerEditView.locator('#field-title')
|
||||||
|
const testTitle = 'Test Title for Persistence'
|
||||||
|
await drawerTitleField.fill(testTitle)
|
||||||
|
await expect(drawerTitleField).toHaveValue(testTitle)
|
||||||
|
|
||||||
|
await drawerEditView.evaluate((el) => {
|
||||||
|
el.setAttribute('data-test-instance', 'This is a test')
|
||||||
|
})
|
||||||
|
|
||||||
|
await expect(drawerEditView).toHaveAttribute('data-test-instance', 'This is a test')
|
||||||
|
|
||||||
|
await saveDocAndAssert(page, '[id^=doc-drawer_posts_1_] .drawer__content #action-save')
|
||||||
|
|
||||||
|
await expect(drawerEditView).toBeVisible()
|
||||||
|
await expect(drawerTitleField).toHaveValue(testTitle)
|
||||||
|
|
||||||
|
// Verify the element instance hasn't changed (i.e., it wasn't re-mounted and discarded the custom attribute)
|
||||||
|
await expect
|
||||||
|
.poll(async () => {
|
||||||
|
return await drawerEditView.getAttribute('data-test-instance')
|
||||||
|
})
|
||||||
|
.toBe('This is a test')
|
||||||
|
})
|
||||||
|
|
||||||
test('document drawers are visually stacking', async () => {
|
test('document drawers are visually stacking', async () => {
|
||||||
await navigateToDoc(page, postsUrl)
|
await navigateToDoc(page, postsUrl)
|
||||||
await page.locator('#field-title').fill(title)
|
await page.locator('#field-title').fill(title)
|
||||||
|
|||||||
@@ -293,6 +293,13 @@ export interface User {
|
|||||||
hash?: string | null;
|
hash?: string | null;
|
||||||
loginAttempts?: number | null;
|
loginAttempts?: number | null;
|
||||||
lockUntil?: string | null;
|
lockUntil?: string | null;
|
||||||
|
sessions?:
|
||||||
|
| {
|
||||||
|
id: string;
|
||||||
|
createdAt?: string | null;
|
||||||
|
expiresAt: string;
|
||||||
|
}[]
|
||||||
|
| null;
|
||||||
password?: string | null;
|
password?: string | null;
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
@@ -820,6 +827,13 @@ export interface UsersSelect<T extends boolean = true> {
|
|||||||
hash?: T;
|
hash?: T;
|
||||||
loginAttempts?: T;
|
loginAttempts?: T;
|
||||||
lockUntil?: T;
|
lockUntil?: T;
|
||||||
|
sessions?:
|
||||||
|
| T
|
||||||
|
| {
|
||||||
|
id?: T;
|
||||||
|
createdAt?: T;
|
||||||
|
expiresAt?: T;
|
||||||
|
};
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* This interface was referenced by `Config`'s JSON-Schema
|
* This interface was referenced by `Config`'s JSON-Schema
|
||||||
|
|||||||
@@ -248,7 +248,7 @@ export async function saveDocHotkeyAndAssert(page: Page): Promise<void> {
|
|||||||
|
|
||||||
export async function saveDocAndAssert(
|
export async function saveDocAndAssert(
|
||||||
page: Page,
|
page: Page,
|
||||||
selector = '#action-save',
|
selector: '#access-save' | '#action-publish' | '#action-save-draft' | string = '#action-save',
|
||||||
expectation: 'error' | 'success' = 'success',
|
expectation: 'error' | 'success' = 'success',
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
await wait(500) // TODO: Fix this
|
await wait(500) // TODO: Fix this
|
||||||
|
|||||||
Reference in New Issue
Block a user