Skip to content

[미션 제출] Animation#137

Merged
crongro merged 13 commits intocode-squad:choisohyunfrom
choisohyun:animation
Feb 17, 2020
Merged

[미션 제출] Animation#137
crongro merged 13 commits intocode-squad:choisohyunfrom
choisohyun:animation

Conversation

@choisohyun
Copy link
Copy Markdown

Git-Page

https://choisohyun.github.io/javascript-amazon-anim/

구현 사항

  • mock data를 활용하여 html 코드를 만들었습니다
  • 슬라이드를 자연스럽게 넘기기 위해 처음과 마지막에 슬라이드를 하나씩 더 붙였습니다.
  • 단일 클래스였던 AmazonCarousel를 navigationBar와 carousel로 나눴습니다.

문제

  • 현재는 클래스를 크게 네비게이션과 슬라이드로 나눴는데, 중복되는 코드가 꽤 많이 존재합니다. transitionend는 따로 나누니 오류가 나서 일단 중복된 상태로 두었습니다.
  • 다이어그램을 간단히 그리고 클래스 나누기를 했는데, 그린 것과 달라졌습니다. 설계할 때 생각을 더 많이 해야 할 것 같습니다.

image

choisohyun and others added 13 commits February 11, 2020 16:44
- 아마존 슬라이드 화면을 클론코딩한 파일 업로드
- 슬라이드 클릭 시 화면 이동은 아직 구현하지 않음
- 이미지, 콘텐츠는 dummy 파일 이용함
- 아마존 슬라이드 화면을 클론코딩한 파일 업로드
- 슬라이드 클릭 시 화면 이동은 아직 구현하지 않음
- 이미지, 콘텐츠는 dummy 파일 이용함
- 슬라이드의 양 옆 화살표 버튼 클릭 시 다음 화면이 나오는 기능을 구현함
- 상단의 네비게이션 클릭 시 이동은 아직 구현하지 못함
- 마지막 슬라이드에서 다음 버튼을 누르면 첫 번째 슬라이드가 나옴
- 처음 슬라이드에서 이전 버튼을 누르면 마지막 슬라이드가 나옴
- js 코드를 ES6 Classes로 표현하기 위해 변경함
- 기능 추가는 없음
- 각각의 navigation 버튼을 클릭하면 해당 버튼의 index를 얻어와 이동함
- 슬라이드 애니메이션 기능은 아직 추가하지 않음
data.js - mock data를 임의로 배열로 저장함
mockData.js - mock data를 html 파일로 순서대로 추가될 수 있도록 함
- addDummySlide(function)
 부드럽게 넘어가는 무한 슬라이드를 구현하기 위해 생성함
 slide의 first, last child를 각각 마지막과 처음에 넣어 줌
 추후 파일명, 변수명 등 리팩토링 예정임
- index.js -> main.js, navigationBar.js, carousel.js로 클래스 나눔
- slideSetting.js -> 슬라이드 구현에 필요한 요소를 객체로 따로 저장함
- sprinkleData.js -> mockData.js에 배열로 있는 데이터를 뿌려주는 역할을
함
- navigationBar.js와 carousel.js는 다른 클래스이지만 중복되는 코드가
존재함
=> 추후 리팩토링 필요
화살표 버튼의 위치가 어색해 width를 100% -> 800px로 변경함
Copy link
Copy Markdown
Contributor

@crongro crongro left a comment

Choose a reason for hiding this comment

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

네 여기까지 미션 확인했어요.

중복되는 코드가 많다고 하셨는데, 그거 수정이 원래 오래 걸려요.
중복없애다가 코드수정이 많이 일어나기도 하기 때문이에요.

}

moveSlides() {
let transformOption = `translateX(${-760 * this.currentIndex}px)`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const MAX_PANEL_SIZE = 760;
let transformOption = translateX(${-MAX_PANEL_SIZE * this.currentIndex}px);

@@ -0,0 +1,19 @@
const carousel = document.querySelector(".slider");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

setting -> config

mockData.forEach(element => {
const title = element.title;

const newLi = document.createElement("li");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

지금 사용한 DOM API는 보통 빠른방법이라고 알려져 있음.
string 을 조작하는 방법. template literal 문법을 잘 활용해서!

if (this.slideChildren[this.currentIndex].className === "lastClone") {
this.slideAll.style.transition = "none";
this.currentIndex = this.slideChildren.length - 2;
this.slideAll.style.transform = `translateX(${-width * this.currentIndex}px)`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

크기가 작더라도 자주 사용되는 코드패턴이 보이면 함수로 만들어보세요.

run() {
window.addEventListener("DOMContentLoaded", () => {
const navigationBar = new NavigationBar(setting);
const carousel = new Carousel(setting);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

setting 이 분리되어 있는데, 좋은가 나쁜가는 전 반반입니다.
단점은 무슨값이 carousel에 들어가는지 너무 드러나지 않기때문인데요. 전 그 부분이 main.js에서 드러나도 될 거 같아요.
그부분을 보기 위해서 별도의 setting.js파일을 열어봐야 하는 것이라서요.

}

addSlide() {
mockData.forEach(element => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DOM API를 많이 사용했네요.
다음 미션을 할때는 template literal 을 활용한 방법으로 해보세요.

@crongro crongro merged commit 2d08d9b into code-squad:choisohyun Feb 17, 2020
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.

2 participants