-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,16 +57,22 @@ const AuthorBarChart = () => { | |
| const svg = d3.select(svgRef.current).attr("width", DIMENSIONS.width).attr("height", DIMENSIONS.height); | ||
| const tooltip = d3.select(tooltipRef.current); | ||
|
|
||
| svg.selectAll("*").remove(); | ||
|
|
||
| const totalMetricValues = data.reduce((acc, item) => acc + item[metric], 0); | ||
|
|
||
| const xAxisGroup = svg | ||
| .append("g") | ||
| .selectAll(".x-axis") | ||
| .data([null]) | ||
| .join("g") | ||
| .attr("class", "author-bar-chart__axis x-axis") | ||
| .style("transform", `translateY(${pxToRem(DIMENSIONS.height)})`); | ||
| const yAxisGroup = svg.append("g").attr("class", "author-bar-chart__axis y-axis"); | ||
| const barGroup = svg.append("g").attr("class", "author-bar-chart__container"); | ||
|
|
||
| const yAxisGroup = svg.selectAll(".y-axis").data([null]).join("g").attr("class", "author-bar-chart__axis y-axis"); | ||
|
|
||
| const barGroup = svg | ||
| .selectAll(".author-bar-chart__container") | ||
| .data([null]) | ||
| .join("g") | ||
| .attr("class", "author-bar-chart__container"); | ||
|
|
||
| // Scales | ||
| const xScale = d3 | ||
|
|
@@ -84,13 +90,15 @@ const AuthorBarChart = () => { | |
|
|
||
| // Axis | ||
| const xAxis = d3.axisBottom(xScale).ticks(0).tickSizeInner(0).tickSizeOuter(0); | ||
| xAxisGroup.call(xAxis); | ||
| xAxisGroup.call(xAxis as any); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 말씀하신대로 type을 axis쪽에서 먹여서 다음 PR에서 진행하시면 되겠습니다!!!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넵 알겠습니다!! |
||
|
|
||
| const yAxis = d3.axisLeft(yScale).ticks(10).tickFormat(convertNumberFormat).tickSizeOuter(0); | ||
| yAxisGroup.call(yAxis); | ||
| yAxisGroup.call(yAxis as any); | ||
|
|
||
| xAxisGroup | ||
| .append("text") | ||
| .selectAll(".x-axis__label") | ||
| .data([null]) | ||
| .join("text") | ||
| .attr("class", "x-axis__label") | ||
| .style("transform", `translate(${pxToRem(DIMENSIONS.width / 2)}, ${pxToRem(DIMENSIONS.margins - 10)})`) | ||
| .text(`${metric} # / Total ${metric} # (%)`); | ||
|
|
@@ -159,48 +167,88 @@ const AuthorBarChart = () => { | |
| }; | ||
|
|
||
| // Draw bars | ||
| barGroup | ||
| .selectAll("rect") | ||
| .data(data) | ||
| const bars = barGroup | ||
| .selectAll(".author-bar-chart__bar") | ||
| .data(data, (d: any) => d?.name || "") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 명시적인 데이터 키를 이름으로 사용한 것 좋습니다 👍
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 키값 넣으신거 좋네요!!! |
||
| .join( | ||
| (enter) => | ||
| enter | ||
| .append("g") | ||
| .attr("class", "author-bar-chart__bar") | ||
| (enter) => { | ||
| const barElement = enter.append("g").attr("class", "author-bar-chart__bar"); | ||
|
|
||
| // 각 바 그룹에 rect 추가 | ||
| barElement | ||
| .append("rect") | ||
| .attr("width", xScale.bandwidth()) | ||
| .attr("height", 0) | ||
| .attr("x", (d) => xScale(d.name) || 0) | ||
| .attr("y", DIMENSIONS.height), | ||
| (update) => update, | ||
| (exit) => exit.attr("height", 0).attr("y", 0).remove() | ||
| ) | ||
| .on("mouseover", handleMouseOver) | ||
| .on("mousemove", handleMouseMove) | ||
| .on("mouseout", handleMouseOut) | ||
| .on("click", handleClickBar) | ||
| .attr("x", (d: any) => xScale(d?.name) || 0) | ||
| .attr("y", DIMENSIONS.height) | ||
| .on("mouseover", handleMouseOver) | ||
| .on("mousemove", handleMouseMove) | ||
| .on("mouseout", handleMouseOut) | ||
| .on("click", handleClickBar); | ||
|
|
||
| return barElement; | ||
| }, | ||
| (update) => { | ||
| update | ||
| .select("rect") | ||
| .on("mouseover", handleMouseOver) | ||
| .on("mousemove", handleMouseMove) | ||
| .on("mouseout", handleMouseOut) | ||
| .on("click", handleClickBar); | ||
|
Comment on lines
+191
to
+197
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. d3 진성 유저이십니다 👍👍👍👍👍 |
||
|
|
||
| return update; | ||
| }, | ||
| (exit) => { | ||
| exit.select("rect").transition().duration(250).attr("height", 0).attr("y", DIMENSIONS.height); | ||
|
|
||
| return exit.transition().duration(250).remove(); | ||
| } | ||
| ); | ||
|
|
||
| bars | ||
| .select("rect") | ||
| .transition() | ||
| .duration(500) | ||
| .attr("width", xScale.bandwidth()) | ||
| .attr("height", (d: AuthorDataType) => DIMENSIONS.height - yScale(d[metric])) | ||
| .attr("x", (d: AuthorDataType) => xScale(d.name) || 0) | ||
| .attr("y", (d: AuthorDataType) => yScale(d[metric])); | ||
| .attr("height", (d: any) => DIMENSIONS.height - yScale(d?.[metric] || 0)) | ||
| .attr("x", (d: any) => xScale(d?.name) || 0) | ||
| .attr("y", (d: any) => yScale(d?.[metric] || 0)); | ||
|
|
||
| // Draw author thumbnails | ||
| const barElements = d3.selectAll(".author-bar-chart__bar").nodes(); | ||
| if (!barElements.length) return; | ||
| bars.selectAll("image.author-bar-chart__profile-image").remove(); | ||
|
|
||
| // 새로운 이미지들 추가 (비동기 로딩) | ||
| const imagePromises = data.map(async (d: AuthorDataType) => { | ||
| if (!d?.name) return null; | ||
|
|
||
| try { | ||
| const profileImgSrc: string = await getAuthorProfileImgSrc(d.name).then((res: AuthorInfo) => res.src); | ||
| return { name: d.name, src: profileImgSrc }; | ||
| } catch (error) { | ||
| console.warn(`Failed to load profile image for ${d.name}:`, error); | ||
| return null; | ||
| } | ||
| }); | ||
|
|
||
| // 모든 이미지 로딩 완료 후 한번에 표시 | ||
| Promise.all(imagePromises).then((imageResults) => { | ||
| const validImages = imageResults.filter((result) => result !== null); | ||
|
|
||
| barElements.forEach(async (barElement, i) => { | ||
| const bar = d3.select(barElement).datum(data[i]); | ||
| const profileImgSrc: string = await getAuthorProfileImgSrc(data[i].name).then((res: AuthorInfo) => res.src); | ||
| bar | ||
| bars | ||
| .selectAll("image.author-bar-chart__profile-image") | ||
| .data(validImages, (d: any) => d?.name || "") | ||
|
Comment on lines
+237
to
+239
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이미지도 join() 활용하면 좋을 것 같아요! (질문) Promise.all으로 일괄 DOM 조작으로 개선하셔서 join 사용안하신건가 궁금합니다!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 처음에는 이미지 쪽에도 join을 적용해봤었는데, 이미지 로딩의 비동기적 완료 시점이랑 join의 데이터 바인딩 시점이 서로 잘 맞지 않아서 화면상 이미지 레이아웃이 좀 이상하게 나오더라구요... 😭 그래서 이미지 부분은 기존 방식을 유지하고 바 차트쪽만 join을 적용했습니다! |
||
| .enter() | ||
| .append("image") | ||
| .attr("class", "author-bar-chart__profile-image") | ||
| .attr("xlink:href", profileImgSrc ?? "") | ||
| .attr("x", (d: AuthorDataType) => (xScale(d.name) ?? 0) + xScale.bandwidth() / 2 - 7) | ||
| .attr("x", (d: any) => (xScale(d?.name) ?? 0) + xScale.bandwidth() / 2 - 7) | ||
| .attr("y", 204) | ||
| .attr("width", 14) | ||
| .attr("height", 14); | ||
| .attr("height", 14) | ||
| .attr("xlink:href", (d: any) => d?.src ?? "") | ||
| .style("opacity", 0) | ||
| .transition() | ||
| .duration(300) | ||
| .style("opacity", 1); | ||
| }); | ||
| }, [ | ||
| data, | ||
|
|
||
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을 넣는 게 코드 가독성이 안 좋아보인다면 다른 요소로 바꿔보겠습니다..!