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): drawSubGraph function without any relationship between commits & e2e test to click on cluster #493

Closed
wants to merge 13 commits into from

Conversation

2taesung
Copy link
Contributor

Related issue

subGraph

Result

  • subGraph 화면
    image

  • e2e test report(transform test, subGraph(x))
    image

Work list

  • 이전 리팩토링 피드백 반영
  • drawSubGraph function
  • circle styling
  • getStartYEndY util function
  • e2e test to click on cluster(transform test, subGraph(x))

Discussion

  • subGraph에 각 commits 의 관계성을 갖는 선은 추가할 계획이긴 합니다.
  • 기타 외 다른 개발 아이디어나 현재 UI상에서도 개선점이 있다면 의견 부탁드립니다!

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.

고생하셨습니다!
이걸 베이스로 조금씩 수정해보면 실제 subgraph도 잘 만들 수 있을 것 같습니다!

@@ -11,11 +11,20 @@ export function getGraphHeight(clusterSizes: number[]) {
return clusterSizes.length * CLUSTER_HEIGHT + clusterSizes.length * NODE_GAP + NODE_GAP;
}

export function getStartYEndY(d: ClusterGraphElement, a: number, detailElementHeight: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

export function getTranslateAfterSelect(
d: ClusterGraphElement,
i: number,
detailElementHeight: number,
isPrev = false
isPrev = false // TODO - 해당 인자를 개선해야할듯
Copy link
Contributor

Choose a reason for hiding this comment

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

// TODO 는 좋은데, 한글 comment는 지양하면 좋겠습니다~.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 알겠습니다!
개발 중에 작성한 부분인데 실수로 commit에 들어갔네요 ㅠ

Copy link
Contributor

@KyuTae98 KyuTae98 left a comment

Choose a reason for hiding this comment

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

LGTM!! Sub graph 좋습니다!!

@2taesung 2taesung closed this Oct 3, 2023
@2taesung 2taesung reopened this Oct 3, 2023
@2taesung 2taesung closed this Oct 3, 2023
@2taesung 2taesung mentioned this pull request Oct 3, 2023
2taesung added a commit that referenced this pull request Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants