Repository navigation
feat: refine forum settings, uploads, and transitions - #28
Conversation
|
Automated review has started. I am checking this pull request now. |
Bundle ReportChanges will increase total bundle size by 47.25kB (1.4%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: forumlify-next-client-array-pushAssets Changed:
view changes for bundle: forumlify-next-server-cjsAssets Changed:
Files in
Files in
Files in
Files in
Files in
Files in
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Superseded automated review
The review for commit f5d173350bb6 has been replaced by the automated review for commit 5f81b819af40.
Its previous inline review and change comments were removed. Refer to the latest commit review for the current result.
|
Automated review completed for commit |
There was a problem hiding this comment.
Automated review
Mode: normal
Comment
All three issues from the earlier review are resolved in the current head: the post.* translation keys (post.imageUpload, post.imageUploading, post.imageCompressing, post.imageTooLarge, post.imageCompressFailed, post.imageUploadFailed) now exist in both locales/en.json and locales/zh.json; the admin favicon swap is now transactional with SELECT ... FOR UPDATE and a concurrency regression test; and admin nav clicks during the opening animation are re-queued via pendingTab instead of being silently dropped. The compression helpers, image-type refactor, settings/favicon storage flow, and view transitions look consistent and are well guarded with fallbacks. The two remaining observations are non-blocking: the edit form's shared upload state is not per-file when multiple images are selected, and the new favicon route returns hardcoded Chinese errors that appear inside English toast templates.
Result for maintainers
All previously blocking findings are fixed in the current payload: post.* i18n keys are present in both locale files, the favicon settings swap is wrapped in a transaction with FOR UPDATE plus a concurrency test, and admin tab clicks during the opening animation are re-queued. No blocking issues remain. Non-blocking review items only: per-file edit-upload state race with multi-image selections, and hardcoded Chinese server errors surfaced through the new English toast template. No malicious code detected.
Model decision: safe to merge
Applied review policy: allow
Final status: safe to merge
Required changes: 0
Review notes: 2
| const url = await uploadImage(file); | ||
| let uploadFile = file; | ||
| if (file.size > DIRECT_IMAGE_LIMIT) { | ||
| setEditUploadStage('compressing'); |
There was a problem hiding this comment.
Review note: Shared edit-upload state is not per-file: button can finish while sibling uploads are still running
The edit form's file input calls handleEditUpload once per file ([...files].forEach(...)) without awaiting, so multiple large files run concurrently while each call mutates the shared editUploading/editUploadStage state. The first call to reach its finally block sets editUploading=false and editUploadStage='uploading' while a second file is still being compressed or uploaded, so the button reverts to 'Upload image' and re-enables mid-batch. Track an in-flight count or process the files sequentially so the button label and disabled state stay correct until the whole batch finishes.
| return Response.json({ error: '请选择图标文件' }, { status: 400 }); | ||
| } | ||
| if (file.size > MAX_SIZE) { | ||
| return Response.json({ error: '图标大小不能超过 5MB' }, { status: 400 }); |
There was a problem hiding this comment.
Review note: Server-side favicon errors are hardcoded Chinese and surface inside English toasts
The new AdminForumSettings UI renders upload failures through the localized template t('admin.forum.faviconSaveFailed', { msg: error.message }), but every error returned by this route ('请选择图标文件', '图标大小不能超过 5MB', '仅支持 PNG、JPEG、GIF、WebP 或 ICO 图标', '上传失败,请稍后重试') is hardcoded Chinese. In an English UI this yields toasts like 'Icon upload failed: 图标大小不能超过 5MB'. Consider returning error codes and mapping them to locale keys, as done for the other new messages in this PR.
Superseded by the automated review for commit 5f81b81.
|
@bot 这个改动主要改动了什么 |
|
这个 PR 主要围绕「管理后台设置、上传体验、页面过渡动画」三块做了优化,具体如下: 1. 管理员网站图标上传(新增)
2. 图片上传前自动压缩
3. Docker 与构建信息
4. 管理后台左侧抽屉过渡动画
5. 首页「发布新帖」按钮动画
6. 其他
简单说:给管理员加了图标上传和版本展示,大图上传前自动压缩,Docker 部署更干净,并且给管理后台和新帖页加了一套更顺滑的过渡动画。 |
Summary
Validation
git diff --checknode --test test/image-type.test.js test/i18n-ui.test.js(6 tests passed)npm run buildin an isolated copyNotes
.playwright-cli/artifacts remain local and are not included in this PR.