From faff65c56f592067f4a77e377fcb2aa1b86882cf Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Thu, 11 Jul 2024 15:20:59 -0400 Subject: [PATCH 01/13] fix: do not send duplicate upload edit data on request --- .../elements/DocumentDrawer/DrawerContent.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx b/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx index 2a295d6a72f..0f64e5582b8 100644 --- a/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx +++ b/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx @@ -12,7 +12,10 @@ import { XIcon } from '../../icons/X/index.js' import { useComponentMap } from '../../providers/ComponentMap/index.js' import { useConfig } from '../../providers/Config/index.js' import { DocumentInfoProvider, useDocumentInfo } from '../../providers/DocumentInfo/index.js' -import { useFormQueryParams } from '../../providers/FormQueryParams/index.js' +import { + FormQueryParamsProvider, + useFormQueryParams, +} from '../../providers/FormQueryParams/index.js' import { useLocale } from '../../providers/Locale/index.js' import { useTranslation } from '../../providers/Translation/index.js' import { Gutter } from '../Gutter/index.js' @@ -114,7 +117,16 @@ export const DocumentDrawerContent: React.FC = ({ onLoadError={onLoadError} onSave={onSave} > - {Edit} + + {Edit} + ) } From 50a27b5c42c649212de11d1d17eace1dde6b3acc Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Mon, 15 Jul 2024 09:58:36 -0400 Subject: [PATCH 02/13] fix: removes FormQueryParams provider for UploadEditsProvider --- docs/migration-guide/overview.mdx | 2 - packages/next/src/views/Account/index.tsx | 42 +++++------ packages/next/src/views/Document/index.tsx | 67 ++++++------------ .../next/src/views/Edit/Default/index.tsx | 19 ++--- .../elements/DocumentDrawer/DrawerContent.tsx | 22 +----- packages/ui/src/elements/EditUpload/index.tsx | 5 +- packages/ui/src/elements/Upload/index.tsx | 38 ++++------ packages/ui/src/exports/client/index.ts | 5 +- packages/ui/src/forms/Form/index.tsx | 22 ++---- .../ui/src/providers/DocumentInfo/index.tsx | 39 ++++++++++- .../ui/src/providers/DocumentInfo/types.ts | 8 +-- .../src/providers/FormQueryParams/index.tsx | 69 ------------------- .../ui/src/providers/FormQueryParams/types.ts | 18 ----- .../ui/src/providers/UploadEdits/index.tsx | 49 +++++++++++++ packages/ui/src/utilities/buildFormState.ts | 6 +- test/fields/collections/Upload/e2e.spec.ts | 40 +++++++++++ 16 files changed, 195 insertions(+), 256 deletions(-) delete mode 100644 packages/ui/src/providers/FormQueryParams/index.tsx delete mode 100644 packages/ui/src/providers/FormQueryParams/types.ts create mode 100644 packages/ui/src/providers/UploadEdits/index.tsx diff --git a/docs/migration-guide/overview.mdx b/docs/migration-guide/overview.mdx index d2ffa09013b..ff1ee1e2b91 100644 --- a/docs/migration-guide/overview.mdx +++ b/docs/migration-guide/overview.mdx @@ -180,7 +180,6 @@ import { useFormInitializing, useFormModified, useFormProcessing, - useFormQueryParams, useFormSubmitted, useHotkey, useIntersect, @@ -221,7 +220,6 @@ import { EntityVisibilityProvider, FieldComponentsProvider, FieldPropsProvider, - FormQueryParamsProvider, ListInfoProvider, ListQueryProvider, LocaleProvider, diff --git a/packages/next/src/views/Account/index.tsx b/packages/next/src/views/Account/index.tsx index 814b08f2532..842d3e6edcf 100644 --- a/packages/next/src/views/Account/index.tsx +++ b/packages/next/src/views/Account/index.tsx @@ -1,6 +1,6 @@ import type { AdminViewProps, ServerSideEditViewProps } from 'payload' -import { DocumentInfoProvider, FormQueryParamsProvider, HydrateClientUser } from '@payloadcms/ui' +import { DocumentInfoProvider, HydrateClientUser } from '@payloadcms/ui' import { RenderCustomComponent } from '@payloadcms/ui/shared' import { notFound } from 'next/navigation.js' import React from 'react' @@ -65,7 +65,6 @@ export const Account: React.FC = async ({ return ( } - action={`${serverURL}${api}/${userSlug}${user?.id ? `/${user.id}` : ''}`} apiURL={`${serverURL}${api}/${userSlug}${user?.id ? `/${user.id}` : ''}`} collectionSlug={userSlug} docPermissions={docPermissions} @@ -84,31 +83,22 @@ export const Account: React.FC = async ({ permissions={permissions} /> - - - + /> ) } diff --git a/packages/next/src/views/Document/index.tsx b/packages/next/src/views/Document/index.tsx index 01e43438624..ac7b2df28d4 100644 --- a/packages/next/src/views/Document/index.tsx +++ b/packages/next/src/views/Document/index.tsx @@ -1,16 +1,6 @@ -import type { - AdminViewComponent, - AdminViewProps, - EditViewComponent, - ServerSideEditViewProps, -} from 'payload' +import type { AdminViewComponent, AdminViewProps, EditViewComponent } from 'payload' -import { - DocumentInfoProvider, - EditDepthProvider, - FormQueryParamsProvider, - HydrateClientUser, -} from '@payloadcms/ui' +import { DocumentInfoProvider, EditDepthProvider, HydrateClientUser } from '@payloadcms/ui' import { RenderCustomComponent, isEditing as getIsEditing } from '@payloadcms/ui/shared' import { notFound, redirect } from 'next/navigation.js' import React from 'react' @@ -65,7 +55,6 @@ export const Document: React.FC = async ({ let ErrorView: AdminViewComponent let apiURL: string - let action: string const { data, formState } = await getDocumentData({ id, @@ -88,8 +77,6 @@ export const Document: React.FC = async ({ notFound() } - action = `${serverURL}${apiRoute}/${collectionSlug}${isEditing ? `/${id}` : ''}` - const params = new URLSearchParams() if (collectionConfig.versions?.drafts) { params.append('draft', 'true') @@ -128,8 +115,6 @@ export const Document: React.FC = async ({ notFound() } - action = `${serverURL}${apiRoute}/globals/${globalSlug}` - const params = new URLSearchParams({ locale: locale?.code, }) @@ -198,7 +183,6 @@ export const Document: React.FC = async ({ return ( = async ({ depth={1} key={`${collectionSlug || globalSlug}${locale?.code ? `-${locale?.code}` : ''}`} > - - {ErrorView ? ( - - ) : ( - - )} - + {ErrorView ? ( + + ) : ( + + )} ) diff --git a/packages/next/src/views/Edit/Default/index.tsx b/packages/next/src/views/Edit/Default/index.tsx index dad04a718b2..3bb944705b0 100644 --- a/packages/next/src/views/Edit/Default/index.tsx +++ b/packages/next/src/views/Edit/Default/index.tsx @@ -13,7 +13,7 @@ import { useDocumentEvents, useDocumentInfo, useEditDepth, - useFormQueryParams, + useUploadEdits, } from '@payloadcms/ui' import { getFormState } from '@payloadcms/ui/shared' import { useRouter, useSearchParams } from 'next/navigation.js' @@ -58,11 +58,13 @@ export const DefaultEditView: React.FC = () => { const { refreshCookieAsync, user } = useAuth() const config = useConfig() const router = useRouter() - const { dispatchFormQueryParams } = useFormQueryParams() const { getComponentMap, getFieldMap } = useComponentMap() - const params = useSearchParams() const depth = useEditDepth() + const params = useSearchParams() const { reportUpdate } = useDocumentEvents() + const { resetUploadEdits } = useUploadEdits() + + const locale = params.get('locale') const { admin: { user: userSlug }, @@ -72,8 +74,6 @@ export const DefaultEditView: React.FC = () => { serverURL, } = config - const locale = params.get('locale') - const collectionConfig = collectionSlug && collections.find((collection) => collection.slug === collectionSlug) @@ -130,12 +130,7 @@ export const DefaultEditView: React.FC = () => { const redirectRoute = `${adminRoute}/collections/${collectionSlug}/${json?.doc?.id}${locale ? `?locale=${locale}` : ''}` router.push(redirectRoute) } else { - dispatchFormQueryParams({ - type: 'SET', - params: { - uploadEdits: null, - }, - }) + resetUploadEdits() } }, [ @@ -151,9 +146,7 @@ export const DefaultEditView: React.FC = () => { isEditing, refreshCookieAsync, adminRoute, - locale, router, - dispatchFormQueryParams, ], ) diff --git a/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx b/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx index 0f64e5582b8..1133052e640 100644 --- a/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx +++ b/packages/ui/src/elements/DocumentDrawer/DrawerContent.tsx @@ -1,7 +1,6 @@ 'use client' import { useModal } from '@faceless-ui/modal' -import * as qs from 'qs-esm' import React, { useCallback, useEffect, useState } from 'react' import { toast } from 'sonner' @@ -12,10 +11,6 @@ import { XIcon } from '../../icons/X/index.js' import { useComponentMap } from '../../providers/ComponentMap/index.js' import { useConfig } from '../../providers/Config/index.js' import { DocumentInfoProvider, useDocumentInfo } from '../../providers/DocumentInfo/index.js' -import { - FormQueryParamsProvider, - useFormQueryParams, -} from '../../providers/FormQueryParams/index.js' import { useLocale } from '../../providers/Locale/index.js' import { useTranslation } from '../../providers/Translation/index.js' import { Gutter } from '../Gutter/index.js' @@ -43,8 +38,6 @@ export const DocumentDrawerContent: React.FC = ({ const [docID, setDocID] = useState(existingDocID) const [isOpen, setIsOpen] = useState(false) const [collectionConfig] = useRelatedCollections(collectionSlug) - const { formQueryParams } = useFormQueryParams() - const formattedQueryParams = qs.stringify(formQueryParams) const { componentMap } = useComponentMap() @@ -53,9 +46,6 @@ export const DocumentDrawerContent: React.FC = ({ const apiURL = docID ? `${serverURL}${apiRoute}/${collectionSlug}/${docID}${locale?.code ? `?locale=${locale.code}` : ''}` : null - const action = `${serverURL}${apiRoute}/${collectionSlug}${ - isEditing ? `/${docID}` : '' - }?${formattedQueryParams}` useEffect(() => { setIsOpen(Boolean(modalState[drawerSlug]?.isOpen)) @@ -107,7 +97,6 @@ export const DocumentDrawerContent: React.FC = ({ } - action={action} apiURL={apiURL} collectionSlug={collectionConfig.slug} disableActions @@ -117,16 +106,7 @@ export const DocumentDrawerContent: React.FC = ({ onLoadError={onLoadError} onSave={onSave} > - - {Edit} - + {Edit} ) } diff --git a/packages/ui/src/elements/EditUpload/index.tsx b/packages/ui/src/elements/EditUpload/index.tsx index 7d0c02b5d28..fbddcb908d7 100644 --- a/packages/ui/src/elements/EditUpload/index.tsx +++ b/packages/ui/src/elements/EditUpload/index.tsx @@ -48,7 +48,7 @@ export type EditUploadProps = { imageCacheTag?: string initialCrop?: CropType initialFocalPoint?: FocalPosition - onSave?: ({ crop, focalPosition }: { crop: CropType; focalPosition: FocalPosition }) => void + onSave?: ({ crop, focalPoint }: { crop: CropType; focalPoint: FocalPosition }) => void showCrop?: boolean showFocalPoint?: boolean } @@ -90,6 +90,7 @@ export const EditUpload: React.FC = ({ ...defaultFocalPosition, ...initialFocalPoint, })) + const [checkBounds, setCheckBounds] = useState(false) const [originalHeight, setOriginalHeight] = useState(0) const [originalWidth, setOriginalWidth] = useState(0) @@ -147,7 +148,7 @@ export const EditUpload: React.FC = ({ widthPixels: Number(widthRef.current?.value ?? crop.widthPixels), } : undefined, - focalPosition, + focalPoint: focalPosition, }) closeModal(editDrawerSlug) } diff --git a/packages/ui/src/elements/Upload/index.tsx b/packages/ui/src/elements/Upload/index.tsx index d126bdee001..410f2260573 100644 --- a/packages/ui/src/elements/Upload/index.tsx +++ b/packages/ui/src/elements/Upload/index.tsx @@ -1,16 +1,15 @@ 'use client' import type { FormState, SanitizedCollectionConfig } from 'payload' +import { useForm, useUploadEdits } from '@payloadcms/ui' import { isImage, reduceFieldsToValues } from 'payload/shared' import React, { useCallback, useEffect, useRef, useState } from 'react' import { toast } from 'sonner' import { FieldError } from '../../fields/FieldError/index.js' import { fieldBaseClass } from '../../fields/shared/index.js' -import { useForm } from '../../forms/Form/context.js' import { useField } from '../../forms/useField/index.js' import { useDocumentInfo } from '../../providers/DocumentInfo/index.js' -import { useFormQueryParams } from '../../providers/FormQueryParams/index.js' import { useTranslation } from '../../providers/Translation/index.js' import { Button } from '../Button/index.js' import { Drawer, DrawerToggler } from '../Drawer/index.js' @@ -92,14 +91,13 @@ export const Upload: React.FC = (props) => { const [fileSrc, setFileSrc] = useState(null) const { t } = useTranslation() const { setModified } = useForm() - const { dispatchFormQueryParams, formQueryParams } = useFormQueryParams() + const { resetUploadEdits, updateUploadEdits, uploadEdits } = useUploadEdits() const [doc, setDoc] = useState(reduceFieldsToValues(initialState || {}, true)) const { docPermissions } = useDocumentInfo() const { errorMessage, setValue, showError, value } = useField({ path: 'file', validate, }) - const [_crop, setCrop] = useState({ x: 0, y: 0 }) const [showUrlInput, setShowUrlInput] = useState(false) const [fileUrl, setFileUrl] = useState('') @@ -167,31 +165,19 @@ export const Upload: React.FC = (props) => { setFileSrc('') setFileUrl('') setDoc({}) + resetUploadEdits() setShowUrlInput(false) - }, [handleFileChange]) + }, [handleFileChange, resetUploadEdits]) const onEditsSave = useCallback( - ({ crop, focalPosition }) => { - setCrop({ - x: crop.x || 0, - y: crop.y || 0, - }) - + ({ crop, focalPoint }) => { setModified(true) - dispatchFormQueryParams({ - type: 'SET', - params: { - uploadEdits: - crop || focalPosition - ? { - crop: crop || null, - focalPoint: focalPosition ? focalPosition : null, - } - : null, - }, + updateUploadEdits({ + crop, + focalPoint, }) }, - [dispatchFormQueryParams, setModified], + [setModified], ) const handlePasteUrlClick = () => { @@ -342,10 +328,10 @@ export const Upload: React.FC = (props) => { fileName={value?.name || doc?.filename} fileSrc={doc?.url || fileSrc} imageCacheTag={doc.updatedAt} - initialCrop={formQueryParams?.uploadEdits?.crop ?? {}} + initialCrop={uploadEdits?.crop ?? {}} initialFocalPoint={{ - x: formQueryParams?.uploadEdits?.focalPoint.x || doc.focalX || 50, - y: formQueryParams?.uploadEdits?.focalPoint.y || doc.focalY || 50, + x: uploadEdits?.focalPoint?.x || doc.focalX || 50, + y: uploadEdits?.focalPoint?.y || doc.focalY || 50, }} onSave={onEditsSave} showCrop={showCrop} diff --git a/packages/ui/src/exports/client/index.ts b/packages/ui/src/exports/client/index.ts index 567d181359b..d2f49c0f390 100644 --- a/packages/ui/src/exports/client/index.ts +++ b/packages/ui/src/exports/client/index.ts @@ -202,10 +202,7 @@ export { FieldComponentsProvider, useFieldComponents, } from '../../providers/FieldComponents/index.js' -export { - FormQueryParamsProvider, - useFormQueryParams, -} from '../../providers/FormQueryParams/index.js' +export { UploadEditsProvider, useUploadEdits } from '../../providers/UploadEdits/index.js' export { type ColumnPreferences, ListInfoProvider, diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 4cba98def13..2c088166489 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -1,7 +1,6 @@ 'use client' import type { FormState } from 'payload' -/* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ import { dequal } from 'dequal/lite' // lite: no need for Map and Set support import { useRouter } from 'next/navigation.js' import { serialize } from 'object-to-formdata' @@ -11,7 +10,6 @@ import { reduceFieldsToValues, wait, } from 'payload/shared' -import * as qs from 'qs-esm' import React, { useCallback, useEffect, useReducer, useRef, useState } from 'react' import { toast } from 'sonner' @@ -27,7 +25,6 @@ import { useThrottledEffect } from '../../hooks/useThrottledEffect.js' import { useAuth } from '../../providers/Auth/index.js' import { useConfig } from '../../providers/Config/index.js' import { useDocumentInfo } from '../../providers/DocumentInfo/index.js' -import { useFormQueryParams } from '../../providers/FormQueryParams/index.js' import { useLocale } from '../../providers/Locale/index.js' import { useOperation } from '../../providers/Operation/index.js' import { useTranslation } from '../../providers/Translation/index.js' @@ -81,7 +78,6 @@ export const Form: React.FC = (props) => { const { i18n, t } = useTranslation() const { refreshCookie, user } = useAuth() const operation = useOperation() - const { formQueryParams } = useFormQueryParams() const config = useConfig() const { @@ -169,7 +165,7 @@ export const Form: React.FC = (props) => { const submit = useCallback( async (options: SubmitOptions = {}, e): Promise => { const { - action: actionArg, + action: actionArg = action, method: methodToUse = method, overrides = {}, skipValidation, @@ -278,14 +274,10 @@ export const Form: React.FC = (props) => { try { let res - const actionEndpoint = - actionArg || - (typeof action === 'string' - ? `${action}${qs.stringify(formQueryParams, { addQueryPrefix: true })}` - : null) + const actionEndpoint = typeof actionArg === 'string' ? actionArg : null if (actionEndpoint) { - res = await requests[methodToUse.toLowerCase()](actionEndpoint, { + res = await requests[methodToUse.toLowerCase()](actionArg, { body: formData, headers: { 'Accept-Language': i18n.language, @@ -401,7 +393,6 @@ export const Form: React.FC = (props) => { t, i18n, waitForAutocomplete, - formQueryParams, ], ) @@ -629,14 +620,9 @@ export const Form: React.FC = (props) => { [contextRef.current.fields, dispatchFields, onChange, modified], ) - const actionString = - typeof action === 'string' - ? `${action}${qs.stringify(formQueryParams, { addQueryPrefix: true })}` - : '' - return (
useContext(Context) -export const DocumentInfoProvider: React.FC< - DocumentInfoProps & { +const DocumentInfo: React.FC< + { children: React.ReactNode - } + } & DocumentInfoProps > = ({ children, ...props }) => { const { id, @@ -66,6 +67,8 @@ export const DocumentInfoProvider: React.FC< const { i18n } = useTranslation() + const { uploadEdits } = useUploadEdits() + const [documentTitle, setDocumentTitle] = useState(() => { if (!initialDataFromProps) return '' @@ -104,15 +107,18 @@ export const DocumentInfoProvider: React.FC< const baseURL = `${serverURL}${api}` let slug: string + let pluralType: 'collections' | 'globals' let preferencesKey: string if (globalSlug) { slug = globalSlug + pluralType = 'globals' preferencesKey = `global-${slug}` } if (collectionSlug) { slug = collectionSlug + pluralType = 'collections' if (id) { preferencesKey = `collection-${slug}-${id}` @@ -510,10 +516,25 @@ export const DocumentInfoProvider: React.FC< data, ]) + const action: string = React.useMemo(() => { + const docURL = `${baseURL}${pluralType === 'globals' ? `/globals` : ''}/${slug}${id ? `/${id}` : ''}` + const params = { + depth: 0, + 'fallback-locale': 'null', + locale, + uploadEdits: uploadEdits || undefined, + } + + return `${docURL}${qs.stringify(params, { + addQueryPrefix: true, + })}` + }, [baseURL, locale, pluralType, id, slug, uploadEdits]) + if (isError) notFound() const value: DocumentInfoContext = { ...props, + action, docConfig, docPermissions, getDocPermissions, @@ -536,3 +557,15 @@ export const DocumentInfoProvider: React.FC< return {children} } + +export const DocumentInfoProvider: React.FC< + { + children: React.ReactNode + } & DocumentInfoProps +> = (props) => { + return ( + + + + ) +} diff --git a/packages/ui/src/providers/DocumentInfo/types.ts b/packages/ui/src/providers/DocumentInfo/types.ts index 731cbfa7405..c5b1e794954 100644 --- a/packages/ui/src/providers/DocumentInfo/types.ts +++ b/packages/ui/src/providers/DocumentInfo/types.ts @@ -37,7 +37,7 @@ export type DocumentInfoProps = { onSave?: (data: Data) => Promise | void } -export type DocumentInfoContext = DocumentInfoProps & { +export type DocumentInfoContext = { docConfig?: ClientCollectionConfig | ClientGlobalConfig getDocPermissions: (data?: Data) => Promise getDocPreferences: () => Promise @@ -47,10 +47,10 @@ export type DocumentInfoContext = DocumentInfoProps & { isInitializing: boolean isLoading: boolean preferencesKey?: string - publishedDoc?: TypeWithID & TypeWithTimestamps & { _status?: string } + publishedDoc?: { _status?: string } & TypeWithID & TypeWithTimestamps setDocFieldPreferences: ( field: string, - fieldPreferences: Partial & { [key: string]: unknown }, + fieldPreferences: { [key: string]: unknown } & Partial, ) => void setDocumentTitle: (title: string) => void slug?: string @@ -58,4 +58,4 @@ export type DocumentInfoContext = DocumentInfoProps & { unpublishedVersions?: PaginatedDocs> versions?: PaginatedDocs> versionsCount?: PaginatedDocs> -} +} & DocumentInfoProps diff --git a/packages/ui/src/providers/FormQueryParams/index.tsx b/packages/ui/src/providers/FormQueryParams/index.tsx deleted file mode 100644 index 37e6cec619b..00000000000 --- a/packages/ui/src/providers/FormQueryParams/index.tsx +++ /dev/null @@ -1,69 +0,0 @@ -'use client' - -import React, { createContext, useContext } from 'react' - -import type { Action, FormQueryParamsContext, State } from './types.js' - -import { useLocale } from '../Locale/index.js' - -export type * from './types.js' - -export const FormQueryParams = createContext({} as FormQueryParamsContext) - -export const FormQueryParamsProvider: React.FC<{ - children: React.ReactNode - initialParams?: State -}> = ({ children, initialParams: formQueryParamsFromProps }) => { - const [formQueryParams, dispatchFormQueryParams] = React.useReducer( - (state: State, action: Action) => { - const newState = { ...state } - - switch (action.type) { - case 'SET': - if (action.params?.uploadEdits === null && newState?.uploadEdits) { - delete newState.uploadEdits - } - if (action.params?.uploadEdits?.crop === null && newState?.uploadEdits?.crop) { - delete newState.uploadEdits.crop - } - if ( - action.params?.uploadEdits?.focalPoint === null && - newState?.uploadEdits?.focalPoint - ) { - delete newState.uploadEdits.focalPoint - } - return { - ...newState, - ...action.params, - } - default: - return state - } - }, - formQueryParamsFromProps || ({} as State), - ) - - const locale = useLocale() - - React.useEffect(() => { - if (locale?.code) { - dispatchFormQueryParams({ - type: 'SET', - params: { - locale: locale.code, - }, - }) - } - }, [locale.code]) - - return ( - - {children} - - ) -} - -export const useFormQueryParams = (): { - dispatchFormQueryParams: React.Dispatch - formQueryParams: State -} => useContext(FormQueryParams) diff --git a/packages/ui/src/providers/FormQueryParams/types.ts b/packages/ui/src/providers/FormQueryParams/types.ts deleted file mode 100644 index 91bd7cb2cea..00000000000 --- a/packages/ui/src/providers/FormQueryParams/types.ts +++ /dev/null @@ -1,18 +0,0 @@ -import type { UploadEdits } from 'payload' - -export type FormQueryParamsContext = { - dispatchFormQueryParams: (action: Action) => void - formQueryParams: State -} - -export type State = { - depth: number - 'fallback-locale': string - locale: string - uploadEdits?: UploadEdits -} - -export type Action = { - params: Partial - type: 'SET' -} diff --git a/packages/ui/src/providers/UploadEdits/index.tsx b/packages/ui/src/providers/UploadEdits/index.tsx new file mode 100644 index 00000000000..eff2fe25a58 --- /dev/null +++ b/packages/ui/src/providers/UploadEdits/index.tsx @@ -0,0 +1,49 @@ +import React from 'react' + +type UploadEdits = { + crop?: { + height?: number + width?: number + x?: number + y?: number + } + focalPoint?: { + x?: number + y?: number + } +} + +export type UploadEditsContext = { + resetUploadEdits: () => void + updateUploadEdits: (edits: UploadEdits) => void + uploadEdits: UploadEdits +} + +const Context = React.createContext({ + resetUploadEdits: undefined, + updateUploadEdits: undefined, + uploadEdits: undefined, +}) + +export const UploadEditsProvider = ({ children }) => { + const [uploadEdits, setUploadEdits] = React.useState(undefined) + + const resetUploadEdits = () => { + setUploadEdits({}) + } + + const updateUploadEdits = (edits: UploadEdits) => { + setUploadEdits((prevEdits) => ({ + ...(prevEdits || {}), + ...(edits || {}), + })) + } + + return ( + + {children} + + ) +} + +export const useUploadEdits = (): UploadEditsContext => React.useContext(Context) diff --git a/packages/ui/src/utilities/buildFormState.ts b/packages/ui/src/utilities/buildFormState.ts index a869f43480b..f56b09e4bd3 100644 --- a/packages/ui/src/utilities/buildFormState.ts +++ b/packages/ui/src/utilities/buildFormState.ts @@ -1,6 +1,5 @@ import type { DocumentPreferences, Field, FormState, PayloadRequest, TypeWithID } from 'payload' -// eslint-disable-next-line payload/no-imports-from-exports-dir import { reduceFieldsToValues } from 'payload/shared' import type { BuildFormStateArgs } from '../forms/buildStateFromSchema/index.js' @@ -14,7 +13,6 @@ import { buildFieldSchemaMap } from './buildFieldSchemaMap/index.js' let cached = global._payload_fieldSchemaMap if (!cached) { - // eslint-disable-next-line no-multi-assign cached = global._payload_fieldSchemaMap = null } @@ -32,7 +30,7 @@ export const getFieldSchemaMap = (req: PayloadRequest): FieldSchemaMap => { } export const buildFormState = async ({ req }: { req: PayloadRequest }): Promise => { - const reqData: BuildFormStateArgs = req.data as BuildFormStateArgs + const reqData: BuildFormStateArgs = (req.data || {}) as BuildFormStateArgs const { collectionSlug, formState, globalSlug, locale, operation, schemaPath } = reqData const incomingUserSlug = req.user?.collection @@ -69,7 +67,7 @@ export const buildFormState = async ({ req }: { req: PayloadRequest }): Promise< const fieldSchemaMap = getFieldSchemaMap(req) const id = collectionSlug ? reqData.id : undefined - const schemaPathSegments = schemaPath.split('.') + const schemaPathSegments = schemaPath && schemaPath.split('.') let fieldSchema: Field[] diff --git a/test/fields/collections/Upload/e2e.spec.ts b/test/fields/collections/Upload/e2e.spec.ts index 78441311020..ab206bd9920 100644 --- a/test/fields/collections/Upload/e2e.spec.ts +++ b/test/fields/collections/Upload/e2e.spec.ts @@ -152,6 +152,46 @@ describe('Upload', () => { await saveDocAndAssert(page) }) + test('should upload after editing image inside a document drawer', async () => { + await uploadImage() + await wait(1000) + // Open the media drawer and create a png upload + + await openDocDrawer(page, '.field-type.upload .upload__toggler.doc-drawer__toggler') + + await page + .locator('[id^=doc-drawer_uploads_1_] .file-field__upload input[type="file"]') + .setInputFiles(path.resolve(dirname, './uploads/payload.png')) + await expect( + page.locator('[id^=doc-drawer_uploads_1_] .file-field__upload .file-field__filename'), + ).toHaveValue('payload.png') + await page.locator('[id^=doc-drawer_uploads_1_] .file-field__edit').click() + await page + .locator('[id^=edit-upload] .edit-upload__input input[name="Width (px)"]') + .nth(1) + .fill('200') + await page + .locator('[id^=edit-upload] .edit-upload__input input[name="Height (px)"]') + .nth(1) + .fill('200') + await page.locator('[id^=edit-upload] button:has-text("Apply Changes")').nth(1).click() + await page.locator('[id^=doc-drawer_uploads_1_] #action-save').click() + await expect(page.locator('.payload-toast-container')).toContainText('successfully') + + // Assert that the media field has the png upload + await expect( + page.locator('.field-type.upload .file-details .file-meta__url a'), + ).toHaveAttribute('href', '/api/uploads/file/payload-1.png') + await expect(page.locator('.field-type.upload .file-details .file-meta__url a')).toContainText( + 'payload-1.png', + ) + await expect(page.locator('.field-type.upload .file-details img')).toHaveAttribute( + 'src', + '/api/uploads/file/payload-1.png', + ) + await saveDocAndAssert(page) + }) + test('should clear selected upload', async () => { await uploadImage() await wait(1000) // TODO: Fix this. Need to wait a bit until the form in the drawer mounted, otherwise values sometimes disappear. This is an issue for all drawers From 71e6ef1947e5fe9fa16c760f315748639dd3585d Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Mon, 15 Jul 2024 13:37:07 -0400 Subject: [PATCH 03/13] fix: simplifies if statement to remove need for actionEndpoint var --- packages/ui/src/forms/Form/index.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ui/src/forms/Form/index.tsx b/packages/ui/src/forms/Form/index.tsx index 2c088166489..cf17120b6b4 100644 --- a/packages/ui/src/forms/Form/index.tsx +++ b/packages/ui/src/forms/Form/index.tsx @@ -274,9 +274,8 @@ export const Form: React.FC = (props) => { try { let res - const actionEndpoint = typeof actionArg === 'string' ? actionArg : null - if (actionEndpoint) { + if (typeof actionArg === 'string') { res = await requests[methodToUse.toLowerCase()](actionArg, { body: formData, headers: { From 31956dc103d1e328378e346829f4d1df6ba5ef09 Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Mon, 15 Jul 2024 15:50:05 -0400 Subject: [PATCH 04/13] fix: adds locale to dep array in default edit view --- packages/next/src/views/Edit/Default/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/next/src/views/Edit/Default/index.tsx b/packages/next/src/views/Edit/Default/index.tsx index 3bb944705b0..24781ebd535 100644 --- a/packages/next/src/views/Edit/Default/index.tsx +++ b/packages/next/src/views/Edit/Default/index.tsx @@ -147,6 +147,7 @@ export const DefaultEditView: React.FC = () => { refreshCookieAsync, adminRoute, router, + locale, ], ) From c33f84b895c417f9418f86c5f93e0ba488374ba6 Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Mon, 15 Jul 2024 16:48:52 -0400 Subject: [PATCH 05/13] fix: adds resetUploadEdits to dep array --- packages/next/src/views/Edit/Default/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/next/src/views/Edit/Default/index.tsx b/packages/next/src/views/Edit/Default/index.tsx index 24781ebd535..554b3cec41c 100644 --- a/packages/next/src/views/Edit/Default/index.tsx +++ b/packages/next/src/views/Edit/Default/index.tsx @@ -148,6 +148,7 @@ export const DefaultEditView: React.FC = () => { adminRoute, router, locale, + resetUploadEdits, ], ) From f8dd65a554dfda02006c84d6b391c6299725f5f4 Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Tue, 16 Jul 2024 10:21:09 -0400 Subject: [PATCH 06/13] chore: adds updateUploadEdits to dep array in onEditsSave --- packages/ui/src/elements/Upload/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/elements/Upload/index.tsx b/packages/ui/src/elements/Upload/index.tsx index 410f2260573..c06d423b568 100644 --- a/packages/ui/src/elements/Upload/index.tsx +++ b/packages/ui/src/elements/Upload/index.tsx @@ -177,7 +177,7 @@ export const Upload: React.FC = (props) => { focalPoint, }) }, - [setModified], + [setModified, updateUploadEdits], ) const handlePasteUrlClick = () => { From 8c4bbe17b8723cc1384ae01674f66fe7117f55bf Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Tue, 16 Jul 2024 17:11:29 -0400 Subject: [PATCH 07/13] chore: fixes focal point cropping --- packages/payload/src/uploads/cropImage.ts | 27 +++- .../payload/src/uploads/generateFileData.ts | 9 +- packages/payload/src/uploads/imageResizer.ts | 135 ++++++++++++------ packages/payload/src/uploads/types.ts | 27 ++-- packages/ui/src/elements/EditUpload/index.tsx | 57 ++++---- packages/ui/src/elements/Upload/index.tsx | 11 +- .../ui/src/providers/UploadEdits/index.tsx | 15 +- 7 files changed, 171 insertions(+), 110 deletions(-) diff --git a/packages/payload/src/uploads/cropImage.ts b/packages/payload/src/uploads/cropImage.ts index c744fa56dff..bbf114a4f4f 100644 --- a/packages/payload/src/uploads/cropImage.ts +++ b/packages/payload/src/uploads/cropImage.ts @@ -1,12 +1,31 @@ import type { SharpOptions } from 'sharp' +import type { SanitizedConfig } from '../config/types.js' +import type { PayloadRequest } from '../types/index.js' +import type { UploadEdits } from './types.js' + export const percentToPixel = (value, dimension) => { return Math.floor((parseFloat(value) / 100) * dimension) } -export async function cropImage({ cropData, dimensions, file, sharp }) { +type CropImageArgs = { + cropData: UploadEdits['crop'] + dimensions: { height: number; width: number } + file: PayloadRequest['file'] + heightInPixels: number + sharp: SanitizedConfig['sharp'] + widthInPixels: number +} +export async function cropImage({ + cropData, + dimensions, + file, + heightInPixels, + sharp, + widthInPixels, +}: CropImageArgs) { try { - const { heightPixels, widthPixels, x, y } = cropData + const { x, y } = cropData const fileIsAnimatedType = ['image/avif', 'image/gif', 'image/webp'].includes(file.mimetype) @@ -15,10 +34,10 @@ export async function cropImage({ cropData, dimensions, file, sharp }) { if (fileIsAnimatedType) sharpOptions.animated = true const formattedCropData = { - height: Number(heightPixels), + height: Number(heightInPixels), left: percentToPixel(x, dimensions.width), top: percentToPixel(y, dimensions.height), - width: Number(widthPixels), + width: Number(widthInPixels), } const cropped = sharp(file.tempFilePath || file.data, sharpOptions).extract(formattedCropData) diff --git a/packages/payload/src/uploads/generateFileData.ts b/packages/payload/src/uploads/generateFileData.ts index 5ff9b81a370..77b4f6d07a6 100644 --- a/packages/payload/src/uploads/generateFileData.ts +++ b/packages/payload/src/uploads/generateFileData.ts @@ -203,7 +203,14 @@ export const generateFileData = async ({ let fileForResize = file if (cropData && sharp) { - const { data: croppedImage, info } = await cropImage({ cropData, dimensions, file, sharp }) + const { data: croppedImage, info } = await cropImage({ + cropData, + dimensions, + file, + heightInPixels: uploadEdits.heightInPixels, + sharp, + widthInPixels: uploadEdits.widthInPixels, + }) filesToSave.push({ buffer: croppedImage, diff --git a/packages/payload/src/uploads/imageResizer.ts b/packages/payload/src/uploads/imageResizer.ts index 0bbc89bbad6..699a1cfeae0 100644 --- a/packages/payload/src/uploads/imageResizer.ts +++ b/packages/payload/src/uploads/imageResizer.ts @@ -1,4 +1,4 @@ -import type { OutputInfo, Sharp, SharpOptions } from 'sharp' +import type { OutputInfo, Sharp, Metadata as SharpMetadata, SharpOptions } from 'sharp' import { fileTypeFromBuffer } from 'file-type' import fs from 'fs' @@ -68,11 +68,20 @@ const getSanitizedImageData = (sourceImage: string): SanitizedImageData => { * @param extension - the extension to use * @returns the new image name that is not taken */ -const createImageName = ( - outputImageName: string, - { height, width }: OutputInfo, - extension: string, -) => `${outputImageName}-${width}x${height}.${extension}` +type CreateImageNameArgs = { + extension: string + height: number + outputImageName: string + width: number +} +const createImageName = ({ + extension, + height, + outputImageName, + width, +}: CreateImageNameArgs): string => { + return `${outputImageName}-${width}x${height}.${extension}` +} type CreateResultArgs = { filename?: FileSize['filename'] @@ -209,6 +218,19 @@ const sanitizeResizeConfig = (resizeConfig: ImageSize): ImageSize => { return resizeConfig } +/** + * Used to extract height from images, animated or not. + * + * @param sharpMetadata - the sharp metadata + * @returns the height of the image + */ +function extractHeightFromImage(sharpMetadata: SharpMetadata): number { + if (sharpMetadata?.pages) { + return sharpMetadata.height / sharpMetadata.pages + } + return sharpMetadata.height +} + /** * For the provided image sizes, handle the resizing and the transforms * (format, trim, etc.) of each requested image size and return the result object. @@ -261,6 +283,12 @@ export async function resizeAndTransformImageSizes({ if (fileIsAnimatedType) sharpOptions.animated = true const sharpBase: Sharp | undefined = sharp(file.tempFilePath || file.data, sharpOptions).rotate() // pass rotate() to auto-rotate based on EXIF data. https://github.com/payloadcms/payload/pull/3081 + const originalImageMeta = await sharpBase.metadata() + + const resizeImageMeta = { + height: extractHeightFromImage(originalImageMeta), + width: originalImageMeta.width, + } const results: ImageSizesResult[] = await Promise.all( imageSizes.map(async (imageResizeConfig): Promise => { @@ -276,8 +304,6 @@ export async function resizeAndTransformImageSizes({ const imageToResize = sharpBase.clone() let resized = imageToResize - const metadata = await sharpBase.metadata() - if (incomingFocalPoint && applyPayloadAdjustments(imageResizeConfig, dimensions)) { let { height: resizeHeight, width: resizeWidth } = imageResizeConfig @@ -293,44 +319,60 @@ export async function resizeAndTransformImageSizes({ resizeHeight = Math.round(resizeWidth / originalAspectRatio) } - // Scale the image up or down to fit the resize dimensions - const scaledImage = imageToResize.resize({ - height: resizeHeight, - width: resizeWidth, - }) + if (!resizeHeight) resizeHeight = resizeImageMeta.height + if (!resizeWidth) resizeWidth = resizeImageMeta.width + + // if requested image is larger than the incoming size, then resize using sharp and then extract with focal point + if (resizeHeight > resizeImageMeta.height || resizeWidth > resizeImageMeta.width) { + resized = imageToResize.resize({ + height: resizeHeight, + width: resizeWidth, + }) + + // must read from buffer, resize.metadata will return the original image metadata + const { info } = await resized.toBuffer({ resolveWithObject: true }) + resizeImageMeta.height = extractHeightFromImage({ + ...originalImageMeta, + height: info.height, + }) + resizeImageMeta.width = info.width + } - const { info: scaledImageInfo } = await scaledImage.toBuffer({ resolveWithObject: true }) + const halfResizeX = resizeWidth / 2 + const xFocalCenter = resizeImageMeta.width * (incomingFocalPoint.x / 100) + const calculatedRightPixelBound = xFocalCenter + halfResizeX + let leftBound = xFocalCenter - halfResizeX - const safeResizeWidth = resizeWidth ?? scaledImageInfo.width - const maxOffsetX = scaledImageInfo.width - safeResizeWidth - const leftFocalEdge = Math.round( - scaledImageInfo.width * (incomingFocalPoint.x / 100) - safeResizeWidth / 2, - ) - const safeOffsetX = Math.min(Math.max(0, leftFocalEdge), maxOffsetX) + // if the right bound is greater than the image width, adjust the left bound + // keeping focus on the right + if (calculatedRightPixelBound > resizeImageMeta.width) { + leftBound = resizeImageMeta.width - resizeWidth + } - const isAnimated = fileIsAnimatedType && metadata.pages + // if the left bound is less than 0, adjust the left bound to 0 + // keeping the focus on the left + if (leftBound < 0) leftBound = 0 - let safeResizeHeight = resizeHeight ?? scaledImageInfo.height + const halfResizeY = resizeHeight / 2 + const yFocalCenter = resizeImageMeta.height * (incomingFocalPoint.y / 100) + const calculatedBottomPixelBound = yFocalCenter + halfResizeY + let topBound = yFocalCenter - halfResizeY - if (isAnimated && resizeHeight === undefined) { - safeResizeHeight = scaledImageInfo.height / metadata.pages + // if the bottom bound is greater than the image height, adjust the top bound + // keeping the image as far right as possible + if (calculatedBottomPixelBound > resizeImageMeta.height) { + topBound = resizeImageMeta.height - resizeHeight } - const maxOffsetY = isAnimated - ? safeResizeHeight - (resizeHeight ?? safeResizeHeight) - : scaledImageInfo.height - safeResizeHeight + // if the top bound is less than 0, adjust the top bound to 0 + // keeping the image focus near the top + if (topBound < 0) topBound = 0 - const topFocalEdge = Math.round( - scaledImageInfo.height * (incomingFocalPoint.y / 100) - safeResizeHeight / 2, - ) - const safeOffsetY = Math.min(Math.max(0, topFocalEdge), maxOffsetY) - - // extract the focal area from the scaled image - resized = (fileIsAnimatedType ? imageToResize : scaledImage).extract({ - height: safeResizeHeight, - left: safeOffsetX, - top: safeOffsetY, - width: safeResizeWidth, + resized = resized.extract({ + height: resizeHeight, + left: Math.floor(leftBound), + top: Math.floor(topBound), + width: resizeWidth, }) } else { resized = imageToResize.resize(imageResizeConfig) @@ -359,11 +401,15 @@ export async function resizeAndTransformImageSizes({ const mimeInfo = await fileTypeFromBuffer(bufferData) - const imageNameWithDimensions = createImageName( - sanitizedImage.name, - bufferInfo, - mimeInfo?.ext || sanitizedImage.ext, - ) + const imageNameWithDimensions = createImageName({ + extension: mimeInfo?.ext || sanitizedImage.ext, + height: extractHeightFromImage({ + ...originalImageMeta, + height: bufferInfo.height, + }), + outputImageName: sanitizedImage.name, + width: bufferInfo.width, + }) const imagePath = `${staticPath}/${imageNameWithDimensions}` @@ -380,7 +426,8 @@ export async function resizeAndTransformImageSizes({ name: imageResizeConfig.name, filename: imageNameWithDimensions, filesize: size, - height: fileIsAnimatedType && metadata.pages ? height / metadata.pages : height, + height: + fileIsAnimatedType && originalImageMeta.pages ? height / originalImageMeta.pages : height, mimeType: mimeInfo?.mime || mimeType, sizesToSave: [{ buffer: bufferData, path: imagePath }], width, diff --git a/packages/payload/src/uploads/types.ts b/packages/payload/src/uploads/types.ts index e86be233bd9..61530054f15 100644 --- a/packages/payload/src/uploads/types.ts +++ b/packages/payload/src/uploads/types.ts @@ -189,15 +189,22 @@ export type FileToSave = { path: string } +type Crop = { + height: number + unit: '%' | 'px' + width: number + x: number + y: number +} + +type FocalPoint = { + x: number + y: number +} + export type UploadEdits = { - crop?: { - height?: number - width?: number - x?: number - y?: number - } - focalPoint?: { - x?: number - y?: number - } + crop?: Crop + focalPoint?: FocalPoint + heightInPixels?: number + widthInPixels?: number } diff --git a/packages/ui/src/elements/EditUpload/index.tsx b/packages/ui/src/elements/EditUpload/index.tsx index fbddcb908d7..5dff698c416 100644 --- a/packages/ui/src/elements/EditUpload/index.tsx +++ b/packages/ui/src/elements/EditUpload/index.tsx @@ -1,5 +1,6 @@ 'use client' -import type CropType from 'react-image-crop' + +import type { UploadEdits } from 'payload' import { useModal } from '@faceless-ui/modal' import React, { forwardRef, useRef, useState } from 'react' @@ -46,19 +47,17 @@ export type EditUploadProps = { fileName: string fileSrc: string imageCacheTag?: string - initialCrop?: CropType + initialCrop?: UploadEdits['crop'] initialFocalPoint?: FocalPosition - onSave?: ({ crop, focalPoint }: { crop: CropType; focalPoint: FocalPosition }) => void + onSave?: (uploadEdits: UploadEdits) => void showCrop?: boolean showFocalPoint?: boolean } -const defaultCrop: CropType = { +const defaultCrop: UploadEdits['crop'] = { height: 100, - heightPixels: 0, unit: '%', width: 100, - widthPixels: 0, x: 0, y: 0, } @@ -76,9 +75,9 @@ export const EditUpload: React.FC = ({ const { closeModal } = useModal() const { t } = useTranslation() - const [crop, setCrop] = useState(() => ({ + const [crop, setCrop] = useState(() => ({ ...defaultCrop, - ...initialCrop, + ...(initialCrop || {}), })) const defaultFocalPosition: FocalPosition = { @@ -92,30 +91,32 @@ export const EditUpload: React.FC = ({ })) const [checkBounds, setCheckBounds] = useState(false) - const [originalHeight, setOriginalHeight] = useState(0) - const [originalWidth, setOriginalWidth] = useState(0) + const [uncroppedPixelHeight, setUncroppedPixelHeight] = useState(0) + const [uncroppedPixelWidth, setUncroppedPixelWidth] = useState(0) const focalWrapRef = useRef(undefined) const imageRef = useRef(undefined) const cropRef = useRef(undefined) - const heightRef = useRef(null) - const widthRef = useRef(null) + const heightInputRef = useRef(null) + const widthInputRef = useRef(null) const [imageLoaded, setImageLoaded] = useState(false) const onImageLoad = (e) => { - setOriginalHeight(e.currentTarget.naturalHeight) - setOriginalWidth(e.currentTarget.naturalWidth) + // set the default image height/width on load + setUncroppedPixelHeight(e.currentTarget.naturalHeight) + setUncroppedPixelWidth(e.currentTarget.naturalWidth) setImageLoaded(true) } const fineTuneCrop = ({ dimension, value }: { dimension: 'height' | 'width'; value: string }) => { const intValue = parseInt(value) - if (dimension === 'width' && intValue >= originalWidth) return null - if (dimension === 'height' && intValue >= originalHeight) return null + if (dimension === 'width' && intValue >= uncroppedPixelWidth) return null + if (dimension === 'height' && intValue >= uncroppedPixelHeight) return null - const percentage = 100 * (intValue / (dimension === 'width' ? originalWidth : originalHeight)) + const percentage = + 100 * (intValue / (dimension === 'width' ? uncroppedPixelWidth : uncroppedPixelHeight)) if (percentage === 100 || percentage === 0) return null @@ -141,14 +142,10 @@ export const EditUpload: React.FC = ({ const saveEdits = () => { if (typeof onSave === 'function') onSave({ - crop: crop - ? { - ...crop, - heightPixels: Number(heightRef.current?.value ?? crop.heightPixels), - widthPixels: Number(widthRef.current?.value ?? crop.widthPixels), - } - : undefined, + crop: crop ? crop : undefined, focalPoint: focalPosition, + heightInPixels: Number(heightInputRef?.current?.value ?? uncroppedPixelHeight), + widthInPixels: Number(widthInputRef?.current?.value ?? uncroppedPixelWidth), }) closeModal(editDrawerSlug) } @@ -204,7 +201,7 @@ export const EditUpload: React.FC = ({ className={`${baseClass}__focal-wrapper`} ref={focalWrapRef} style={{ - aspectRatio: `${originalWidth / originalHeight}`, + aspectRatio: `${uncroppedPixelWidth / uncroppedPixelHeight}`, }} > {showCrop ? ( @@ -260,10 +257,8 @@ export const EditUpload: React.FC = ({ onClick={() => setCrop({ height: 100, - heightPixels: originalHeight, unit: '%', width: 100, - widthPixels: originalWidth, x: 0, y: 0, }) @@ -280,14 +275,14 @@ export const EditUpload: React.FC = ({ fineTuneCrop({ dimension: 'width', value })} - ref={widthRef} - value={((crop.width / 100) * originalWidth).toFixed(0)} + ref={widthInputRef} + value={((crop.width / 100) * uncroppedPixelWidth).toFixed(0)} /> fineTuneCrop({ dimension: 'height', value })} - ref={heightRef} - value={((crop.height / 100) * originalHeight).toFixed(0)} + ref={heightInputRef} + value={((crop.height / 100) * uncroppedPixelHeight).toFixed(0)} /> diff --git a/packages/ui/src/elements/Upload/index.tsx b/packages/ui/src/elements/Upload/index.tsx index c06d423b568..6178b145186 100644 --- a/packages/ui/src/elements/Upload/index.tsx +++ b/packages/ui/src/elements/Upload/index.tsx @@ -1,5 +1,5 @@ 'use client' -import type { FormState, SanitizedCollectionConfig } from 'payload' +import type { FormState, SanitizedCollectionConfig , UploadEdits } from 'payload' import { useForm, useUploadEdits } from '@payloadcms/ui' import { isImage, reduceFieldsToValues } from 'payload/shared' @@ -170,12 +170,9 @@ export const Upload: React.FC = (props) => { }, [handleFileChange, resetUploadEdits]) const onEditsSave = useCallback( - ({ crop, focalPoint }) => { + (args: UploadEdits) => { setModified(true) - updateUploadEdits({ - crop, - focalPoint, - }) + updateUploadEdits(args) }, [setModified, updateUploadEdits], ) @@ -328,7 +325,7 @@ export const Upload: React.FC = (props) => { fileName={value?.name || doc?.filename} fileSrc={doc?.url || fileSrc} imageCacheTag={doc.updatedAt} - initialCrop={uploadEdits?.crop ?? {}} + initialCrop={uploadEdits?.crop ?? undefined} initialFocalPoint={{ x: uploadEdits?.focalPoint?.x || doc.focalX || 50, y: uploadEdits?.focalPoint?.y || doc.focalY || 50, diff --git a/packages/ui/src/providers/UploadEdits/index.tsx b/packages/ui/src/providers/UploadEdits/index.tsx index eff2fe25a58..46083495069 100644 --- a/packages/ui/src/providers/UploadEdits/index.tsx +++ b/packages/ui/src/providers/UploadEdits/index.tsx @@ -1,17 +1,6 @@ -import React from 'react' +import type { UploadEdits } from 'payload' -type UploadEdits = { - crop?: { - height?: number - width?: number - x?: number - y?: number - } - focalPoint?: { - x?: number - y?: number - } -} +import React from 'react' export type UploadEditsContext = { resetUploadEdits: () => void From 8dff5d866ad63b6d9e6948b7ac6254af607b49aa Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Wed, 17 Jul 2024 09:10:09 -0400 Subject: [PATCH 08/13] chore: adds e2e test for incorrect resized animated image file names --- packages/payload/src/uploads/imageResizer.ts | 2 +- test/uploads/e2e.spec.ts | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/payload/src/uploads/imageResizer.ts b/packages/payload/src/uploads/imageResizer.ts index 699a1cfeae0..11fd3278436 100644 --- a/packages/payload/src/uploads/imageResizer.ts +++ b/packages/payload/src/uploads/imageResizer.ts @@ -1,4 +1,4 @@ -import type { OutputInfo, Sharp, Metadata as SharpMetadata, SharpOptions } from 'sharp' +import type { Sharp, Metadata as SharpMetadata, SharpOptions } from 'sharp' import { fileTypeFromBuffer } from 'file-type' import fs from 'fs' diff --git a/test/uploads/e2e.spec.ts b/test/uploads/e2e.spec.ts index c0c69529a06..ce3d581f319 100644 --- a/test/uploads/e2e.spec.ts +++ b/test/uploads/e2e.spec.ts @@ -143,6 +143,25 @@ describe('uploads', () => { await saveDocAndAssert(page) }) + test('should show proper file names for resized animated file', async () => { + await page.goto(animatedTypeMediaURL.create) + + await page.setInputFiles('input[type="file"]', path.resolve(dirname, './animated.webp')) + const animatedFilename = page.locator('.file-field__filename') + + await expect(animatedFilename).toHaveValue('animated.webp') + + await saveDocAndAssert(page) + + await page.locator('.file-field__previewSizes').click() + + const smallSquareFilename = page + .locator('.preview-sizes__list .preview-sizes__sizeOption') + .nth(1) + .locator('.file-meta__url a') + await expect(smallSquareFilename).toContainText(/480x480\.webp$/) + }) + test('should show resized images', async () => { await page.goto(mediaURL.edit(pngDoc.id)) From 7a60357baf2f57fd501289b6820f53c7691942c3 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Wed, 17 Jul 2024 15:36:20 -0400 Subject: [PATCH 09/13] chore: consolidate resize logic --- packages/payload/src/uploads/imageResizer.ts | 127 +++++++++---------- 1 file changed, 60 insertions(+), 67 deletions(-) diff --git a/packages/payload/src/uploads/imageResizer.ts b/packages/payload/src/uploads/imageResizer.ts index 11fd3278436..93c657b5f8f 100644 --- a/packages/payload/src/uploads/imageResizer.ts +++ b/packages/payload/src/uploads/imageResizer.ts @@ -131,71 +131,61 @@ const createResult = ({ } /** - * Check if the image needs to be resized according to the requested dimensions - * and the original image size. If the resize options withoutEnlargement or withoutReduction are provided, - * the image will be resized regardless of the requested dimensions, given that the - * width or height to be resized is provided. + * Determine whether or not to resize the image. + * - resize using image config + * - resize using image config with focal adjustments + * - do not resize at all * - * @param resizeConfig - object containing the requested dimensions and resize options - * @param original - the original image size - * @returns true if resizing is not needed, false otherwise + * `imageResizeConfig.withoutEnlargement`: + * - undefined [default]: uploading images with smaller width AND height than the image size will return null + * - false: always enlarge images to the image size + * - true: if the image is smaller than the image size, return the original image + * + * `imageResizeConfig.withoutReduction`: + * - false [default]: always enlarge images to the image size + * - true: if the image is smaller than the image size, return the original image + * + * @return 'omit' | 'resize' | 'resizeWithFocalPoint' */ -const preventResize = ( - { height: desiredHeight, width: desiredWidth, withoutEnlargement, withoutReduction }: ImageSize, - original: ProbedImageSize, -): boolean => { - // default is to allow reduction - if (withoutReduction !== undefined) { - return false // needs resize - } - - // default is to prevent enlargement - if (withoutEnlargement !== undefined) { - return false // needs resize +const getImageResizeAction = ({ + dimensions: originalImage, + hasFocalPoint, + imageResizeConfig, +}: { + dimensions: ProbedImageSize + hasFocalPoint?: boolean + imageResizeConfig: ImageSize +}): 'omit' | 'resize' | 'resizeWithFocalPoint' => { + const { + fit, + height: targetHeight, + width: targetWidth, + withoutEnlargement, + withoutReduction, + } = imageResizeConfig + + // prevent upscaling by default when x and y are both smaller than target image size + if (targetHeight && targetWidth) { + const originalImageIsSmallerXAndY = + originalImage.width < targetWidth && originalImage.height < targetHeight + if (withoutEnlargement === undefined && originalImageIsSmallerXAndY) { + return 'omit' // prevent image size from being enlarged + } } - const isWidthOrHeightNotDefined = !desiredHeight || !desiredWidth - if (isWidthOrHeightNotDefined) { - // If width and height are not defined, it means there is a format conversion - // and the image needs to be "resized" (transformed). - return false // needs resize - } + const originalImageIsSmallerXOrY = + originalImage.width < targetWidth || originalImage.height < targetHeight + if (fit === 'contain' || fit === 'inside') return 'resize' + if (!isNumber(targetHeight) && !isNumber(targetWidth)) return 'resize' - const hasInsufficientWidth = desiredWidth > original.width - const hasInsufficientHeight = desiredHeight > original.height - if (hasInsufficientWidth && hasInsufficientHeight) { - // doesn't need resize - prevent enlargement. This should only happen if both width and height are insufficient. - // if only one dimension is insufficient and the other is sufficient, resizing needs to happen, as the image - // should be resized to the sufficient dimension. - return true // do not create a new size - } + const targetAspectRatio = targetWidth / targetHeight + const originalAspectRatio = originalImage.width / originalImage.height + if (originalAspectRatio === targetAspectRatio) return 'resize' - return false // needs resize -} + if (withoutEnlargement && originalImageIsSmallerXOrY) return 'resize' + if (withoutReduction && !originalImageIsSmallerXOrY) return 'resize' -/** - * Check if the image should be passed directly to sharp without payload adjusting properties. - * - * @param resizeConfig - object containing the requested dimensions and resize options - * @param original - the original image size - * @returns true if the image should passed directly to sharp - */ -const applyPayloadAdjustments = ( - { fit, height, width, withoutEnlargement, withoutReduction }: ImageSize, - original: ProbedImageSize, -) => { - if (fit === 'contain' || fit === 'inside') return false - if (!isNumber(height) && !isNumber(width)) return false - - const targetAspectRatio = width / height - const originalAspectRatio = original.width / original.height - if (originalAspectRatio === targetAspectRatio) return false - - const skipEnlargement = withoutEnlargement && (original.height < height || original.width < width) - const skipReduction = withoutReduction && (original.height > height || original.width > width) - if (skipEnlargement || skipReduction) return false - - return true + return hasFocalPoint ? 'resizeWithFocalPoint' : 'resize' } /** @@ -294,17 +284,18 @@ export async function resizeAndTransformImageSizes({ imageSizes.map(async (imageResizeConfig): Promise => { imageResizeConfig = sanitizeResizeConfig(imageResizeConfig) - // This checks if a resize should happen. If not, the resized image will be - // skipped COMPLETELY and thus will not be included in the resulting images. - // All further format/trim options will thus be skipped as well. - if (preventResize(imageResizeConfig, dimensions)) { - return createResult({ name: imageResizeConfig.name }) - } + const resizeAction = getImageResizeAction({ + dimensions, + hasFocalPoint: Boolean(incomingFocalPoint), + imageResizeConfig, + }) + console.log({ resizeAction }) + if (resizeAction === 'omit') return createResult({ name: imageResizeConfig.name }) const imageToResize = sharpBase.clone() let resized = imageToResize - if (incomingFocalPoint && applyPayloadAdjustments(imageResizeConfig, dimensions)) { + if (resizeAction === 'resizeWithFocalPoint') { let { height: resizeHeight, width: resizeWidth } = imageResizeConfig const originalAspectRatio = dimensions.width / dimensions.height @@ -324,9 +315,11 @@ export async function resizeAndTransformImageSizes({ // if requested image is larger than the incoming size, then resize using sharp and then extract with focal point if (resizeHeight > resizeImageMeta.height || resizeWidth > resizeImageMeta.width) { + const resizeAspectRatio = resizeWidth / resizeHeight + const prioritizeHeight = resizeAspectRatio < originalAspectRatio resized = imageToResize.resize({ - height: resizeHeight, - width: resizeWidth, + height: prioritizeHeight ? resizeHeight : undefined, + width: prioritizeHeight ? undefined : resizeWidth, }) // must read from buffer, resize.metadata will return the original image metadata From 6e38be839f88e13344e297f657fdc6e57ca59ae4 Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Wed, 17 Jul 2024 17:28:31 -0400 Subject: [PATCH 10/13] chore: stubs out e2e test for focal point issue updates --- test/uploads/another-test-image.jpg | Bin 0 -> 2601 bytes test/uploads/e2e.spec.ts | 46 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 test/uploads/another-test-image.jpg diff --git a/test/uploads/another-test-image.jpg b/test/uploads/another-test-image.jpg new file mode 100644 index 0000000000000000000000000000000000000000..e543b942a02701057a6a66fe3fa0e8fb59814c43 GIT binary patch literal 2601 zcmex=PKf)jmb{qpT;N;>4N{9$BA`61pKv4;ZGmvGtxH&jM>IE^?-(uilW(3;BEXZKb z(EVQX`Uhl<5cXx~_8a~U7~+<*S3Jzc5M|&p*vBr7sanffr)&nMYF-nI+Lj*};> { let page: Page @@ -59,6 +61,7 @@ describe('uploads', () => { relationURL = new AdminUrlUtil(serverURL, relationSlug) adminThumbnailSizeURL = new AdminUrlUtil(serverURL, adminThumbnailSizeSlug) adminThumbnailFunctionURL = new AdminUrlUtil(serverURL, adminThumbnailFunctionSlug) + focalOnlyURL = new AdminUrlUtil(serverURL, focalOnlySlug) const context = await browser.newContext() page = await context.newPage() @@ -418,5 +421,48 @@ describe('uploads', () => { expect(greenDoc.filesize).toEqual(1205) expect(redDoc.filesize).toEqual(1207) }) + + test('should update image alignment based on focal point correctly', async () => { + const updateFocalPosition = async (page: Page) => { + await page.goto(focalOnlyURL.create) + await page.waitForURL(focalOnlyURL.create) + // select and upload file + const fileChooserPromise = page.waitForEvent('filechooser') + await page.getByText('Select a file').click() + const fileChooser = await fileChooserPromise + await wait(1000) + await fileChooser.setFiles(path.join(dirname, 'another-test-image.jpg')) + + await page.locator('.file-field__edit').click() + + // set focal point + await page.locator('.edit-upload__input input[name="X %"]').fill('12') // init left focal point + await page.locator('.edit-upload__input input[name="Y %"]').fill('50') // init top focal point + + // apply crop + await page.locator('button:has-text("Apply Changes")').click() + await page.waitForSelector('button#action-save') + await page.locator('button#action-save').click() + await expect(page.locator('.payload-toast-container')).toContainText('successfully') + await wait(1000) // Wait for the save + } + + await updateFocalPosition(page) // red square + const redSquareMediaID = page.url().split('/').pop() // get the ID of the doc + + const { doc: redDoc } = await client.findByID({ + id: redSquareMediaID, + slug: mediaSlug, + auth: true, + }) + + const redFocalTest = redDoc.sizes.find((size) => size.name === 'focalTest') + const redFocalTest2 = redDoc.sizes.find((size) => size.name === 'focalTest2') + const redFocalTest3 = redDoc.sizes.find((size) => size.name === 'focalTest3') + + expect(redFocalTest.focalPoint).toEqual({ x: 100, y: 75 }) // Example expected value + expect(redFocalTest2.focalPoint).toEqual({ x: 150, y: 75 }) // Example expected value + expect(redFocalTest3.focalPoint).toEqual({ x: 225, y: 75 }) // Example expected value + }) }) }) From 1ebc1c6b80871c61265e65fb678e9c66eae72a6f Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Thu, 18 Jul 2024 11:53:16 -0400 Subject: [PATCH 11/13] fix: jarrods negligence --- packages/payload/src/uploads/imageResizer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/payload/src/uploads/imageResizer.ts b/packages/payload/src/uploads/imageResizer.ts index 93c657b5f8f..db81e1ee6a1 100644 --- a/packages/payload/src/uploads/imageResizer.ts +++ b/packages/payload/src/uploads/imageResizer.ts @@ -289,7 +289,6 @@ export async function resizeAndTransformImageSizes({ hasFocalPoint: Boolean(incomingFocalPoint), imageResizeConfig, }) - console.log({ resizeAction }) if (resizeAction === 'omit') return createResult({ name: imageResizeConfig.name }) const imageToResize = sharpBase.clone() From a9c69ce2feae32401c21d173b7081ff43e4360e7 Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Thu, 18 Jul 2024 11:53:43 -0400 Subject: [PATCH 12/13] fix: updates focal point e2e test --- test/uploads/e2e.spec.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/test/uploads/e2e.spec.ts b/test/uploads/e2e.spec.ts index 3ee42d53490..aabb3b238b4 100644 --- a/test/uploads/e2e.spec.ts +++ b/test/uploads/e2e.spec.ts @@ -422,7 +422,7 @@ describe('uploads', () => { expect(redDoc.filesize).toEqual(1207) }) - test('should update image alignment based on focal point correctly', async () => { + test('should update image alignment based on focal point', async () => { const updateFocalPosition = async (page: Page) => { await page.goto(focalOnlyURL.create) await page.waitForURL(focalOnlyURL.create) @@ -436,10 +436,10 @@ describe('uploads', () => { await page.locator('.file-field__edit').click() // set focal point - await page.locator('.edit-upload__input input[name="X %"]').fill('12') // init left focal point - await page.locator('.edit-upload__input input[name="Y %"]').fill('50') // init top focal point + await page.locator('.edit-upload__input input[name="X %"]').fill('12') // left focal point + await page.locator('.edit-upload__input input[name="Y %"]').fill('50') // top focal point - // apply crop + // apply focal point await page.locator('button:has-text("Apply Changes")').click() await page.waitForSelector('button#action-save') await page.locator('button#action-save').click() @@ -452,17 +452,12 @@ describe('uploads', () => { const { doc: redDoc } = await client.findByID({ id: redSquareMediaID, - slug: mediaSlug, + slug: focalOnlySlug, auth: true, }) - const redFocalTest = redDoc.sizes.find((size) => size.name === 'focalTest') - const redFocalTest2 = redDoc.sizes.find((size) => size.name === 'focalTest2') - const redFocalTest3 = redDoc.sizes.find((size) => size.name === 'focalTest3') - - expect(redFocalTest.focalPoint).toEqual({ x: 100, y: 75 }) // Example expected value - expect(redFocalTest2.focalPoint).toEqual({ x: 150, y: 75 }) // Example expected value - expect(redFocalTest3.focalPoint).toEqual({ x: 225, y: 75 }) // Example expected value + // without focal point update this generated size was equal to 1736 + expect(redDoc.sizes.focalTest.filesize).toEqual(1598) }) }) }) From 2daa52a4f798f6d4960f33732450ec9294c1cb84 Mon Sep 17 00:00:00 2001 From: PatrikKozak Date: Thu, 18 Jul 2024 13:08:00 -0400 Subject: [PATCH 13/13] chore: update image file name --- test/uploads/e2e.spec.ts | 2 +- ...nother-test-image.jpg => horizontal-squares.jpg} | Bin 2 files changed, 1 insertion(+), 1 deletion(-) rename test/uploads/{another-test-image.jpg => horizontal-squares.jpg} (100%) diff --git a/test/uploads/e2e.spec.ts b/test/uploads/e2e.spec.ts index aabb3b238b4..d83d1e95dc3 100644 --- a/test/uploads/e2e.spec.ts +++ b/test/uploads/e2e.spec.ts @@ -431,7 +431,7 @@ describe('uploads', () => { await page.getByText('Select a file').click() const fileChooser = await fileChooserPromise await wait(1000) - await fileChooser.setFiles(path.join(dirname, 'another-test-image.jpg')) + await fileChooser.setFiles(path.join(dirname, 'horizontal-squares.jpg')) await page.locator('.file-field__edit').click() diff --git a/test/uploads/another-test-image.jpg b/test/uploads/horizontal-squares.jpg similarity index 100% rename from test/uploads/another-test-image.jpg rename to test/uploads/horizontal-squares.jpg