-
Notifications
You must be signed in to change notification settings - Fork 4
[Mission003] 자바스크립트로 Form 과 Modal 에 대한 UI 만들기 #31
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
base: master
Are you sure you want to change the base?
Conversation
|
미션때문에 자주 사용되고 있지만 몰랐던 Modal이라는 것에 대해 접하게 해주셔서 감사합니다. |
eastjun-dev
left a comment
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.
prototype을 통해 메모리 효율과, 재사용성을 높이신게 매우 인상 깊은 코드였어요~!
그런데, style에 대한 부분들을 css 파일에서 class name으로 하지 않고, js code에서 해결하신 이유가 있으실까요? 역할의 분리를 위해 css에서 해결하는것도 좋을 것 같은데 어떤 장점 때문에 이용하신건지 궁금합니다 :)
| this.$main = document.createElement('button') | ||
| this.$main.innerText = name | ||
|
|
||
| const buttonStyle = { |
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.
이 style에 해당하는 클래스를 css에 선언하고 class name으로 스타일을 적용한건 어떨까요?
|
|
||
| this.$rowLine = document.createElement('div') | ||
| const rowLineStyle = { | ||
| 'background-color': 'grey', |
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.
이 style에 해당하는 클래스를 css에 선언하고 class name으로 스타일을 적용한건 어떨까요?
그리고 현재는 컨벤션이 맞지 않는 것 같아요~
|
|
||
| this.$colLine = document.createElement('div') | ||
| const colLineStyle = { | ||
| 'background-color': 'grey', |
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.
이 style에 해당하는 클래스를 css에 선언하고 class name으로 스타일을 적용한건 어떨까요?
그리고 현재는 컨벤션이 맞지 않는 것 같아요~
| setStyle(this.$colLine, colLineStyle) | ||
| this.$main.appendChild(this.$colLine) | ||
|
|
||
| const buttonStyle = { |
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.
이 style에 해당하는 클래스를 css에 선언하고 class name으로 스타일을 적용한건 어떨까요?
그리고 현재는 컨벤션이 맞지 않는 것 같아요~
| } | ||
|
|
||
| export const setAttrs = (el, attrs) => { | ||
| for(const [k, v] of Object.entries(attrs)) { |
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.
k, v와 같은 너무 간략한 약어는 다른 사람들과 협업할 때 알아보기 어려운 것 같아요 ㅠ
조금 긴 것 같아도 key, value로 작성해주시는건 어떨까요?
#8
미션 진행 내역
소감
제가 가져온 미션이지만, 좀 많이 힘드네요 ㅜㅜ
아직 모달 이라던가, 디테일한 요구사항은 진행하지 못하였습니다. 조만간 업데이트 하겠습니다..!!
React + StyledComponent 의 아이디어를 이용해서 미션을 진행해 보았는데, 기능을 추가해나가면서 일관성이 점점 무너지는 것 같았습니다..ㅜㅜ JS 로 마크업 및 스타일링을 모두 진행하려 하니, CSS 의 가상요소를 처리하는 것이 쉽지 않았던 것 같습니다.