-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add support for adding images and beautified attachments #100
Conversation
src/components/TheAttachments.vue
Outdated
const ALLOWED_EXTENSIONS = [ | ||
'txt', 'docx', 'doc', 'pptx', 'ppt', 'pdf', 'hwp', 'zip', '7z', | ||
'png', 'jpg' | ||
] |
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.
요 파일 형식들 모두 지금 지원이 되는 건가요?
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.
지원되는지 확인은 못해봤고 일단은 #68 보고 그대로 써둔 거에요!
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.
백엔드 쪽에 물어봐서 일단 되는 형식만 두고 나머진 빼는게 좋을 것 같아요. 업로드 하다가 에러 나는 것 보다는 지원 안한다는 쪽으로 가는게 좋을 듯? 그리고 jpeg 는 안되나염
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.
앗 jpeg 생각을 못했네요...
아니면 차라리 백엔드와 동일하게 파일 매직을 체크하거나 브라우저에서 제공하는 MIME type 체킹을 하는 것도 괜찮을 것 같네요.
일단 백엔드에 업로드 되는거 테스트해보고 목록 변경하겠습니다!
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.
확인해봤는데 전부 업로드 가능한 듯 합니다.
(혹은 아직 검증로직이 없는 거일 수도 있습니다.)
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.
음 검증 로직이 없을 가능성도 배제할 수는 없겠군요... 프론트에서 일단 몇가지 걸러내서 안전한 파일만 허용하는 방향은 어떻게 생각하세요?
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.
근데 사실 프론트에서 걸러내는 건 그냥 유저에게 알림만 하기 위함이기도 하고 (직접 요청 등으로 우회할 수도 있으니) 프론트에서 파일을 깊게 분석하는 것도 한계가 있고 해서 간단한 확장자 체킹 정도만 해도 괜찮을 것 같네요.
파일 매직까지 분석해보는 건 생각은 해봤는데 일단은 확장자랑 매직이 따로 노는 경우가 고의로 확장자를 바꾼 거 제외하면 많은 케이스가 아니라서 그냥 서버에서 리젝됐을 때 알림 주는 것만으로도 괜찮을 듯 하네요.
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.
알겠습니당 그 부분은 일단 다른 티켓으로 분리해서 나중에 더 다듬어도 좋을 거 같습니다. 사진 두개씩 보이는 버그까지만 수정해주세요!
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.
네 너무 방대해진 감이 없잖아 있네요ㅠㅠ 일단 수정했습니다
<i18n> | ||
ko: | ||
upload: '첨부파일' | ||
dropzone-normal: '여기를 클릭하거나 파일을 드래그 앤 드롭해주세요.' | ||
dropzone-drop: '여기에 드롭해주세요.' | ||
dropzone-unallowed-extensions: '허용되지 않은 확장자입니다.' | ||
|
||
en: | ||
upload: 'Upload Attachments' | ||
dropzone-normal: 'Please click here or drag & drop the files.' | ||
dropzone-drop: 'Please drop here.' | ||
dropzone-unallowed-extensions: 'This file type is not allowed.' | ||
</i18n> |
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.
👍
Deploy preview 에서 확인해보니 같은 첨부파일이 두개 올라가는 이슈가 있네요. (Mobile Safari, iPhone XS) +) 더 보니까 두번 올라가는게 아니고 두번 렌더링을 하는 것 같습니다. 기존의 포스트들도 전부 첨부파일이 두개가 보이는 군요. |
혹시 이미지라면 그게 아마 첫번째 이미지는 inline image로 들어간 거고 두번째 이미지는 첨부파일 목록에서 그리 뜨는 거일 거에요ㅠㅠ |
으흠 그렇군요 그런거면 postdetail 에서 첨부파일 목록은 가려야겠네요. 아예 첨부파일 목록 자체를 없애도 될 듯 (글 쓸때는 있고, 보여줄때는 없애는 쪽으로) |
Co-Authored-By: Leo Jeong <[email protected]>
src/components/ThePostDetail.vue
Outdated
@@ -53,6 +53,7 @@ | |||
<div class="content"> | |||
<TextEditor :editable="editable" :content="post.content"/> | |||
</div> | |||
|
|||
<div v-if="pictureUrls && pictureUrls.length !== 0"> |
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.
여기 v-for 로 구현된 부분 삭제하면 첨부파일 중복으로 안뜰 것 같은데 확인 바랍니당
<div v-if"pictureUrls ... /></div>
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.
아니면 차라리 이미지 미리보기 대신에 그냥 모든 첨부파일의 파일명만 띄울까요?
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.
글쓰기 화면에서는 그것도 괜찮을 것 같습니다.
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.
그 사실 글쓰기 화면이 아니더라도 지금 docx 같은 형식의 파일을 업로드 시에 다운받을 수 있는 링크가 없어서 그것 때문에 말씀 드린 거였습니다ㅠㅠ
일단은 파일명을 띄우게 했는데 혹시라도 첨부파일 없는 편이 더 괜찮다 싶으시면 지우고 다른 방법을 찾아보겠습니다.
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.
아하 그 부분 얘기셨군요 ㅋㅋㅋ 제가 이해를 못했네요... 네 일단은 첨부파일 명 띄우는 걸로 충분할 거 같아요. 더 좋은 방법은 나중에 고민해볼까요?
@HelloWorld017 검증 로직이 없는 게 맞습니다;; 열심히 만들어보겠습니다. 저 리스트에 있는 친구들만 하면 되려나요? |
일단은 이슈 #68 에서 들고왔습니다. gif 파일도 있으면 괜찮을 것 같긴 합니다. 사실 압축파일을 지원하면 굳이 막 늘릴 필요는 없을 것 같긴 합니다. |
* Added showing attachment, removed bottom redundant images showing * Fixed URIEncoded attachment names when editing post. * Added allowed extensions: jpeg, gif * Fixed clicking image decreases brightness even while viewing content. * Fixed lint errors
압축파일은 최대한 안받는게 좋아서 일반적으로 쓰일법한 문서 포맷들은 다 검증을 받으려고 합니다. 바이너리라도 압축해서 올렸다가 뉴아라가 랜섬웨어 배포의 장이 될 수도 있으니까요. - 즉 압축파일에 대해서 다운로드 시 경고 메시지를 구현해주셨으면 좋겠네요 ㅋㅋ (시간 되는대로..) |
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.
LGTM
+) 첨부파일 명 보여주는 곳은 제가 나중에 디자인 살짝 다듬을게요.
Changes
Beautified attachments
Added features that enables users to upload attachments by open dialog or drag & drop
Inline image support
When user adds image to the attachment, click image button and embed url, or paste the images are added to the content.
Editing attachment support
When user edits written post, the user can add/delete attachments
Related Issues
#64 #68