10000 refactor:update tui-code-snippet@2.2.0 by heenakwag · Pull Request #25 · nhn/tui.color-picker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 5 commits into from
Jan 9, 2020

Conversation

heenakwag
Copy link
Member
  • set webpack optimization
  • apply toast-ui.release-notes
  • apply tui-code-snippet@2.2.0
  • refactor examples

Comment on lines +40 to +48
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README는 ES6로 작성이 된 것 같은데, 그럼 예제코드도 같이 수정이 되어야 하지 않을까요?_?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후 다른 PR을 통해 수정하겠습니다

Comment on lines 15 to 18
import ColorPicker from 'tui-color-picker';
import 'tui-color-picker/dist/tui-color-picker.css';

const colorpicker = ColorPicker.create({
Copy link
Member
@seonim-ryu seonim-ryu Jan 3, 2020

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({ ... });

Copy link
Member Author

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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

바벨을 사용하나요? 그렇다면 es6 코드를 사용해도 되겠네요.
일단 코드 변경하는건 작업량이 많으니 추후에 진행할 수 있도록 이슈 등록 부탁드릴게요~

Copy link
Member
@seonim-ryu seonim-ryu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[01/03] 리뷰 완료합니다. 컴포넌트마다 조금씩 구현 방식이 달라서(예: 인스턴스 생성 방식) 다른 컴포넌트들 작업할 때도 이 부분 유의해서 작업 부탁드릴게요~ 고생 많으셨어요! :)

@heenakwag heenakwag merged commit 6875715 into master Jan 9, 2020
@heenakwag heenakwag deleted the refactor/update-tui-code-snippet branch January 9, 2020 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0