add files for codereview #1
Conversation
.eslintrc.js
Outdated
| globals: {'_': true}, | ||
| // add your custom rules here | ||
| rules: { | ||
| indent: [2, 2, {SwitchCase: 1, ignoreComments: false, ImportDeclaration: 1, flatTernaryExpressions: false}] |
There was a problem hiding this comment.
여기도 tsconfig 쪽처럼 주석으로 가이드 하는 것이 어떨까요? 여기에서 예제로 설정한 것들은 상속받아서 쓰고 있으면 빼는게 맞을거 같아요.
.prettierrc
Outdated
| @@ -0,0 +1,8 @@ | |||
| { | |||
| "trailingComma": "none", | |||
There was a problem hiding this comment.
기본값은 굳이 선언할 필요가 없어 보입니다. 정적 분석 문서에서 예제 코드로 썼던 부분들을 활용한 경우 디폴트값인지 확인 부탁 드려요.
|
의논 필요 사항
|
README.md
Outdated
|
|
||
| ## Browser support | ||
|
|
||
| #### Supports all browsers |
package.json
Outdated
| "eslint-plugin-prettier": "^3.0.0", | ||
| "prettier": "^1.15.1" | ||
| }, | ||
| "dependencies": { |
There was a problem hiding this comment.
디펜던시는 없어야 할 것 같고, 있다면 차라리 코드 스니펫을 쓰는게 어떨까 합니다.
| } | ||
|
|
||
| .button:hover { | ||
| box-shadow: 0 12px 16px 0 rgba(0,0,0,0.24), 0 17px 50px 0 rgba(0,0,0,0.19); |
There was a problem hiding this comment.
브라우저 지원 범위가 IE9이상인 보일러 플레이트인지 확인이 필요할 거 같아요.
package.json
Outdated
| "name": "es5-boilerplate", | ||
| "version": "0.0.1", | ||
| "description": "", | ||
| "main": "index.js", |
There was a problem hiding this comment.
이 보일러플레이트에서는 필요없는 필드로 보여요. 모듈 개발 패턴으로 가지 않으면 제거하는게 나을 것 같습니다.
package.json
Outdated
| "lint": "eslint src", | ||
| "dev": "browser-sync start --config bs-config.js --startPath 'src/index.html'" | ||
| }, | ||
| "author": "", |
There was a problem hiding this comment.
작성자는 다른 컴포넌트처럼 아래와 같이 작성하면 될 것 같습니다.
NHNEnt FE Development Lab <dl_javascript@nhnent.com>
| text-align: center; | ||
| position: relative; | ||
| top: 50%; | ||
| transform: translateY(-50%); |
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <meta charset="UTF-8"> |
There was a problem hiding this comment.
head 쪽에 보다 풍부하게 보일러플레이트가 들어가야 할 것 같아요.
뷰포트 메타 태그랄지, og 태그 랄지.
예> https://gist.github.com/nunosans/3028849
이 부분은 우리의 보일러플레이트가 어떤 범위까지 제공하고 집중하는지 의논해 보아야 할 것 같습니다.
| </div> | ||
| <div id="message"></div> | ||
|
|
||
| <script> |
There was a problem hiding this comment.
문제 없는 코드이지만, 내부 스크립트는 외부 스크립트보다 아래에 있는게 안심하고 쓸 수 있지 않을까 의견 드립니다.
src/js/index.js
Outdated
| * @param {HTMLElement} element - control element | ||
| */ | ||
| function HelloWorld(element) { | ||
| var $wrapper = $(element); |
There was a problem hiding this comment.
jquery 바로 쓰면 린트에러 안 나던가요? .eslintrc의 "globals"에 추가해야 될거 같은데 확인 부탁합니다.
| this.addEvent(); | ||
| } | ||
|
|
||
| HelloWorld.prototype.addEvent = function() { |
|
[01/02] 리뷰 완료했습니다. 수고하셨어요. |
| @@ -0,0 +1,8 @@ | |||
| { | |||
| "trailingComma": "none", | |||
| "printWidth": 120, | |||
There was a problem hiding this comment.
토스트파일은 100 으로 사용중입니다만.. 우리 표준이 필요할 것 같네요.
|
|
||
| [hidden] { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
css가 불필요하게 긴 느낌이네요. 굳이 normalize를 포함하지 않아도 될 것 같아요.
|
* delete code for codereview * add files for codereview (#1) * add files for codereview * apply codereview * apply to codereview for readme * apply codereview 2 * apply codereview 3
No description provided.