-
Notifications
You must be signed in to change notification settings - Fork 19
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
맞춤법 검사 기능 추가 #6
맞춤법 검사 기능 추가 #6
Conversation
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.
안녕하세요 @rycont 님.
멋진 기능과 함께 제 프로젝트에 기여해 주셔서 감사합니다.
코드 리뷰 결과 몇가지 이슈 사항이 있어서 한번 확인해 주시면 감사하겠습니다 :)
index.html
Outdated
|
||
document.getElementById("go_spellcheck")?.addEventListener("click", (e) => { | ||
e.target.style.setProperty("opacity", "0.5") | ||
fetch("https://spell.rycont.workers.dev/" + encodeURIComponent(textarea.innerText)) |
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.
해당 부분에서 버튼을 클릭하면 개인이 보유하신 cf worker server 로 작성된 내용을 보내는 듯 하는데, 이에 대한 이슈가 몇가지 있습니다.
- 현재 해당 웹사이트는 하루에 7000명이 접속하는 웹 사이트이고, 동시 접속자가 생각보다 많은 사이트 입니다. 보유하신 cf worker 로 많은 요청이 갈 수 있을 듯 한데요, 이에 대한 비용 발생 문제가 생길 가능성이 있어 보이는데 확인 부탁드립니다!
- 사이트로 요청을 보내게 될 경우, modal 등을 이용하여 이용자에게 입력 데이터가 맞춤법 검사 서버로 가게 된다는 걸 사전 공지해야 할 것 같습니다.
- 그리고 @rycont 님이 올려주신 개인 서버가 데이터를 수집하지 않는다는 것을 증명하여야 할 것 같습니다.. (근데 어떻게 증명을 해 주셔야 할지 저도 잘 모르겠습니다 😢)
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.
제 PR에 대해 깊이 고민해주셔서 고맙습니다🙏
-
Workers 프리티어가 1일 10만회로 여유있는 사용량을 가지고 있어서, 비용 관련해서 문제가 되진 않을 것 같습니다. 맞춤법 검사 기능이 서비스에 적용된다면 사용량 증가에 대비해서 rate limit을 설정해두려고 합니다.
-
다만 말씀해주신 것과 같은 정보수집 우려에 대한 문제는 제가 미처 생각하지 못했네요ㅜ 말씀주신 것과 같이 텍스트 정보만 맞춤법 검사기 서버로 전송되며, 내용은 저장되지 않는다는 안내와 함께 기능을 추가하는 편이 맞을 것 같습니다.
2.1. 다만 걱정되는 부분이 한 가지 있는데요, 현재 운용중인 페이지는 사용자의 이용면에서도, 개발용 코드면에서도 매우 단순한 구조를 가지고 있어 직관적으로 이용하고 개발할 수 있다는 점이 가장 큰 장점이라고 생각합니다. 여기에 데이터 전송에 관련된 내용을 안내하는 모달과 같은 추가 요소가 들어간다면, 이와 같은 장점이 희석될 수도 있어 보입니다. 현재 코드에 안내 모달을 추가하는 방안을 허락해 주신다면 진행하도록 하겠습니다! 만약 본 서비스의 테마와 어울리지 않는다면 포크한 제 레포에만 적용해두겠습니다.
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.
답변해 주셔서 감사합니다!
그러면 한번 적용해 주셔도 좋을 것 같습니다 :)
UX 적인 측면에서 현재 사이트와 큰 차이가 없도록 (맞춤법 검사 버튼을 클릭해야지만 모달이 띄워지게 되고 모달 내에 동의 버튼을 클릭하면 localStorage 에 동의 버튼을 누른 시각을 기록한 후 사용할 수 있도록) 구현해 주시면 정말 감사하겠습니다!
3번의 경우 2번 방법으로 동의를 구하면 해결될 것 같습니다 👍
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.
네, 현재 페이지의 디자인을 최대한 고수하는 한에서 정보 전송 동의 모달을 구현해서 다시 PR드리겠습니다. 긍정적으로 고려해주셔서 고맙습니다 🤗
|
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.
멋진 기능 추가해 주셔서 감사합니다!
추가 작업 사항이 없으실 경우 해당 리뷰에 👍 남겨주시면 머지 진행하도록 하겠습니다.
다시 한번 감사드립니다 😊
(좀 예전에 개발한 토이 프로젝트여서 코드가 많이 난잡하였을텐데, 작업해 주셔서 감사합니다 ㅠ)
마지막으로 작동 테스트 했습니다! 제 깃헙페이지에 올렸을 때는 잘 됐는데, 프로덕션에 올라가서도 잘 됐으면 좋겠네요.... 머지 부탁드리겠습니다🤗 |
부산대 맞춤법검사기 Integration을 추가하였습니다