[fix]: 커밋 메시지 가독성 및 Detail 컴포넌트 UI 레이아웃 개선 #828 by 2Estella · Pull Request #829 · githru/githru-vscode-ext

2Estella · 2025-08-10T14:46:57Z

Related issue

Closes #828

Result

Before	After

Before:

커밋 메시지가 한 줄로 표시되어 가독성 부족
제목과 본문 구분이 어려움
UI 레이아웃이 직관적이지 않음

After:

커밋 메시지 제목과 본문이 명확하게 구분되어 표시
아바타-제목-메타정보의 직관적인 레이아웃
마우스 오버 시 본문 표시로 정보 접근성 향상
깔끔하고 읽기 쉬운 UI 구조

Work list

커밋 메시지 제목과 본문 분리 파싱 및 표시 기능 구현
Detail 컴포넌트 UI 레이아웃 최적화 (아바타 위치, 메타정보 배치)
Summary 컴포넌트 제목만 표시하도록 단순화
ClusterGraph 툴팁에서 커밋 메시지 제목과 본문 분리 표시
호버 효과 및 애니메이션 개선
LinkedMessage 타입 정의 및 적용
불필요한 div 구조 정리 및 코드 최적화

Discussion

주요 변경사항

커밋 메시지 구조 개선: 제목과 본문을 명확하게 분리하여 가독성 향상
UI 레이아웃 최적화: 아바타를 제목 옆에 배치하고 메타정보를 한 줄에 표시
Summary 컴포넌트 단순화: 제목만 표시하여 정보 과부하 방지
ClusterGraph 툴팁 개선: 커밋 메시지 제목과 본문을 두 줄로 구분하여 표시
사용자 경험 개선: 본문은 필요할 때만 표시하여 정보 접근성 향상
코드 품질 향상: 불필요한 중첩 구조 제거로 유지보수성 개선

기술적 고려사항

정규표현식을 활용한 커밋 메시지 파싱
React 상태 관리를 통한 동적 UI 처리
CSS transition을 활용한 부드러운 애니메이션
TypeScript 타입 안정성 강화
D3.js 툴팁 내용 개선

테스트 시나리오

커밋 메시지 제목과 본문이 올바르게 분리되어 표시되는지 확인
GitHub 이슈 번호가 클릭 가능한 링크로 변환되는지 확인
아바타와 제목이 올바르게 정렬되는지 확인
메타정보가 한 줄에 깔끔하게 표시되는지 확인
ClusterGraph 툴팁에서 제목과 본문이 구분되어 표시되는지 확인

### 관련 타입 수정 - LinkedMessage 타입 정의 및 적용 - Detail, Content 컴포넌트 타입 안정성 향상 ### 기능 수정 - 커밋 메시지 제목과 본문 분리 파싱 및 표시 - GitHub 이슈 링크 자동 변환 기능 복구 - 마우스 오버 시 본문 표시 기능 구현 ### 스타일 수정 - Detail 컴포넌트: 아바타를 제목 옆에 배치, 메타정보 한 줄 표시 - Summary 컴포넌트: 제목만 표시하도록 단순화 - 호버 효과 및 애니메이션 개선 - 불필요한 div 구조 정리 및 레이아웃 최적화 ### 개선 효과 - 커밋 메시지 가독성 대폭 향상 - UI 레이아웃의 직관성 개선 - 코드 구조 단순화 및 유지보수성 향상

ytaek

잘 정리되었으면!! 했던 가려운 부분을 팍 긁어주셨네요!!!

전체적으로 LGGGTM!!!입니다!!

@2Estella님,님, 추가로 중간중간에 구조적으로 refactoring할 부분들이 보이는 것 같은데용.

refactoring 할 item들을 이슈로 등록하시고,
해당 PR merge 하신 다음에!
이슈에 본인 할 당 후 다음 PR로 refactoring 진행하시면 좋을 것 같습니다!

refactoring해서 별도의 ts logic들이 튀어나오면 거기에 대해 Test case를 추가하는 것도 별도의 추가 task 로 잡으면 좋을 것 같습니다 👍

ytaek · 2025-08-11T13:15:03Z

+
+  useEffect(() => {
+    const processMessage = (message: string) => {
+      // GitHub 이슈 번호 패턴: #123 또는 (#123)


#826 (comment)

에서도 얘기했지만, 주석은 영어로 하면 좋을 것 같습니다.
저는 필요없다면 주석 없이 코드로 얘기하는걸 선호하긴 합니다 : )

예을 들자면, regex 변수 이름을 githubIssuePatternRegex 라고 짓는다던지요 😸

ytaek · 2025-08-11T13:16:55Z

  };
+
+  useEffect(() => {
+    const processMessage = (message: string) => {


전체적으로 processMessage function이 길어져서
component를 읽기가 조금 힘들어지는 경향이 있는 것 같습니다.
요렇게 긴건 extract method해서 다른 곳에 옮겨놓는 것도 좋을 것 같습니다!!

(다음 PR에서 refactoring으로 새로/따로 올려주시면 리뷰하기 편할 것 같습니다!!!)

함수 내 헬퍼함수를 만들면 내부로직을 보지 않아도 processMessage가 어떤 역할을 하는 함수인지 흐름을 따라가기 쉬워질 것 같아요!

주석으로 나눠진 코드 덩어리마다 헬퍼함수를 만드는 것도 좋을 것 같습니다! 추상화가 너무 과하다고 생각하신다면 두번째 주석만 헬퍼함수로 추출해도 좋을 것 같은데 어떻게 생각하시나요??

이슈 번호 앞의 텍스트 추가, 이슈 번호를 링크로 변환, 마지막 부분 추가

ytaek · 2025-08-11T13:17:43Z

+      const processedTitle = processMessage(title);
+      const processedBody = body ? processMessage(body) : null;
+
+      setLinkedMessage({


LinkedMessage라는게 어떤 의미일까요?
git이나 github 쪽에서는 저는 잘 못 본 것 같아서 여쭤봅니당!!

ytaek · 2025-08-11T13:18:29Z

+                <div className="commit-item__message-container">
+                  <div className="commit-message">
+                    <div className="commit-message__header">


(html 잘몰라서 여쭤봅니당,,,어려워요 ㅜ.ㅜ) div가 많이 중첩되는데, 줄일 수 있는 방법은 없을까요?

ytaek · 2025-08-11T13:20:00Z

-        const matchedStrNum: string = matchedStr.substring(1);
-        const linkIssues = `https://github.com/${owner}/${repo}/issues/${matchedStrNum}`;
-        acc.push(
+    const processMessage = (message: string) => {


Detail.tsx

에서도 동일한 내용이 있는 것 같습니다!!
공용화 시킬 수 있는 부분은 공용화 해도 좋을 것 같아요!!

ytaek · 2025-08-11T13:20:55Z

+        parts.push(
          <a
-            href={linkIssues}
-            key={`issues-${matchedStr}`}
-            className="summary__commit-link"
+            key={`issue-${issueNumber}-${match.index}`}
+            href={issueLink}
+            target="_blank"
+            rel="noopener noreferrer"
+            className="summary__commit-issue-link"
+            title={`GitHub Issue #${issueNumber}`}
          >
-            {matchedStr}
+            {match[1]}
          </a>
        );


useEffect안에 jsx 가 섞여 있긴 한데, 이런 경우는 따로 component로 빼는 것도 방법일 것 같습니다!

grapefruit13

UI가 많이 개선되었네요 고생하셨습니다!
다만 커밋양을 잘게 쪼개서 PR 올려주시면 더 좋을 것 같습니다 !!
코드보면서 궁금한 점들 코멘트 달았는데 시간되실 때 봐주십쇼👍🏻👍🏻

grapefruit13 · 2025-08-11T22:33:28Z

+                        {(() => {
+                          const messageLines = message.split("\n");
+                          const title = messageLines[0];
+                          return linkedMessage.title.length > 0 ? linkedMessage.title : title;
+                        })()}


여기서 즉시 실행 함수를 사용하신 이유가 궁금합니다!!

grapefruit13 · 2025-08-11T22:35:39Z

-      .commit-item__right {
-        display: flex;
+        flex: 1;
+        margin-left: 0.125rem; // 아주 미세한 여백


이 부분은 주석없이 코드만 봐도 이해가 충분할 것 같습니다 ㅎㅎ

grapefruit13 · 2025-08-11T22:38:50Z

+                    {(() => {
+                      const messageLines = message.split("\n");
+                      const body = messageLines
+                        .slice(1)
+                        .filter((line: string) => line.trim())
+                        .join("\n");
+
+                      return body ? (
+                        <div className="commit-message__body">{linkedMessage.body ? linkedMessage.body : body}</div>
+                      ) : null;
+                    })()}


여기도 IIFE를 특별히 사용하신 이유가 있는지 궁금합니다 ㅎㅎ

grapefruit13 · 2025-08-11T22:41:13Z

+            gap: 0.75rem;
+            margin-bottom: 0.5rem;
+
+            // 아바타를 맨 앞에 배치


여기 주석도 마찬가지입니다!

grapefruit13 · 2025-08-11T22:42:01Z

      }
+
+      .commit-item__right {
+        display: none; // 오른쪽 레이아웃 숨김


이 부분도 마찬가지입니다! css는 특히 주석없이도 이해가 어렵지 않아서 주석은 웬만하면 없어도 될 것 같습니다 ㅎㅎ

SingTheCode

githru 사용할 때마다 항상 눈에 걸리던 부분인데 깔끔하게 구현해주셨네요ㅎㅎ 사용성이 대폭 개선된 것 같아요ㅎㅎ 고생하셨습니다!!!

Detail.tsx, Content.tsx에 중복된 코드들이 조금 보여서 중복제거까지 해주시면 너무 좋을 것 같아요ㅎㅎ

추출할 수 있는 함수는 이정도 일 것 같아요!

GitHub 이슈번호를 링크로 변환하는 함수
커밋 메세지를 제목과 본문으로 분리하는 함수
커밋 메시지에서 GitHub 이슈 링크를 처리하여 LinkedMessage 객체를 생성하는 함수

SingTheCode · 2025-08-17T14:21:53Z

+                          const messageLines = message.split("\n");
+                          const title = messageLines[0];
+                          return linkedMessage.title.length > 0 ? linkedMessage.title : title;
+                        })()}


함수 추출해서 사용하는 것도 가독성에 좋을 것 같습니다ㅎㅎ 그리고 const title = message.split("\n")[0];로 바로 title을 사용해도 괜찮을 것 같아요ㅎㅎ

SingTheCode · 2025-08-17T14:26:38Z

  };
+
+  useEffect(() => {
+    const processMessage = (message: string) => {


함수 내 헬퍼함수를 만들면 내부로직을 보지 않아도 processMessage가 어떤 역할을 하는 함수인지 흐름을 따라가기 쉬워질 것 같아요!

주석으로 나눠진 코드 덩어리마다 헬퍼함수를 만드는 것도 좋을 것 같습니다! 추상화가 너무 과하다고 생각하신다면 두번째 주석만 헬퍼함수로 추출해도 좋을 것 같은데 어떻게 생각하시나요??

이슈 번호 앞의 텍스트 추가, 이슈 번호를 링크로 변환, 마지막 부분 추가

SingTheCode · 2025-08-17T14:28:30Z

-      const info = commitNodeList[circleIndex].commit.message;
-      tooltip.text(info);
+      const { message } = commitNodeList[circleIndex].commit;
+      const messageLines = message.split("\n");


title과 body를 추출하는 로직이 중복되는 것 같은데 함수로 추출해서 중복을 제거하는 것이 좋을 것 같습니다ㅎㅎ

chae-dahee

추후 작업으로 Issue #880 에서 코드리뷰 달린 리팩토링 내용들 진행하겠습니다!

2Estella requested a review from a team as a code owner August 10, 2025 14:46

ytaek approved these changes Aug 11, 2025

View reviewed changes

grapefruit13 reviewed Aug 11, 2025

View reviewed changes

SingTheCode reviewed Aug 17, 2025

View reviewed changes

chae-dahee mentioned this pull request Sep 7, 2025

[fix]: PR #829 refactoring item 논의 #880

Open

20 tasks

chae-dahee approved these changes Sep 7, 2025

View reviewed changes

chae-dahee merged commit fcf0e8b into main Sep 7, 2025
2 checks passed

Uh oh!

Conversation

2Estella commented Aug 10, 2025 • edited by ojspp41 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issue

Result

Before:

After:

Work list

Discussion

주요 변경사항

기술적 고려사항

테스트 시나리오

Uh oh!

ytaek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grapefruit13 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SingTheCode left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chae-dahee left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

2Estella commented Aug 10, 2025 •

edited by ojspp41

Loading

chae-dahee left a comment •

edited

Loading