fix: help menu tooltip (fixed #266) #330
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
dongsik-yoo
reviewed
Feb 12, 2020
src/js/ui.js
Outdated
| @@ -159,9 +150,9 @@ class Ui { | |||
| */ | |||
| changeUndoButtonStatus(enableStatus) { | |||
| if (enableStatus) { | |||
| this._els.undo.classList.add('enabled'); | |||
| this._buttonElements.undo.classList.add('enabled'); | |||
There was a problem hiding this comment.
변수 cache 하는게 어떠세요? Dot으로 쓰는게 넘 기네요.
There was a problem hiding this comment.
그러게요... dot chain 길어지니 뭔가 불편하네요...ㅎㅎ
지금의 메서드 구성 말고 더 좋은 방법은 없을까요?
There was a problem hiding this comment.
undo, redo, reset, delete, deleteAll 버튼에 해당하는 메서드들의 패턴이 동일하네요. 메서드 한개로 바꾸겠습니다.
classList를 직접 사용하면서 chain이 더 길어 보이는것 같네요..
좀 고민해보고 추후에 UI쪽에 같은 패턴으로 구현된 부분들, 이슈 만들어서 일괄 수정해보도록 하겠습니다.
이번에는 dot chain을 일단 변수캐시 하여 사용해보겠습니다.
src/js/ui.js
Outdated
|
|
||
| /** | ||
| * Make tooltip for help menus | ||
| * @param {string} menuName - menu name |
There was a problem hiding this comment.
파라미터가 없습니다. JSDOC에서 빼야 할거 같아요.
There was a problem hiding this comment.
넵 반영하겠습니다. 감사합니다.
jinwoo-kim-nhn
changed the title
HerlinMatos
pushed a commit
to EveryMundo/tui.image-editor
that referenced
this pull request
Jul 2, 2020
* fix: help menu tooltip * apply codereview Co-authored-by: superlucky84 <[email protected]>
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.
Please check if the PR fulfills these requirements
fix #xxx[,#xxx], where "xxx" is the issue number)Description
issue
작업내용