-
Notifications
You must be signed in to change notification settings - Fork 103
feat: AuthorBarChart 렌더링 최적화 및 애니메이션 개선 #954
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
feat: AuthorBarChart 렌더링 최적화 및 애니메이션 개선 #954
Conversation
hyemimi
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.
LGTM👍👍 너무 멋진 PR인 것 같아요! PR 상세가 엄청 자세하고 전 후 비교가 잘 되어 있어서 이해하기 수월했습니다. d3 활용할 때 참고해보겠습니다ㅎㅎ
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.
데이터가 많아질 수록 확실히 성능 향상이 눈에 보이긴 하네요 👍 성능 측정 부분 그래프 이미지로 추출해서 보고서에 활용하면 좋을 것 같습니다!
PR 문서도 꼼꼼히 작성해주셔서 좋았습니다~ 핵심 기술적 개선 사항으로 요약해주셔서 감사합니다~
LGTM!!! 고생하셨습니다!!
| .data(data) | ||
| const bars = barGroup | ||
| .selectAll(".author-bar-chart__bar") | ||
| .data(data, (d: any) => d?.name || "") |
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.
명시적인 데이터 키를 이름으로 사용한 것 좋습니다 👍
다만 문서에 말씀하신대로 타입을 any 가 아니고 명확히 정의하면 좋을 것 같습니다. any 가 여러곳에서 반복되는 만큼 제네릭 타입 정의 활용해도 좋고요~!!
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.
키값 넣으신거 좋네요!!!
| bars | ||
| .selectAll("image.author-bar-chart__profile-image") | ||
| .data(validImages, (d: any) => d?.name || "") |
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.
이미지도 join() 활용하면 좋을 것 같아요!
이 부분 이슈 생성해서 다음 작업으로 진행하시는거 어떠신가요?ㅎㅎ
(질문) Promise.all으로 일괄 DOM 조작으로 개선하셔서 join 사용안하신건가 궁금합니다!
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.
제가 처음에는 이미지 쪽에도 join을 적용해봤었는데, 이미지 로딩의 비동기적 완료 시점이랑 join의 데이터 바인딩 시점이 서로 잘 맞지 않아서 화면상 이미지 레이아웃이 좀 이상하게 나오더라구요... 😭 그래서 이미지 부분은 기존 방식을 유지하고 바 차트쪽만 join을 적용했습니다!
ytaek
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.
멋진 PR 입니당!!!! LGGGGGGTM!
| // Axis | ||
| const xAxis = d3.axisBottom(xScale).ticks(0).tickSizeInner(0).tickSizeOuter(0); | ||
| xAxisGroup.call(xAxis); | ||
| xAxisGroup.call(xAxis as any); |
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.
말씀하신대로 type을 axis쪽에서 먹여서 다음 PR에서 진행하시면 되겠습니다!!!
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.
넵 알겠습니다!!
| const xAxisGroup = svg | ||
| .append("g") | ||
| .selectAll(".x-axis") | ||
| .data([null]) |
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.
(궁금) null 값이 하나만 들어간 배열을 바인딩 하신 이유가 따로 있을까요?
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.
d3는 데이터 기반으로 동작하기 때문에 .data() 메서드는 항상 배열을 요구합니다.
여기서는 x축, y축, 컨테이너 그룹처럼 단일 요소만 필요한 경우이기 때문에 길이가 1인 배열 [null]을 전달했는데, 사실 null 값 자체는 큰 의미가 없었습니다 😅
null을 넣는 게 코드 가독성이 안 좋아보인다면 다른 요소로 바꿔보겠습니다..!
| .data(data) | ||
| const bars = barGroup | ||
| .selectAll(".author-bar-chart__bar") | ||
| .data(data, (d: any) => d?.name || "") |
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.
키값 넣으신거 좋네요!!!
| (update) => { | ||
| update | ||
| .select("rect") | ||
| .on("mouseover", handleMouseOver) | ||
| .on("mousemove", handleMouseMove) | ||
| .on("mouseout", handleMouseOut) | ||
| .on("click", handleClickBar); |
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.
d3 진성 유저이십니다 👍👍👍👍👍
Related issue
#834
Result
🔍 기존 코드의 문제점 분석
1. 전체 DOM 재생성 문제
문제점:
svg.selectAll("*").remove()로 모든 요소 삭제2. 비효율적인 데이터 바인딩
문제점:
3. 개별적 이미지 DOM 조작
문제점:
⚡ 최적화 구현 내용
1. 선택적 DOM 업데이트 시스템
핵심 개선사항:
join패턴 활용)2. 안정적인 데이터 바인딩 시스템
핵심 개선사항:
d?.name을 키로 사용하여 요소 식별3. 일괄 이미지 처리 및 안정성 강화 시스템
핵심 개선사항:
📊 성능 테스트 결과
실제 브라우저 환경 측정 결과
데이터 크기별 성능 개선 현황
핵심 성과 요약
✅ 전체 성능 개선: 평균 25.96%
📈 성능 개선 패턴 분석
🎯 데이터 크기별 상세 분석
1. 소규모 데이터 (10개)
2. 중간 규모 데이터 (50개)
3. 중간 규모 데이터 (100개)
4. 대규모 데이터 (500개)
5. 대용량 데이터 (1000개)
실제 측정값 상세 데이터
50개 데이터 측정값
1000개 데이터 측정값
성능 개선 시각화
🎯 성능 개선 효과 분석
1. 사용자 경험 개선
2. 시스템 리소스 효율성
3. 확장성 및 안정성
📝 결론
AuthorBarChart 컴포넌트의 성능 최적화 작업을 통해 실제 브라우저 환경에서 평균 25.96%의 렌더링 성능 향상을 달성했습니다.
🎯 주요 성과
✅ 성능 개선 검증
📊 데이터 크기별 성능 향상
🔧 핵심 기술적 개선 사항
1. DOM 조작 최적화
2. 안정적인 데이터 바인딩
3. 일괄 이미지 처리 및 안정성 강화
기존 버전
authorbarchart_.2.mov
개선된 버전
authorbarchart_.2.mov
Discussion
any 타입 사용
any타입을 사용하고 있습니다.d?.name)과 기본값 처리로 어느정도 보장하였으나, 만약 이 부분에서 개선이 필요하다면 추후 새로운 이슈로 발행하여 해결하겠습니다!