-
Notifications
You must be signed in to change notification settings - Fork 1
Issue-836 Document Types #1072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Issue-836 Document Types #1072
Changes from all commits
555b00e
368f567
69dc0ba
601118a
ad0e10e
56198f4
9e4beff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,24 +5,46 @@ import { getMediaURL } from '@/utilities/getURL' | |
| import { isValidRelationship } from '@/utilities/relationships' | ||
| import { getHostnameFromTenant } from '@/utilities/tenancy/getHostnameFromTenant' | ||
| import { cn } from '@/utilities/ui' | ||
| import { FileDown } from 'lucide-react' | ||
|
|
||
| type Props = DocumentBlockProps & { | ||
| isLayoutBlock: boolean | ||
| // layout is present in the block config but absent from generated types until pnpm generate:types is run | ||
| layout?: 'download' | 'embed' | null | ||
| } | ||
|
|
||
| export const DocumentBlockComponent = (props: Props) => { | ||
| const { document, isLayoutBlock = true } = props | ||
| const { document, layout, isLayoutBlock = true } = props | ||
| const { tenant } = useTenant() | ||
|
|
||
| if (!isValidRelationship(document) || !document.url) { | ||
| return null | ||
| } | ||
|
|
||
| const src = getMediaURL(document.url, null, getHostnameFromTenant(tenant)) | ||
| const filename = document.filename ?? 'Download' | ||
|
|
||
| // Treat missing layout as 'embed' so existing blocks keep their current behavior | ||
| const resolvedLayout = layout ?? 'embed' | ||
|
|
||
| if (resolvedLayout === 'download') { | ||
| return ( | ||
| <div className={cn('my-4', { container: isLayoutBlock })}> | ||
| <a | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| href={src} | ||
| download={filename} | ||
| className="inline-flex items-center gap-2 rounded-md border px-4 py-2 text-sm font-medium hover:bg-accent transition-colors" | ||
| > | ||
| <FileDown className="h-4 w-4 shrink-0" /> | ||
| <span>{filename}</span> | ||
| </a> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <div className={cn('my-4', { container: isLayoutBlock })}> | ||
| <iframe src={src} width="100%" height="600px" title="Document PDF" /> | ||
| <iframe src={src} width="100%" height="600px" title="Document" /> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is no browser viewer for a given file type and the layout is set to
I can't find the behavior documented anywhere but it seems that if we set an iframe's src attribute to a file with a MIME type that it does not have a built-in viewer for, the file is automatically downloaded when the iframe is rendered. We definitely don't want this behavior so we need to account for this. It seems that PDF, HTML, Plain text, and XML/KML can all be rendered in an iframe. So we need conditional logic in the collection that only allows setting the |
||
| </div> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,5 +11,14 @@ export const DocumentBlock: Block = { | |
| relationTo: 'documents', | ||
| required: true, | ||
| }, | ||
| { | ||
| name: 'layout', | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we name this |
||
| type: 'select', | ||
| defaultValue: 'download', | ||
| options: [ | ||
| { label: 'Download Link', value: 'download' }, | ||
| { label: 'Embed (iframe)', value: 'embed' }, | ||
| ], | ||
| }, | ||
| ], | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { APIError, CollectionConfig } from 'payload' | ||
|
|
||
| type BeforeOperationHook = Exclude< | ||
| Exclude<CollectionConfig['hooks'], undefined>['beforeOperation'], | ||
| undefined | ||
| >[number] | ||
|
|
||
| export const validateNotImageOrVideo: BeforeOperationHook = ({ operation, req }) => { | ||
| if ((operation !== 'create' && operation !== 'update') || !req.file) { | ||
| return | ||
| } | ||
|
|
||
| const { mimetype } = req.file | ||
|
|
||
| if (mimetype.startsWith('image/') || mimetype.startsWith('video/')) { | ||
| throw new APIError( | ||
| 'Images and videos must be uploaded to the Media collection, not Documents.', | ||
| 400, | ||
| null, | ||
| true, | ||
| ) | ||
| } | ||
| } |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a good assumption to make but I think we should fallback to
downloaddue to the browser behavior that might result in a file being automatically downloaded if the browser can't render it in an iframe. See my other comment.We only have one page with DocumentBlocks in production, so this would be easy enough to go and update manually after we deploy this.