Skip to content
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

fix(view): #553 Author(other bar) 클릭시 해당 사용자들 관련 커밋내역이 나오지는 않는 문제 수정 #559

Merged
merged 6 commits into from
Jul 30, 2024

Conversation

HIITMEMARIO
Copy link
Contributor

@HIITMEMARIO HIITMEMARIO commented Jul 26, 2024

Related issue

#553

Result

10명의 사용자가 초과되는 경우 slice로 나눠 나머지는 Others로 분류시켰습니다. 또 그 Others bar를 클릭하게 되면 topAuthors 를 제외한 나머지(Others)의 커밋 내역들 역시 summary에 보여지도록 수정하였습니다. 다만 (#540)해당 이슈는 며칠전 수정이 되었음에도 others바에는 적용이 안되었기에 불가피하게 이전 수정 부분에서 수치를 좀 더 주었습니다.

Work list

image
image

Discussion

아쉬운 점은 others 바를 끝까지 타고 들어갔을때 그 이전의 author들을 보여주지 못한다는 점이 조금 아쉽습니다...ㅠ 이 부분은 따로 버튼을 넣어 뒤로가기를 만들어야만 할거 같아 해당부분 이슈 올리겠습니다...!! (해당 이슈는 이미 올라와있는 것 같아 따로 올리지 않겠습니다 #529) 그 외에도 보다 효율적인 방법이 있다면 피드백 주시면 감사하겠습니다...!😁

@HIITMEMARIO HIITMEMARIO added bug Something isn't working ✨ view labels Jul 26, 2024
@HIITMEMARIO HIITMEMARIO self-assigned this Jul 26, 2024
@HIITMEMARIO HIITMEMARIO requested review from a team as code owners July 26, 2024 09:40
@@ -51,12 +51,16 @@ export const convertNumberFormat = (d: number | { valueOf(): number }): string =
return d3.format("~s")(d);
};

export const sortDataByAuthor = (data: ClusterNode[], author: string): ClusterNode[] => {
export const sortDataByAuthor = (data: ClusterNode[], names: string[]): ClusterNode[] => {
Copy link
Contributor

Choose a reason for hiding this comment

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

변수명을 바꾸신 이유가 있는지 궁금합니당!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

기존에는 각각의 이름을 받았다면 수정 된 이후에는 배열을 받기때문에 names로 변경하게 되었습니다!!

joonwonBaek
joonwonBaek previously approved these changes Jul 28, 2024
Copy link
Contributor

@joonwonBaek joonwonBaek left a comment

Choose a reason for hiding this comment

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

준혁님 issue 해결을 너무 깔끔하게 해주신거 같아요 👍👍

Comment on lines 124 to 139
if (isAuthorSelected) {
setFilteredData(prevData);
setPrevData([]);
// 현재 선택된 사용자를 다시 클릭하면 이전 데이터로 복원
const newFilteredData = prevData.length > 0 ? prevData.pop() : filteredData;
setFilteredData(newFilteredData ?? filteredData);
setPrevData([...prevData]);
setSelectedAuthor("");
} else if (d.name === "others") {
// "others" 바를 클릭할 때
setPrevData([...prevData, filteredData]);
setFilteredData(getNewFilteredData(d.names || []));
setSelectedAuthor(d.name);
} else {
setFilteredData(sortDataByAuthor(filteredData, d.name));
setPrevData(filteredData);
// 특정 사용자를 클릭할 때
setPrevData([...prevData, filteredData]);
setFilteredData(getNewFilteredData([d.name]));
setSelectedAuthor(d.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 early return 방식으로 수정하면 가독성이 더 좋아질 것 같습니다!!

if (isAuthorSelected) {
  // 현재 선택된 사용자를 다시 클릭하면 이전 데이터로 복원
  const newFilteredData = prevData.length > 0 ? prevData.pop() : filteredData;
  setFilteredData(newFilteredData ?? filteredData);
  setPrevData([...prevData]);
  setSelectedAuthor("");
  setSelectedData([]);
  tooltip.style("display", "none");
  return;
}

if (d.name === "others") {
  // "others" 바를 클릭할 때
  setPrevData([...prevData, filteredData]);
  setFilteredData(getNewFilteredData(d.names || []));
  setSelectedAuthor(d.name);
  setSelectedData([]);
  tooltip.style("display", "none");
  return;
}

// 특정 사용자를 클릭할 때
setPrevData([...prevData, filteredData]);
setFilteredData(getNewFilteredData([d.name]));
setSelectedAuthor(d.name);
setSelectedData([]);
tooltip.style("display", "none");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

감사합니다!!! 수정하도록 하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정했습니다!!

Copy link
Contributor

Choose a reason for hiding this comment

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

이런 제안 좋습니다!!

mdgarden

This comment was marked as off-topic.

@HIITMEMARIO
Copy link
Contributor Author

꼼꼼한 수정 넘 좋습니다!!👍👍👍👍👍 타입과 관련해서 한가지 코멘트 남겼습니다. 검토 해주시면 감사하겠습니다!

혹시 어디에 남겨주셨을까요...! 보이질 않는거 같아서....ㅠ

@mdgarden
Copy link
Contributor

@HIITMEMARIO 에고 죄송합니다 제가 확인을 잘못하고 남긴것 같아서 지웠습니다 ㅠㅠ 패스해주세욥🙇‍♀️

Copy link
Contributor

@ytaek ytaek left a comment

Choose a reason for hiding this comment

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

꼼꼼하게 디테일들 많이 수정해주셨네요 LGGGGTM 입니다!

Comment on lines 124 to +142
if (isAuthorSelected) {
setFilteredData(prevData);
setPrevData([]);
} else {
setFilteredData(sortDataByAuthor(filteredData, d.name));
setPrevData(filteredData);
// 현재 선택된 사용자를 다시 클릭하면 이전 데이터로 복원
const newFilteredData = prevData.length > 0 ? prevData.pop() : filteredData;
setFilteredData(newFilteredData ?? filteredData);
setPrevData([...prevData]);
setSelectedAuthor("");
setSelectedData([]);
tooltip.style("display", "none");
return;
}

if (d.name === "others") {
// "others" 바를 클릭할 때
setPrevData([...prevData, filteredData]);
setFilteredData(getNewFilteredData(d.names || []));
setSelectedAuthor(d.name);
setSelectedData([]);
tooltip.style("display", "none");
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍👍👍👍👍👍👍👍👍👍

@@ -2,6 +2,7 @@

.file-icicle-summary {
text {
fill: $white;
fill: var(--primary-color);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -2,6 +2,7 @@

.file-icicle-summary {
text {
fill: $white;
fill: var(--primary-color);
filter: invert(100) grayscale(100) contrast(100);
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 궁금합니다!!!! (css를 잘 모릅니다 🥲)

Copy link
Contributor

Choose a reason for hiding this comment

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

아, #558에 있던 내용이군요; 참고하였습니다.

@ytaek
Copy link
Contributor

ytaek commented Jul 30, 2024

정리되셨으면 merge 가시지요!

@HIITMEMARIO HIITMEMARIO merged commit 2db9fb5 into githru:main Jul 30, 2024
2 checks passed
@ytaek ytaek added this to the v0.7.0 milestone Aug 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ✨ view
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants