Carousel slider[Fetch이용]#157
Merged
crongro merged 4 commits intocode-squad:superfly9from Feb 27, 2020
superfly9:animation
Merged
Carousel slider[Fetch이용]#157crongro merged 4 commits intocode-squad:superfly9from superfly9:animation
crongro merged 4 commits intocode-squad:superfly9from
superfly9:animation
Conversation
crongro
approved these changes
Feb 27, 2020
Contributor
crongro
left a comment
There was a problem hiding this comment.
좋아요. 리팩토링 없이 진행해도 될듯!
무리한 분리는 다음기회에 하시죠.
잘했어요.
| margin:0 auto; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| -webkit-transition: transform .5s; |
Contributor
There was a problem hiding this comment.
-webkit 요놈을 붙여야 할지 안붙여도 될지 한번 찾아보세요.
transition의 지원범위를 살펴보시면 될 듯.
| this.makingMenuList(); | ||
| this.makingSlider(); | ||
| this.btnHandler(); | ||
| this.count = 0; |
Contributor
There was a problem hiding this comment.
무슨 count인지 잘 모르겠어요. 구체적으로 의도를 드러내세요.
| const width = element.children[0].getBoundingClientRect().width; | ||
| element.style.transform = `translateX(${-this.count*width}px)`; | ||
| } | ||
| scaleMenu (element,index) { |
| scaleMenu (element,index) { | ||
| this.count = index; | ||
| Array.from(element.parentNode.children).forEach((element)=>element.style.transform=`scale(0.99)`) | ||
| element.parentNode.children[this.count].style.transform = 'scale(1.1)'; |
Contributor
There was a problem hiding this comment.
이 표현은 너무 길어요.
가로로 너무 길면 변수화를 시켜두는 게 좋아요.
디버깅을 할때도 유리하고요.
const parent = element.parentNode;
| const left =document.querySelector('div.carousel__button--prev'); | ||
| const right =document.querySelector('div.carousel__button--next'); | ||
| [left,right].forEach((btn,index)=>{ | ||
| btn.addEventListener('click',()=>{ |
Contributor
There was a problem hiding this comment.
수업때 말한 것처럼 콜백으로 동작하는 핸들러 같은 함수를 분리해보세요~
| btn.addEventListener('click',()=>{ | ||
| index === 0 ? this.count-- : this.count++; | ||
| if (this.count >3) this.count =0; | ||
| if (this.count<0) this.count =3; |
Contributor
There was a problem hiding this comment.
0,3 이런건 매직넘버에요. 변수화 시켜서 의미를 붙여서 사용하는게 더 좋아요.
| makingSlider () { | ||
| const carouselContainer = document.querySelector('.carousel__container'); | ||
| carouselContainer.innerHTML=this.contentData.map((data,index)=>` | ||
| <div class='carousel__contents'> |
Contributor
There was a problem hiding this comment.
html template 문자열이 너무 길다면 코드가 지저분해질 수 있겠죠?
만약 그렇다면 문자열도 다른곳에 선언해서 모아둘 수 있을거에요.
물론 이렇게 지금한것처럼 관련로직에 두고, 어떤 내용의 HTML 업데이트가 일어나는지 볼 수 있는 것도 응집도 차원에선 좋고요.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
-데모페이지: https://superfly9.github.io/javascript-amazon/
-작업을 할 때마다 커밋을 해야하는데 완성이 안되면 부끄럽다는 생각이 커서,
무언가 완성되야만 커밋해야 하지 않나라는 강박이 있습니다.더 많은 커밋로그를 남기도록 해보겠습니다.
-클래스를 나눠서 해보려 하고,파일도 나눠보려 했으나 어떻게 해야할지 생각이 잘 안나서 코드를 치다보니
하나의 파일과 클래스에 모든 것을 때려넣게 되었습니다.설계에 많은 시간을 투자한 후에 코드를 쳐보려 했으나
정말이지 감이 안와서요.오히려 코드를 치다보니 감이 오는 것 같습니다.꾸준한 생각과 설계면 이보다 더
큰 프로그램의 설계도 손쉽게 가능해질까 하는 의문이 들었습니다.
-express사용 대신 제 로컬 저장소에 데이터 저장 후 불러와서 vscode에서 live-server를 사용했습니
다.
-localStorage.setItem으로 불러온 데이터를 저장하려고 하는 데 이것도 얼른 해보겠습니다.