feat: redesign insert image modal with URL embed, caption, and link options#128
Merged
Merged
Conversation
wendyyuchensun
force-pushed
the
image-modal
branch
4 times, most recently
from
07b602d to
50640af
Compare
wendyyuchensun
force-pushed
the
image-modal
branch
18 times, most recently
from
f050c68 to
4ffce39
Compare
wendyyuchensun
force-pushed
the
image-modal
branch
from
4ffce39 to
c9f5c8b
Compare
wendyyuchensun
changed the title
wendyyuchensun
force-pushed
the
image-modal
branch
from
c9f5c8b to
edd5e99
Compare
Collaborator
Author
|
drag & drop upload image 的區域，可以試用、討論有沒有辦法更 accessible。 按照 WCAG，理論上有下面的「選擇檔案上傳」按鈕，應就符合規範：
但覺得仍可以討論看看有沒有更好的做法～ |
Collaborator
Author
|
另外有兩個細節，目前沒有設計，會再跟設計師確認是否需要，會另外在 PR 加上：
|
wendyyuchensun
force-pushed
the
image-modal
branch
from
edd5e99 to
88ee40e
Compare
wendyyuchensun
force-pushed
the
image-modal
branch
from
88ee40e to
512bf1d
Compare
jamliaoo
reviewed
May 18, 2026
Comment on lines
+63
to
+68
| img.onload = () => | ||
| setImageInfo({ | ||
| width: img.width, | ||
| height: img.height, | ||
| size: (file.size / 1024).toFixed(0) + 'KB', | ||
| }); |
Collaborator
There was a problem hiding this comment.
一個比較極端的情境，目前 processFile 沒有取消機制，如果使用者連續換圖片（A→B），A 的 onload 可能晚於 B 的 setState 才觸發，導致 imageInfo 顯示為 A 的尺寸但預覽是 B。
可能可以用一個 requestId ref 來做個檢查
Comment on lines
+99
to
+108
| useEffect(() => { | ||
| if (uploadError || embedError || !previewUrl) { | ||
| onChange(null); | ||
| return; | ||
| } | ||
| onChange({ | ||
| file: uploadFile, | ||
| sourceUrl: uploadFile ? null : previewUrl, | ||
| }); | ||
| }, [uploadError, embedError, previewUrl, onChange, uploadFile]); |
Collaborator
There was a problem hiding this comment.
nit
覺得可以評估看看是不是在 processFile / handleEmbed / handleRemoveImage 等情境裡直接呼叫 onChange，就可以不用這個 useEffect。好處是避免再多一輪 render、也讓內部 state 和外部通知（onChange）一次完成
Collaborator
Author
There was a problem hiding this comment.
感謝建議，make sense，而且覺得這樣改 code 比較清爽。
Updated in: fed1269
wendyyuchensun
force-pushed
the
image-modal
branch
4 times, most recently
from
f079fef to
438b06b
Compare
wendyyuchensun
force-pushed
the
image-modal
branch
from
438b06b to
c755377
Compare
jamliaoo
approved these changes
May 19, 2026
Collaborator
Author
|
先 merge，讓大家可以在 stage 環境測試。有問題或新的討論會再 follow up，感謝。 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Solves #115 .
Summary
BasicModal,TextInput,RadioGroup,@tabler/icons-react).role="status"live region announcing upload/embed error to screen readers![alt][[caption]](source)((targetUrl))fromedit-icons-tab.jsDesign: https://www.figma.com/design/7gvWa3UzE4w5Kloy2CYGTb/%E5%B7%A5%E5%85%B7%E9%A1%9E%E8%A8%AD%E8%A8%88%E7%B3%BB%E7%B5%B1_2025?node-id=708-4804&t=uGQQmEwBkFofw7qY-0