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

feat(view): return Summary value and modify content data #97

Merged
merged 8 commits into from
Sep 8, 2022

Conversation

jejecrunch
Copy link
Contributor

@jejecrunch jejecrunch commented Sep 7, 2022

Recent Issue

#40

Work List

86fb41c 37e1946
npm install 실행으로 인한 package-lock.json 내용 변경

76e447e

  1. 이전 Summary 데이터로 돌아가서 클러스터 별로 commit의 author을 가져와서 출력합니다.

author는 중복없이 한 사람의 프로필만 보여지며 background-color는 author의 char code를 이용하여 변경되도록 하였습니다.

  1. content가 들어가는 부분에 기존 클러스터 마지막 커밋 메시지를 출력하기로 했으나 다음과 같이 변경하여 구현하였습니다.

메시지의 경우는 클러스터의 마지막 커밋 메시지가 아닌 클러스터 안에 있는 커밋 메시지의 맨 앞 단어만 추출하여 counting하고 가장 많이 나온 단어부터 보여주도록 수정하였습니다.
그렇게 변경한 이유는 기존의 마지막 커밋 메시지만 보여주는 게 그 클러스터에서 어떤 일이 일어나는지 정확히 알 수 없다고 느껴졌고, 커밋 메시지만 보여준다면 해당 부분에서 오해의 소지가 발생할 수 있을 것 같아 변경하게 되었습니다 ! 현재로서는 앞 단어 모음 배열이 가장 현실적이고 해당 클러스터를 얘기해줄 수 있을 것 같아 진행하였습니다. 이후 엔진팀에서 가공해주신다면 그 데이터로 대체될 예정입니다.

Result

(변경전)
ezgif com-gif-maker (1)

(변경후)
ezgif com-gif-maker

고민해봐야할 점

  1. content 부분에서 커밋 메시지 키워드를 count하여 많이 나온 단어부터 보여주도록 하였는데 여기서 font size 등으로 style에 변화를 주는 것이 괜찮은 부분인지
  2. author 부분에서 name마다 background-color를 다르게 주고 있는데 비슷한 색이 나오지 않도록 어떻게 처리할지

To do

  • 데이터 내려오는 거 어떻게 내려오는지 파악 후 수정할 것
  • fake asset에서 taskId가 clusterId로 바꼈기 때문에 이 부분 util에서 수정할 것
  • 컬러 부분 함수 한번 나온 색은 나올 수 없도록 수정할 것

@jejecrunch jejecrunch self-assigned this Sep 7, 2022
@jejecrunch jejecrunch requested a review from a team as a code owner September 7, 2022 09:52
@jejecrunch jejecrunch changed the title [view] return Summary feat(view): return Summary value and modify content data Sep 7, 2022
Comment on lines 72 to 74
&:nth-child(2n){
&:hover {
@include animation();
margin-right: 10px;
}

&:first-child {
margin-right: 10px;
}

&:last-child {
margin-right: 0px;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

author의 마지막 부분이 움직이지 않게 조정했습니다 !

justify-content: center;
align-items: center;
text-align: center;
font-size: 15pt;
margin-left: -5px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

author 간 간격이 넓다는 의견을 주셔서 지정하였습니다 !

Comment on lines +21 to +26
// set names
const authorSet: Set<string> = new Set();
commitNode.commit.author.names.map((name) => {
authorSet.add(name.trim());
return name.trim();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

여기서는 한 커밋 안에 있는 author를 중복 제거해줍니다.

Comment on lines +51 to +63
const authorsSet = cluster.summary.authorNames.reduce(
(set, authorArray) => {
authorArray.forEach((author) => {
set.add(author);
});
return set;
},
new Set()
);

cluster.summary.authorNames = [];

cluster.summary.authorNames.push(Array.from(authorsSet) as Array<string>);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

여기서 한 클러스터 안에 있는 author를 중복 제거 해줍니다.

@@ -1,15 +1,15 @@
export type Author = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

기존의 Author를 제거한 이유는 현재 중복제거가 된 name이 들어가기 때문에 name 자체가 고유한 값입니다. 그래서 id를 따로 지정해주지 않아도 된다고 생각하여 id 속성을 제거하다보니 Author type도 굳이 사용할 필요가 없어서 그렇게 진행하였습니다.


export function getInitData({ data }: GlobalProps) {
const clusters: Cluster[] = [];

data.map((clusterNode) => {
const cluster: Cluster = {
id: nanoid(),
commits: [],
clusterId: clusterNode.commitNodeList[0].taskId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jin-Pro 님과 협의하여 clusterId로 작업하기로 했는데 아직 merge된 부분이 아니라 우선 제 코드에서는 taskId로 돌아가기 때문에 남겨두었습니다.

className="name"
data-tooltip-text={authorName.name}
key={authorName}
className={["name"].join(" ")}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

처음에는 className으로 어떻게 해보려고 하다가 어떻게 해야할지 감이 잘 안와서 style 인라인 태그로 계산된 color값을 넣고 있습니다!

<span className="nameBox">
{commit.authorNames.map((authorName: Author) => {
<div className="cluster" key={cluster.clusterId}>
<p className="summary" key={cluster.summary.summaryId}>
Copy link
Contributor

Choose a reason for hiding this comment

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

(궁금) 여기의 key도 필요한지요? loop의 제일 가장자리만 key가 있으면 될 것 같아서 여쭤봅니다.

Copy link
Contributor Author

@jejecrunch jejecrunch Sep 7, 2022

Choose a reason for hiding this comment

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

생각해보니 저 부분 id는 필요 없겠군요 ! 피드백 감사합니다 !! 해당 부분 수정해서 다시 올렸습니다 !!

Copy link
Contributor

Choose a reason for hiding this comment

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

오. 그렇다면
아래의 nameBox도 필요없을 것 같고,
keywords들에 걸린 것도 keyword 값으로 해도 충분할 것 같은데, 한번 검토 부탁드립니다~.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이전에 중복된 값이 계속 들어가서 id가 필요했는데 keyword도 author와 마찬가지로 중복없이 들어가서 id 없이도 괜찮네요 !! 해당 부분 수정해서 올리겠습니다 !!

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.

WOW!!!!
컴포넌트들이 합체!! 될때의 모습이 정말 기대됩니다!! 🛩️

@jejecrunch
Copy link
Contributor Author

jejecrunch commented Sep 8, 2022

e7a3b9f
color 계열이 변해서 해당 color 계열로 맞춰 작업 진행했고, author의 이름이 다르더라도 같은 color가 나오던 문제를 해결했습니다.

Comment on lines +1 to +9
export const authorBgColorArray = [
"00ADF7",
"0077AA",
"4AC3F7",
"0BD9E0",
"33C2FF",
"4BC4F9",
"0089C4",
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

최대한 비슷한 계열로 넣어보았는데 너무 색이 튀면 다시 조정해보는 게 필요해보입니다.

return cluster;
});

return clusters;
}

export function getColorValue(name: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

author의 앞 2자를 가져와서 컬러값을 지정해줍니다.

@hy57in hy57in self-requested a review September 8, 2022 07:14
@jejecrunch jejecrunch merged commit 9077c99 into githru:main Sep 8, 2022
@hanseul-lee hanseul-lee added this to the v0.1.0 milestone Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants