feat(ui): redesign uploads document view#17044
Conversation
paulpopus
requested review from
AlessioGr,
DanRibbens,
JarrodMFlesch,
denolfe and
jacobsfletch
as code owners
paulpopus
changed the title
📦 esbuild Bundle Analysis for payloadThis analysis was generated by esbuild-bundle-analyzer. 🤖
Largest pathsThese visualization shows top 20 largest paths in the bundle.Meta file: packages/next/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_shared.json, Out file: esbuild/exports/shared.js
Meta file: packages/richtext-lexical/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_shared.json, Out file: esbuild/exports/shared_optimized/index.js
DetailsNext to the size is how much the size has increased or decreased compared with the base branch of this PR.
|
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
EditUpload element still has a library.scss file. Needs to be either converted or removed if those styles are not being used anymore
| import { Button } from '../Button/index.js' | ||
| import { DialogBody, DialogFooter, DialogHeader, DialogModal } from '../Dialog/index.js' | ||
| import './index.css' | ||
| import './library.scss' |
There was a problem hiding this comment.
EditUpload is using a library.scss file - this should be converted to css or removed if the styles aren't being used anymore
| .edit-upload__sidebar { | ||
| flex-shrink: 0; | ||
| width: 240px; | ||
| border-left: 1px solid var(--color-border); |
There was a problem hiding this comment.
Need to use var(--stroke-width-small) instead of 1px value here.
I'm noticing this throughout a lot of your edited css files so I'm going to just leave this one comment but make sure to search through all your edited files and replace there as well.
Really any usage of 1px should be replaced with var(--stroke-width-small) even if not being used for a border style necessarily.
| .audio-preview { | ||
| width: 100%; | ||
| max-width: 480px; | ||
| } |
There was a problem hiding this comment.
For specific pixel values like 480px here and common pattern we have been following is defining it at the top of the file as a var, I'd recommend following this pattern here as well
| font-weight: var(--text-body-medium-strong-font-weight); | ||
| line-height: var(--text-body-medium-strong-line-height); | ||
| letter-spacing: var(--text-body-medium-letter-spacing); | ||
| max-width: 200px; |
There was a problem hiding this comment.
Same thing as mentioned in the above comment - any hard coded pixel values if necessary should be defined at the top of the file as variables
| align-items: center; | ||
| justify-content: center; | ||
| width: 24px; | ||
| height: 24px; |
There was a problem hiding this comment.
Are these widths & heights necessary? The icons themselves should already be of that dimension.
Our icons are either 16x16 or 24x24 by default and sometimes have options for both
| readOnly={isReadOnlyForIncomingUser || !hasSavePermission || isTrashed} | ||
| schemaPathSegments={schemaPathSegments} | ||
| /> | ||
| {upload && !BeforeFields ? ( |
There was a problem hiding this comment.
CustomUpload does not fall back to the legacy single-column layout it seems.
The PR description says custom Upload components "continue to render in the legacy single-column path." But the branch is upload && !BeforeFields, and CustomUpload is rendered inside the new __upload-layout (side-by-side) div. Only setting BeforeFields forces the legacy path.
So a collection with a custom components.edit.Upload (but no BeforeFields) now renders side-by-side, which contradicts the description and may break existing custom upload UIs.
| <MiniCarouselItem | ||
| active={selectedSize === null} | ||
| imageCacheTag={imageCacheTag} | ||
| label="Original" |
There was a problem hiding this comment.
Hard coded label "Original" - need to use translation key
| <DocumentFields | ||
| AfterFields={AfterFields} | ||
| BeforeFields={ | ||
| auth ? ( |
There was a problem hiding this comment.
The <Auth> JSX is copy-pasted into both branches of the Edit ternary - should we extract it to a local const authComponent to avoid drift?
| * native PDF viewer via an iframe. | ||
| */ | ||
| export const PdfPreview: React.FC<{ fileSrc: string; title?: string }> = ({ fileSrc, title }) => { | ||
| return <iframe className={baseClass} src={fileSrc} title={title ?? 'PDF preview'} /> |
There was a problem hiding this comment.
Has no sandbox prop defined - served as application/pdf, same-origin admin
Might be worth a deliberate decision
| name: 'photographer', | ||
| type: 'text', | ||
| admin: { | ||
| // position: 'sidebar', |
There was a problem hiding this comment.
Leftover commented out code here
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
AppHeader, DocumentControls, and the new DocumentHeaderRoot each set documentElement CSS variables via ResizeObserver.
DocumentControls also renders inside document drawers, so multiple instances could clobber --doc-controls-height, and none of the three clear their variable on unmount.
Worth confirming drawer/edit-in-place scenarios still lay out correctly.
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
These icons vary too much in size difference. I see that they're all 24 x 24 px but its possible you're pulling in the large variant from fpl - I'd double check
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
Border radius on .file-preview__info-label needs to be --radius-medium instead of --radius-small
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
Drawer padding in upload edit drawer is inconsistent with some of the upload content
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
Uploads with no fields looks like this right now, looks odd. What should uploads with no fields look like? Question for design.
PatrikKozak
left a comment
PatrikKozak
left a comment
There was a problem hiding this comment.
Tooltip is sitting underneath the doc controls on hover of the icons in below image
Maybe we can adjust the tooltip to be positioned below the icon on hover
2 participants
What
Redesigns the admin UI for upload-enabled collections, moving from the stacked single-column layout to a side-by-side document view: file management on the right, document fields on the left.
Highlights
__upload-layout) alongside the fields, instead of stacking the upload above the fields.FileManagerelement replaces the inlineUploadin the edit view, encapsulating the dropzone, preview, and toolbar.CropandDownloadicons.New config API
Adds
upload.admin.components.filePreview, letting collections override the side-panel preview:Resolution priority for the map is exact match → category wildcard (
video/*) → universal fallback (*), falling back to the default thumbnail when nothing matches. A newmatchMimeTypehelper (exported frompayload/shared) implements this, aUploadFilePreviewdocument slot andUploadFilePreviewClientPropstype are added, and the import-map generator now picks upfilePreviewcomponents.i18n
Adds translation keys:
general:original,upload:fromURL,upload:linkToFile,upload:renameFile,upload:replaceFile.Notes
Uploadcomponents (BeforeFields/CustomUpload) continue to render in the legacy single-column path, so existing overrides are unaffected.