-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
🎉 Cloudflare images image updating #4252
Merged
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,26 +3,17 @@ import React, { | |
useContext, | ||
useEffect, | ||
useMemo, | ||
useRef, | ||
useState, | ||
} from "react" | ||
import { | ||
Button, | ||
Flex, | ||
Input, | ||
Mentions, | ||
Popconfirm, | ||
Space, | ||
Table, | ||
Upload, | ||
} from "antd" | ||
import { Button, Flex, Input, Mentions, Popconfirm, Table, Upload } from "antd" | ||
import { AdminLayout } from "./AdminLayout.js" | ||
import { AdminAppContext } from "./AdminAppContext.js" | ||
import { DbEnrichedImageWithUserId, DbPlainUser } from "@ourworldindata/types" | ||
import { Timeago } from "./Forms.js" | ||
import { ColumnsType } from "antd/es/table/InternalTable.js" | ||
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" | ||
import { faClose, faUpload } from "@fortawesome/free-solid-svg-icons" | ||
import { Admin } from "./Admin.js" | ||
import { RcFile } from "antd/es/upload/interface.js" | ||
import TextArea from "antd/es/input/TextArea.js" | ||
import { CLOUDFLARE_IMAGES_URL } from "../settings/clientSettings.js" | ||
|
@@ -37,6 +28,19 @@ type ImageEditorApi = { | |
image: DbEnrichedImageWithUserId, | ||
patch: Partial<DbEnrichedImageWithUserId> | ||
) => void | ||
putImage: ( | ||
id: number, | ||
payload: { | ||
filename: string | ||
content?: string | ||
type: string | ||
} | ||
) => void | ||
postImage: (payload: { | ||
filename: string | ||
content?: string | ||
type: string | ||
}) => void | ||
deleteImage: (image: DbEnrichedImageWithUserId) => void | ||
getImages: () => void | ||
getUsers: () => void | ||
|
@@ -114,7 +118,11 @@ function UserSelect({ | |
} | ||
|
||
const handleSelect = async (option: { value?: string; label?: string }) => { | ||
const selectedUser = usersMap[option.value!] | ||
// iterating because we only have the label when using the admin context | ||
const selectedUser = Object.values(usersMap).find( | ||
(user) => user.fullName === option.label | ||
) | ||
|
||
if (selectedUser) { | ||
setValue(selectedUser.fullName) | ||
await onUserSelect(selectedUser) | ||
|
@@ -141,7 +149,7 @@ function UserSelect({ | |
<div> | ||
<Button | ||
type="text" | ||
onClick={() => handleSelect({ value: admin.username })} | ||
onClick={() => handleSelect({ label: admin.username })} | ||
> | ||
+ {admin.username} | ||
</Button> | ||
|
@@ -152,6 +160,33 @@ function UserSelect({ | |
) | ||
} | ||
|
||
// when updatedAt changes, the image will reload the src | ||
// but it looks like sometimes cloudflare doesn't update in time :( | ||
function ImgWithRefresh({ | ||
src, | ||
updatedAt, | ||
}: { | ||
src: string | ||
updatedAt: number | null | ||
}) { | ||
const ref = useRef<HTMLImageElement>(null) | ||
useEffect(() => { | ||
if (ref.current && updatedAt) { | ||
ref.current.src = "" | ||
fetch(src, { cache: "reload" }) | ||
.then(() => { | ||
if (ref.current) { | ||
ref.current.src = src | ||
} | ||
}) | ||
.catch(() => { | ||
console.log("Something went wrong refreshing the image") | ||
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 I understand this correctly, we are effectively discarding the error here instead of letting it bubble up, which would be more helpful for debugging. |
||
}) | ||
} | ||
}) | ||
return <img ref={ref} src={src} style={{ maxHeight: 100, maxWidth: 100 }} /> | ||
} | ||
|
||
function createColumns({ | ||
api, | ||
users, | ||
|
@@ -165,7 +200,7 @@ function createColumns({ | |
dataIndex: "cloudflareId", | ||
width: 100, | ||
key: "cloudflareId", | ||
render: (cloudflareId, { originalWidth }) => { | ||
render: (cloudflareId, { originalWidth, updatedAt }) => { | ||
const srcFor = (w: number) => | ||
`${CLOUDFLARE_IMAGES_URL}/${encodeURIComponent( | ||
cloudflareId | ||
|
@@ -177,9 +212,9 @@ function createColumns({ | |
href={`${srcFor(originalWidth!)}`} | ||
rel="noopener" | ||
> | ||
<img | ||
<ImgWithRefresh | ||
src={`${srcFor(200)}`} | ||
style={{ maxHeight: 100, maxWidth: 100 }} | ||
updatedAt={updatedAt} | ||
/> | ||
</a> | ||
</div> | ||
|
@@ -285,7 +320,8 @@ function createColumns({ | |
key: "action", | ||
width: 100, | ||
render: (_, image) => ( | ||
<Space size="middle"> | ||
<Flex vertical> | ||
<PutImageButton putImage={api.putImage} id={image.id} /> | ||
<Popconfirm | ||
title="Are you sure?" | ||
description="This will delete the image being used in production." | ||
|
@@ -297,48 +333,77 @@ function createColumns({ | |
Delete | ||
</Button> | ||
</Popconfirm> | ||
</Space> | ||
</Flex> | ||
), | ||
}, | ||
] | ||
} | ||
|
||
function ImageUploadButton({ | ||
setImages, | ||
admin, | ||
}: { | ||
setImages: React.Dispatch<React.SetStateAction<ImageMap>> | ||
admin: Admin | ||
}) { | ||
function uploadImage({ file }: { file: string | Blob | RcFile }) { | ||
if (typeof file === "string") return | ||
type File = string | Blob | RcFile | ||
|
||
const reader = new FileReader() | ||
reader.onload = async () => { | ||
const base64Data = reader.result?.toString() | ||
type FileToBase64Result = { | ||
filename: string | ||
content: string | ||
type: string | ||
} | ||
|
||
const payload = { | ||
/** | ||
* Uploading as base64, because otherwise we'd need multipart/form-data parsing middleware in the server. | ||
* This seems easier as a one-off. | ||
**/ | ||
function fileToBase64(file: File): Promise<FileToBase64Result | null> { | ||
if (typeof file === "string") return Promise.resolve(null) | ||
|
||
return new Promise((resolve) => { | ||
const reader = new FileReader() | ||
reader.onload = () => { | ||
resolve({ | ||
filename: file.name, | ||
content: base64Data, | ||
content: reader.result?.toString() ?? "", | ||
type: file.type, | ||
} | ||
}) | ||
} | ||
reader.readAsDataURL(file) | ||
}) | ||
} | ||
|
||
const { image } = await admin.requestJSON<{ | ||
sucess: true | ||
image: DbEnrichedImageWithUserId | ||
}>("/api/image", payload, "POST") | ||
function PostImageButton({ | ||
postImage, | ||
}: { | ||
postImage: ImageEditorApi["postImage"] | ||
}) { | ||
async function uploadImage({ file }: { file: File }) { | ||
const result = await fileToBase64(file) | ||
if (result) { | ||
postImage(result) | ||
} | ||
} | ||
return ( | ||
<Upload showUploadList={false} customRequest={uploadImage}> | ||
<Button type="primary"> | ||
<FontAwesomeIcon icon={faUpload} /> Upload | ||
</Button> | ||
</Upload> | ||
) | ||
} | ||
|
||
setImages((imagesMap) => ({ | ||
...imagesMap, | ||
[image.id]: image, | ||
})) | ||
function PutImageButton({ | ||
putImage, | ||
id, | ||
}: { | ||
putImage: ImageEditorApi["putImage"] | ||
id: number | ||
}) { | ||
async function uploadImage({ file }: { file: File }) { | ||
const result = await fileToBase64(file) | ||
if (result) { | ||
putImage(id, result) | ||
} | ||
reader.readAsDataURL(file) | ||
} | ||
return ( | ||
<Upload showUploadList={false} customRequest={uploadImage}> | ||
<Button type="primary" icon={<FontAwesomeIcon icon={faUpload} />}> | ||
Upload image | ||
<Button className="ImageIndexPage__update-image-button" type="text"> | ||
Upload new version | ||
</Button> | ||
</Upload> | ||
) | ||
|
@@ -377,18 +442,44 @@ export function ImageIndexPage() { | |
success: true | ||
image: DbEnrichedImageWithUserId | ||
}>(`/api/images/${image.id}`, patch, "PATCH") | ||
setImages((prevMap) => ({ | ||
...prevMap, | ||
[image.id]: response.image, | ||
})) | ||
if (response.success) { | ||
setImages((prevMap) => ({ | ||
...prevMap, | ||
[image.id]: response.image, | ||
})) | ||
} | ||
}, | ||
postImage: async (image) => { | ||
const response = await admin.requestJSON<{ | ||
success: true | ||
image: DbEnrichedImageWithUserId | ||
}>(`/api/images`, image, "POST") | ||
if (response.success) { | ||
setImages((prevMap) => ({ | ||
...prevMap, | ||
[response.image.id]: response.image, | ||
})) | ||
} | ||
}, | ||
putImage: async (id, payload) => { | ||
const response = await admin.requestJSON<{ | ||
success: true | ||
image: DbEnrichedImageWithUserId | ||
}>(`/api/images/${id}`, payload, "PUT") | ||
if (response.success) { | ||
setImages((prevMap) => ({ | ||
...prevMap, | ||
[id]: response.image, | ||
})) | ||
} | ||
}, | ||
postUserImage: async (user, image) => { | ||
const result = await admin.requestJSON( | ||
`/api/users/${user.id}/image/${image.id}`, | ||
const response = await admin.requestJSON( | ||
`/api/users/${user.id}/images/${image.id}`, | ||
{}, | ||
"POST" | ||
) | ||
if (result.success) { | ||
if (response.success) { | ||
setImages((prevMap) => ({ | ||
...prevMap, | ||
[image.id]: { ...prevMap[image.id], userId: user.id }, | ||
|
@@ -397,7 +488,7 @@ export function ImageIndexPage() { | |
}, | ||
deleteUserImage: async (user, image) => { | ||
const result = await admin.requestJSON( | ||
`/api/users/${user.id}/image/${image.id}`, | ||
`/api/users/${user.id}/images/${image.id}`, | ||
{}, | ||
"DELETE" | ||
) | ||
|
@@ -439,7 +530,7 @@ export function ImageIndexPage() { | |
onChange={(e) => setFilenameSearchValue(e.target.value)} | ||
style={{ width: 500, marginBottom: 20 }} | ||
/> | ||
<ImageUploadButton setImages={setImages} admin={admin} /> | ||
<PostImageButton postImage={api.postImage} /> | ||
</Flex> | ||
<Table columns={columns} dataSource={filteredImages} /> | ||
</main> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 only works sometimes, so maybe it's not worth it and we just let authors know that it can take up to a minute for the image to update after refreshing the page. Not sure.
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.
If it doesn't work reliably, I'd add the disclaimer to the page. I'm not sure if I understand the intended functionality. If we want to refresh the image every time the
updatedAt
changes, I think we could pass it as akey
instead.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.
Updating the key was the first thing I tried, but it wouldn't actually make a new request, and then when I tried forcing it to re-render, it would load the cached version, so this is the only way I found to make it work. (Though it also sometimes hits the CF cache 😫)
I think I'll just keep it as it currently is and see what the authors make of it. I'll probably have to explain in slack that sometimes the preview doesn't update and that they need to click on the image to see what the actual current image is. Seems.. fine 🤷