Conversation
✅ Deploy Preview for mellifluous-gnome-003496 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
jungHyeonS
left a comment
jungHyeonS
left a comment
There was a problem hiding this comment.
영민님 과제 하시느라 고생하셨습니다~
js로 기능별로 분리해주셨고 css도 flex,grid적절히 사용해서 구현하신거같습니다~
전달드린 리뷰내용들만 추후에는 한번 고민하시면서 작업하시면 좋을꺼같습니다~
| circle.style.border = '1px solid #1b1b1b'; // 테두리 색상 변경 | ||
| } | ||
| }); | ||
|
|
||
| // 마우스가 요소에서 벗어났을 때 | ||
| circle.addEventListener('mouseleave', () => { | ||
| // 클릭된 요소가 아닌 경우에만 효과를 초기화 | ||
| if (circle !== clickedElement) { | ||
| circle.style.border = '1px solid #999999'; // 테두리 색상을 원래대로 되돌리기 |
There was a problem hiding this comment.
css 스타일은 가능한 자바스크립테서는 직접 조작하는 않는것을 추천드립니다, style 속성보다는 클래스를 만들고 class add, remove 를 이용해주시면 스타일 과 로직이 분리가 되어 코드 유지보수가 더 쉬워집니다
function addHoverEffect(circle) {
circle.addEventListener('mouseenter', () => {
if (circle !== clickedElement) {
circle.classList.add('hovered');
}
});
circle.addEventListener('mouseleave', () => {
if (circle !== clickedElement) {
circle.classList.remove('hovered');
}
});
}
| circle1.addEventListener('click', () => { | ||
| clickedElement = circle1; // 클릭한 요소를 저장 | ||
| circles.forEach(circle => circle.classList.remove('clicked')); // 모든 요소의 'clicked' 클래스를 제거 | ||
| clickedElement.classList.add('clicked'); // 클릭한 요소에만 'clicked' 클래스를 추가 | ||
| }); No newline at end of file |
There was a problem hiding this comment.
해당 코드를 확인해보니 페이지의 모든 circle 을 반복하면 clicked 클래스를 제거하는걸로 보입니다
그러나 실제로는 하나의 circle 만 clicked라는 클래스가 부여된 상태인거 같아 아래와 같은 코드를 제안드립니다
circle.addEventListener('click', () => {
if (clickedElement) {
clickedElement.classList.remove('clicked');
}
clickedElement = circle;
clickedElement.classList.add('clicked');
});
| const circleInfo = [ | ||
| { | ||
| href: "https://www.melon.com/album/detail.htm?albumId=11294520", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230727030134.jpg/melon/quality/80", | ||
| }, | ||
| { | ||
| href: "https://www.melon.com/album/detail.htm?albumId=11292865", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230726010900.png/melon/quality/80", | ||
| }, | ||
| { | ||
| href: "https://www.melon.com/album/detail.htm?albumId=11294303", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230727030153.jpg/melon/quality/80", | ||
| }, | ||
| { | ||
| href: "https://www.melon.com/album/detail.htm?albumId=11294409", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230727030212.jpg/melon/quality/80", | ||
| }, | ||
| { | ||
| href: "https://www.melon.com/album/detail.htm?albumId=11292440", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230726112148.png/melon/quality/80", | ||
| }, | ||
| { | ||
| href: "https://www.melon.com/album/detail.htm?albumId=11292423", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230726012238.png/melon/quality/80", | ||
| }, | ||
| { | ||
| href: "https://www.melon.com/event/view/index.htm?eventId=35425", | ||
| src: "https://cdnimg.melon.co.kr/svc/images/main/imgUrl20230727030231.jpg/melon/quality/80", | ||
| }, | ||
| ] |
There was a problem hiding this comment.
작성해주신 코드와 같이 정적인 데이터들은 따로 파일로 관리해주시는게 좋을꺼같습니다~
| for (let i = 0; i < circleInfo.length; i++) { | ||
| const circle = document.getElementById(`circle${i + 1}`); | ||
| circle.addEventListener("click", handleCircleClick); | ||
| // 각 circle 요소에 클릭 이벤트를 추가하고, 클릭 시 handleCircleClick 함수를 실행 | ||
| // circle 요소들은 HTML에서 id가 'circle1', 'circle2', ...와 같이 숫자를 포함한 형태로 되어 있다. | ||
| // 숫자를 이용하여 circleInfo 배열에서 해당 이미지 정보를 찾는다. | ||
| } | ||
|
|
||
| // 이벤트 처리 함수 정의 | ||
| function handleCircleClick(event) { | ||
| // 모든 circle의 테두리를 원래 색상으로 초기화 | ||
| for (let i = 0; i < circleInfo.length; i++) { | ||
| const circle = document.getElementById(`circle${i + 1}`); | ||
| circle.style.border = "1px solid #999999"; | ||
| } |
There was a problem hiding this comment.
해당 코드에서 모든 circle들에 이벤트 리스너가 등록되는것을 확인할수있습니다 이러한 부분을 개선하기 위해 이벤트 위임을 해주시면 성능, 코드 가독성이 더욱 올라갑니다~
const move = document.getElementById('move')
move.addEventListener('click',(event) => {
if(event.target && event.target.id.includes('circle')) {
circleInfo.forEach((circle, i) => {
const element = document.getElementById(`circle${i + 1}`);
element.style.border = "1px solid #999999";
});
handleCircleClick(event);
}
});
There was a problem hiding this comment.
전체적으로 기능 주석을 잘 달아두셔서
가독성이 좋았습니다!
또 css 파일을 각 구역별로 나눠놓으신 것도 확인했습니다!
There was a problem hiding this comment.
자바스크립트를 기능별로 나눠놓으신 부분도
이후 중요한 기능 구현 때 참고해보겠습니다!
There was a problem hiding this comment.
호버 기능을 넣을 때 이거는 JS 로 구현해야 되는게 아닌가 라고 생각해서 JS로 구현 했습니다! css로 구현이 가능하다면 수정 해보겠습니다 ㅎㅎ
There was a problem hiding this comment.
아하 넵!! li태그에 hover를 주는 방식으로 구현해보셔도 좋을 것 같습니다!
|
구조가 쉽지 않아 보이는데 grid 와 flex로 저렇게 구현하신게 대단하신데요? |
|
와! 싱크로율이 너무 좋아요! 클릭시 링크까지 연결 해주셔서 실제사이트인 줄 알았어요! |
| <dt> | ||
| <span class="none">The 6th Single Album 'Find Summer'</span> | ||
| </dt> | ||
| <dd id="img" class="img"> | ||
| <span class="none">앨범이미지</span> | ||
| <span class="thum"> | ||
| <img src="/assets/images/new_album/img1.jpeg" alt="최신 앨범1"> | ||
| </span> | ||
| <section id="singer_section" class="singer_section"> | ||
| <dd class="singer"> | ||
| <span class="none">가수명</span> | ||
| <a class="singer_a" href="https://www.melon.com/artist/timeline.htm?artistId=2204001" title="세러데이 (SATURDAY) - 페이지 이동"> | ||
| <span>세러데이 (SATURDAY)</span> | ||
| </a> | ||
| </dd> | ||
| </section> | ||
| </dd> |
There was a problem hiding this comment.
dl dt dd 마크업은 정의형 목록으로 대상과 대상에 대한 설명을 위한 마크업 입니다. 위 코드에서 The 6th Single Album 'Find Summer'이라는 것을 설명하기 위한 이미지로 dd를 설정했지만 이미지를 dt로 설정하고 그 이미지에 대한 설명으로 dd를 사용하는 것이 더 본래 목적에 가까울 것 같습니다. 이것또한 html5에서는 description 역할이 가능하기 때문에 문제가 있는 마크업은 아니지만 본래 목적인 독립적 컨텐츠와 그 설명을 마크업하는 figure요소가 더욱 시멘틱한 마크업이라고 생각합니다.
https://aoa.gitbook.io/skymimo/aoa-2019/tips-2/dl-dt-dd-. (dl,dt,dd 태그 관련 웹 접근성)
https://developer.mozilla.org/ko/docs/Web/HTML/Element/figure (mdn / 공식 문서 figuer 태그 설명)
또한 hover시 변경되는 디자인을 html에 마크업 한 뒤 js로 해당 요소를 탐색 및 변경하는 작업 하기보다는 DOM요소에 직접적으로 추가시키지 않고 가상요소 선택자를 활용하여 hover시에 추가해준다면 (ex dt:hover::after) HTML을 콘텐츠 없이 사용하는 것을 최소화할 수 있고, 해당 DOM을 탐색하고 접근하고 변경하는 동작들을 추가 수행해줄 필요가 없어질 것 같습니다.
There was a problem hiding this comment.
상세한 설명 감사합니다! 해당 내용 참고해서 코드 리팩토링 해보겠습니다!
7 participants
🎧 멜론 홈페이지 클론 코딩 🎧
🔗 프로젝트 URL
멜론 홈페이지 URL : https://www.melon.com/index.htm
과제 URL : yeongmin-melonclone.netlify.app
📌 필수 요구사항 체크리스트
📌 선택 요구사항 체크리스트
🗓️ 개발 기간
2023.07.24 ~ 2023.07.28
🔨 사용 기술 스택
🧑🏻💻 주요 구현 사항
HTML
CSS
JavaScript
📝 아쉬운점 & 느낀점
⌨️ 추후 구현사항