[Feat] Todo List 구현 #2
Open
Conversation
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
mike2ox
reviewed
Mar 30, 2023
Comment on lines
+7
to
+9
| <link rel="preconnect" href="https://fonts.googleapis.com"> | ||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | ||
| <link href="https://fonts.googleapis.com/css2?family=Lilita+One&family=Nanum+Gothic&display=swap" rel="stylesheet"> |
There was a problem hiding this comment.
폰트 관련해서 link해주셨는데 궁금한게 있어서 질문드립니다!
- preconnect 적용하고 안하고의 차이가 궁금합니다.
- gstatic에서만 crossorigin을 설정해주셨는데 이유가 있을까요?
There was a problem hiding this comment.
- preconnect를 적용하지 않았을 경우에는 css 파일 로딩을 위한 연결 → css 파일 로드 → 폰트 파일 로딩을 위한 연결 → 폰트 파일 로드 의 과정을 순차적으로 거쳐야 하기 때문에 최종 폰트 파일 로드까지 시간이 걸리지만, preconnect를 적용해서 미리 연결을 위한 초기 설정을 해 두면 최종 폰트 파일 로드까지 걸리는 시간이 preconnect를 적용하지 않았을 때 보다 빨라집니다.
- 폰트 파일을 요청하는 css 파일의 도메인과 폰트 파일이 도메인이 다르기 때문에 gstatic에 crossorigin을 설정해주었습니다.
Comment on lines
+37
to
+41
| @media (max-height: 500px) { | ||
| #todo-wrapper { | ||
| height: 300px; | ||
| } | ||
| } |
There was a problem hiding this comment.
미디어쿼리를 사용하셨는데 해당 스타일이 어떤 목적으로 사용되는지 알려주세요
Comment on lines
+15
to
+23
| <div id="todo-wrapper"> | ||
| <form id="todo-form"> | ||
| <input id="todo-input" autocomplete="off" placeholder="TODO 추가하기" type="text" required> | ||
| </form> | ||
| <hr /> | ||
| <ul id="todo-list"> | ||
| <!-- Dynamically added --> | ||
| </ul> | ||
| </div> |
There was a problem hiding this comment.
의도하신건지 모르겠다만 고정적인 부분은 미리 선언하셔서 좋은거 같아요 👍
todo.js
Outdated
There was a problem hiding this comment.
구현해야할 기능들에 대해 잘 만들어주신거 같아요!
몇가지 개선해 볼 만한 거 추천해 드릴게요
- UI와 logic 분리
- 재사용성을 고려한 구조
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
구현 사항