-
Notifications
You must be signed in to change notification settings - Fork 21
refactor:update tui-code-snippet@2.2.0 #25
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
Conversation
- set webpack optimization
- apply toast-ui.release-notes
- apply tui-code-snippet@2.2.0
- refactor examples
var result = document.getElementById('result'); | ||
|
||
var colorpicker = tui.colorPicker.create({ | ||
container: document.getElementById('color-picker') | ||
}); | ||
|
||
colorpicker.on('selectColor', function(obj) { | ||
console.log(obj); | ||
colorpicker.on('selectColor', function(ev) { | ||
result.value = JSON.stringify(ev, null, 8); | ||
console.log(ev); |
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.
README는 ES6로 작성이 된 것 같은데, 그럼 예제코드도 같이 수정이 되어야 하지 않을까요?_?
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을 통해 수정하겠습니다
docs/getting-started.md
Outdated
import ColorPicker from 'tui-color-picker'; | ||
import 'tui-color-picker/dist/tui-color-picker.css'; | ||
|
||
const colorpicker = ColorPicker.create({ |
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.
여기도 생성자 함수처럼 가져오고 있는데 변경해야겠습니다. 아니면 이렇게 인스턴스 생성하는 팩토리 함수만 가져오는 방식으로 가이드하는건 어떨까요?
import {create} from 'tui-color-picker';
const instance = create({ ... });
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.
create
만 있다면 어떤 인스턴스를 만드는지 명확하게 드러나지 않아서 코드 가독성이 떨어지지 않을까요? 기존 방식인 colorPicker.create
가 더 낫다고 생각합니다!
@@ -31,7 +31,7 @@ | |||
"license": "MIT", | |||
"devDependencies": { | |||
"@babel/core": "^7.5.5", | |||
"@toast-ui/release-notes": "^2.0.0", | |||
"@toast-ui/release-notes": "^2.0.1", | |||
"babel-eslint": "^7.1.0", | |||
"babel-loader": "^8.0.6", |
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.
바벨을 사용하나요? 그렇다면 es6 코드를 사용해도 되겠네요.
일단 코드 변경하는건 작업량이 많으니 추후에 진행할 수 있도록 이슈 등록 부탁드릴게요~
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.
[01/03] 리뷰 완료합니다. 컴포넌트마다 조금씩 구현 방식이 달라서(예: 인스턴스 생성 방식) 다른 컴포넌트들 작업할 때도 이 부분 유의해서 작업 부탁드릴게요~ 고생 많으셨어요! :)