Feat/design ui useable #38
Merged
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
kyuwoo-choi
reviewed
Jun 19, 2018
src/js/component/icon.js
Outdated
| @@ -38,6 +39,8 @@ class Icon extends Component { | |||
| * @type {Object} | |||
| */ | |||
| this._pathMap = pathMap; | |||
| // this.useDragAddIcon = graphics.useDragAddIcon; | |||
kyuwoo-choi
reviewed
Jun 19, 2018
| realTimeEvent: true, | ||
| min: 0, | ||
| realTimeEvent: false, | ||
| min: 2, |
kyuwoo-choi
reviewed
Jun 19, 2018
| @@ -21,6 +21,13 @@ describe('UI', () => { | |||
| }; | |||
| ui = new UI(document.createElement('div'), uiOptions, {}); | |||
| }); | |||
| describe('_changeMenu()', () => { | |||
seonim-ryu
reviewed
Jun 19, 2018
src/css/submenu.styl
Outdated
| .{prefix}-main.tui-image-editor-menu-draw, | ||
| .{prefix}-main.tui-image-editor-menu-filter | ||
| .{prefix}-submenu |
There was a problem hiding this comment.
프리픽스 매번 붙여줄 필요 없이 아래 함수 쓰면 될 것 같아요.
+prefix-classes(prefix)
seonim-ryu
reviewed
Jun 19, 2018
src/js/graphics.js
Outdated
| * @ignore | ||
| */ | ||
| class Graphics { | ||
| constructor(element, cssMaxWidth, cssMaxHeight) { | ||
| constructor(element, {cssMaxWidth, cssMaxHeight, useItext = false, useDragAddIcon = false} = {}) { |
There was a problem hiding this comment.
이렇게 개행하면 더 깔끔하지 않을까요?
constructor(element, {
cssMaxWidth,
cssMaxHeight,
useItext = false,
useDragAddIcon = false
} = {}) {
// ...
}
seonim-ryu
reviewed
Jun 19, 2018
src/js/graphics.js
Outdated
| @@ -68,6 +70,18 @@ class Graphics { | |||
| */ | |||
| this.cssMaxHeight = cssMaxHeight || DEFAULT_CSS_MAX_HEIGHT; | |||
|
|
|||
| /** | |||
| * Use Itext mode for text component | |||
| * @type {number} | |||
seonim-ryu
reviewed
Jun 19, 2018
src/js/graphics.js
Outdated
|
|
||
| /** | ||
| * Use add drag icon mode for icon component | ||
| * @type {number} |
seonim-ryu
reviewed
Jun 19, 2018
src/js/graphics.js
Outdated
| * @private | ||
| */ | ||
| _onSelectionCreated(fEvent) { | ||
| const {target} = fEvent; |
There was a problem hiding this comment.
디스트럭처링해서 변수 할당까지 할 필요는 없어보여요~
seonim-ryu
reviewed
Jun 19, 2018
src/js/graphics.js
Outdated
| obj.hoverCursor = 'move'; | ||
| } else { | ||
| obj.hoverCursor = 'crosshair'; | ||
| } |
There was a problem hiding this comment.
한줄정리요ㅎㅎ
obj.hoverCursor = selectable ? 'move' : 'crosshair';
seonim-ryu
reviewed
Jun 19, 2018
src/js/imageEditor.js
Outdated
| * @private | ||
| */ | ||
| _removeObjectStream(targetObjects) { | ||
| if (targetObjects.length === 0) { |
There was a problem hiding this comment.
if (!targetObjects.length) { ... }
seonim-ryu
reviewed
Jun 19, 2018
src/js/ui.js
Outdated
|
|
||
| this._editorElement.style.top = `${editortop}px`; | ||
| this._editorElement.style.left = `${editorleft}px`; | ||
| if (menuBarPosition === 'top' && this._selectedElement.offsetWidth < 1235) { |
There was a problem hiding this comment.
1235라...... 이런 값은 매직넘버로 처리하던가 주석 꼭 추가해주세요~
seonim-ryu
reviewed
Jun 19, 2018
src/js/ui.js
Outdated
| if (menuBarPosition === 'top' && this._selectedElement.offsetWidth < 1235) { | ||
| this._selectedElement.classList.add('tui-image-editor-top-optimization'); | ||
| } else { | ||
| this._selectedElement.classList.remove('tui-image-editor-top-optimization'); |
There was a problem hiding this comment.
클래스명 상수로 빼서 사용하던가 변수 할당할 수 있겠네요~
위에 클래스 추가해주는 로직이랑 거의 유사해서요~
seonim-ryu
reviewed
Jun 19, 2018
src/js/ui.js
Outdated
| */ | ||
| _setUiSize(uiSize = this.options.uiSize) { | ||
| this._selectedElement.style.width = uiSize.width; | ||
| this._selectedElement.style.height = uiSize.height; |
There was a problem hiding this comment.
스타일 객체는 캐싱해서 사용해주세요~
let style = this._selectedElement.style;
seonim-ryu
reviewed
Jun 19, 2018
| * Interface method whose implementation is optional. | ||
| * Executed when the menu starts. | ||
| */ | ||
| changeStartMode() {} |
There was a problem hiding this comment.
changeStandbyMode 이 메서드랑 요건 추상 메서드인가요??
|
[6/19] 리뷰 완료하였습니다. 고생 많으셨어요~ |
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.
work
examples